All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=726937

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDghsmIvAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDghuzptwoM


Bot(s) for this bug's original alert(s):

android-nexus5X
android-nexus6

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8978488622069441360

=== Auto-CCing suspected CL author bcwhite@chromium.org ===

Hi bcwhite@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : bcwhite
  Commit : 9963f7ea173756bc0b8014ed0dcd9bd48a6d3598
  Date   : Wed May 24 20:52:36 2017
  Subject: Added support for 'spare' file that can be used at startup.

Bisect Details
  Configuration: android_nexus6_perf_bisect
  Benchmark    : thread_times.simple_mobile_sites
  Metric       : thread_other_cpu_time_per_frame/thread_other_cpu_time_per_frame
  Change       : 441.63% | 0.0621548969402 -> 0.336649333227

Revision             Result                      N
chromium@474399      0.0621549 +- 0.0225715      6      good
chromium@474413      0.0759609 +- 0.0170985      6      good
chromium@474414      0.338287 +- 0.0238352       6      bad       <--
chromium@474415      0.341598 +- 0.039378        6      bad
chromium@474417      0.341508 +- 0.0192857       6      bad
chromium@474420      0.340323 +- 0.00764274      6      bad
chromium@474426      0.341494 +- 0.0369557       6      bad
chromium@474453      0.335502 +- 0.00779843      6      bad
chromium@474507      0.336649 +- 0.0284368       6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests thread_times.simple_mobile_sites

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8978488622069441360

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5592751783018496


| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Speed>Bisection.  Thank you!

Yes.  If persistent metrics are enabled (which will eventually become the default) then a background thread (priority LOWEST) will at some point be tasked with creating the file in which metrics will be stored.  Currently this happens about 10 seconds after startup.

This is expected and it's better to do it as this background thread that spend time doing it during startup.

 Issue 727508  has been merged into this issue.

cc vmiura for thread_times benchmark, and johnchen for media benchmarks that are affected.

This seems like a pretty huge regression in cpu usage. Do we know how long this background task runs for? Here's some example traces from nytimes on thread_times benchmark:

Before:
https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_2-2017-05-24_20-23-21-29479.html
After:
https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_2-2017-05-24_20-23-21-29479.html

It's a huge percentage but a small amount: 200-300us.  It saves about 100-150ms off of startup, though, to move the i/o here.

This is the code that is running:
https://cs.chromium.org/chromium/src/base/files/memory_mapped_file_posix.cc?rcl=31d985109db0a11cdd7cd57f342ea260736f8e5c&l=115

It's delayed until 10s after startup and runs with "lowest" priority.

Thanks! Do you think it makes sense to flag this off during benchmarks? Essentially, we're measuring this in most benchmarks because they run from a fresh browser start. Although it doesn't seem to increase noise, it affects the score quite a bit, and based on your comments that seems artificial.

For android, it can certainly be turned off.  The best would to force persistent metrics into local memory rather than a mapped file.

--enable-features='PersistentHistograms<PersistentHistograms' --force-fieldtrials=PersistentHistograms/cmdline --force-fieldtrial-params=PersistentHistograms.cmdline:storage/LocalMemory

The plan is to remove that flag and make this an always-on feature but if it's important for some testing then we can leave some kind of flag to disable the on-disk portion of it.

Left a closing ">" out of the flags.

--enable-features='PersistentHistograms<PersistentHistograms>' --force-fieldtrials=PersistentHistograms/cmdline --force-fieldtrial-params=PersistentHistograms.cmdline:storage/LocalMemory

 Issue 727630  has been merged into this issue.

 Issue 727631  has been merged into this issue.

After some off-line discussion...  Perhaps you should talk to the Lucky-Luke team about a flag to disable all unimportant background tasks during performance benchmarks.

The scheduler should just be good at delaying background work such that it doesn't cause regressions, artificially hiding the background tasks doesn't sound great. +fdoray is working on making sure background tasks don't perturb the system even on OSes where we don't toggle kernel-level thread priorities (which includes Android because it sadly fails to support some POSIX APIs to avoid priority inversions)

Re #14: This and several other similar background tasks in the past have created the same problem for telemetry:

1) Someone adds some code which was meant to run once at startup.
2) The code is deferred and run in the background so as not to block startup.
3) Since the telemetry benchmarks start the browser for every test case, we see large regressions across the board caused by these bits of background code, but they aren't reflective of the general browsing case we're trying to measure, just what happens immediately after startup.

This bug is one example, and bug 644258 and bug 588745.

The problem this causes for telemetry is that the metrics can be skewed or more noisy because of these background tasks. In #14, are you suggesting that Lucky Luke should be able to schedule these tasks such that they don't cause any regressions at all? If so, should we reopen this and assign to someone from Lucky Luke?

That's the ultimate goal yes. fdoray@ is working on that right now.

Awesome, I'm very happy to hear that!

We observed the same problem with a background task causing a benchmark regression in  bug 737394 

I have suspicions that delaying of all BACKGROUND tasks won't currently work for many benchmarks because there are some misattributed tasks that have BACKGROUND priority but are necessary for a page loading. Initialization of the sqlite_persistent_cookie_store is an example of such task.

FYI pasko@ and I are working on fix.

Re. "It won't work per some work being mislabeled". I was thinking of
adding a browser test that forces this flag and confirms everything still
works as a cross-check that nothing critical runs at BACKGROUND priority.

Thanks for working on it!

Le mer. 9 août 2017 13 h 27, alexilin via monorail <
monorail+v2.2530406448@chromium.org> a écrit :

Perf sheriff checking in: are we still keeping this bug open for the work in #16?

Yes we can use this bug for #18 too and earlier comments to require that
the browser be operational without running background tasks (and provide a
flag to force them not to run for first X minutes).

[Removing Performance-Sheriff flag since I don't think it helps to have sheriffs pinging for an update on the graph]

Having a way to disable background work could help evaluating more reliably the impact of a hardware configuration change on Chrome performance https://groups.google.com/a/google.com/d/msg/chrome-speed/n6YHmNwWMI8/D3VMDLEMCAAJ

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/088609dd812fbf6084a5bfbdfcb7e5252265ff46

commit 088609dd812fbf6084a5bfbdfcb7e5252265ff46
Author: Francois Doray <fdoray@chromium.org>
Date: Mon Feb 12 19:52:05 2018

Use the same arguments for the constructors of TaskTracker and TaskTrackerPosix.

This is required because both classes are used interchangeably by
TaskSchedulerImpl (cf.
https://cs.chromium.org/chromium/src/base/task_scheduler/task_scheduler_impl.h?l=46&rcl=14b157142ce0febc22c1bd836f7d8ccdc2931e81)

Bug: 726937
Change-Id: Ia82edbb446b44551fd579b4ebc63be9c94682e62
Reviewed-on: https://chromium-review.googlesource.com/912097
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536162}
[modify] https://crrev.com/088609dd812fbf6084a5bfbdfcb7e5252265ff46/base/task_scheduler/task_tracker_posix.cc
[modify] https://crrev.com/088609dd812fbf6084a5bfbdfcb7e5252265ff46/base/task_scheduler/task_tracker_posix.h

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/puppet/+/af6f7666715b75625993c23cf51e69bb9beccee0

commit af6f7666715b75625993c23cf51e69bb9beccee0
Author: Charlie Andrews <charliea@chromium.org>
Date: Mon Feb 12 19:57:26 2018

Currently working on a --disable-background-tasks command-line flag that will cause background tasks to be queued until shutdown starts.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/762ffa9585784d093784efe1b6a27a04ede24fd9

commit 762ffa9585784d093784efe1b6a27a04ede24fd9
Author: Francois Doray <fdoray@chromium.org>
Date: Tue Feb 13 19:27:23 2018

[TaskScheduler] Do not limit concurrent background sequences during shutdown.

Background sequences are often used to write data to disk. During
normal execution, it is not urgent to run these tasks, but during
shutdown, they should run as quickly as possible to allow the
process to exit.

Another reason not to limit concurrent background sequences
during shutdown is to support a --disable-background-tasks flag.
With this flag, Chrome doesn't run any background tasks during
normal execution. It does however need to run these tasks on
shutdown for correctness.

Bug: 726937
Change-Id: I4ca1b34ad293620acd8a148479c94a59ef2a2c85
Reviewed-on: https://chromium-review.googlesource.com/914447
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536419}
[modify] https://crrev.com/762ffa9585784d093784efe1b6a27a04ede24fd9/base/task_scheduler/task_tracker.cc
[modify] https://crrev.com/762ffa9585784d093784efe1b6a27a04ede24fd9/base/task_scheduler/task_tracker.h
[modify] https://crrev.com/762ffa9585784d093784efe1b6a27a04ede24fd9/base/task_scheduler/task_tracker_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9550a3b80c28166fd8fe2c80867543b6077fc224

commit 9550a3b80c28166fd8fe2c80867543b6077fc224
Author: Francois Doray <fdoray@chromium.org>
Date: Tue Feb 20 17:29:22 2018

Initialize command line before TaskScheduler on iOS.

The command line is initialized before TaskScheduler on non-iOS
platforms. That allows TaskScheduler to read configuration flags
on the command line.

Bug: 726937
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I43e430aecbfa88927244f6db94bcb0b81e608426
Reviewed-on: https://chromium-review.googlesource.com/924375
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537808}
[modify] https://crrev.com/9550a3b80c28166fd8fe2c80867543b6077fc224/ios/web/public/global_state/ios_global_state.mm

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b4401f0ecb3645eb61c7e9f7a2b4348bfb036a2

commit 7b4401f0ecb3645eb61c7e9f7a2b4348bfb036a2
Author: Francois Doray <fdoray@chromium.org>
Date: Tue Feb 20 20:03:15 2018

Add --disable-background-tasks flag.

This flag delays execution of base::TaskPriority::BACKGROUND tasks
until shutdown.

Bug: 726937
Change-Id: I2ce529a410382e7199558a0ed95476767b14786b
Reviewed-on: https://chromium-review.googlesource.com/919866
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537862}
[modify] https://crrev.com/7b4401f0ecb3645eb61c7e9f7a2b4348bfb036a2/base/base_switches.cc
[modify] https://crrev.com/7b4401f0ecb3645eb61c7e9f7a2b4348bfb036a2/base/base_switches.h
[modify] https://crrev.com/7b4401f0ecb3645eb61c7e9f7a2b4348bfb036a2/base/task_scheduler/task_tracker.cc
[modify] https://crrev.com/7b4401f0ecb3645eb61c7e9f7a2b4348bfb036a2/base/task_scheduler/task_tracker.h
[modify] https://crrev.com/7b4401f0ecb3645eb61c7e9f7a2b4348bfb036a2/base/task_scheduler/task_tracker_posix.cc
[modify] https://crrev.com/7b4401f0ecb3645eb61c7e9f7a2b4348bfb036a2/base/task_scheduler/task_tracker_posix.h
[modify] https://crrev.com/7b4401f0ecb3645eb61c7e9f7a2b4348bfb036a2/chrome/browser/ui/startup/bad_flags_prompt.cc
[modify] https://crrev.com/7b4401f0ecb3645eb61c7e9f7a2b4348bfb036a2/content/browser/browser_child_process_host_impl.cc
[modify] https://crrev.com/7b4401f0ecb3645eb61c7e9f7a2b4348bfb036a2/content/browser/renderer_host/render_process_host_impl.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc8ef3f79ae198d7527e10dedbae19a76772826c

commit bc8ef3f79ae198d7527e10dedbae19a76772826c
Author: Andrei Kapishnikov <kapishnikov@chromium.org>
Date: Wed Feb 21 00:04:17 2018

Revert "Add --disable-background-tasks flag."

This reverts commit 7b4401f0ecb3645eb61c7e9f7a2b4348bfb036a2.

Reason for revert: Broke Cronet tests:
https://ci.chromium.org/buildbot/chromium.android/Android%20Cronet%20Marshmallow%2064bit%20Perf/16545

Original change's description:
> Add --disable-background-tasks flag.
> 
> This flag delays execution of base::TaskPriority::BACKGROUND tasks
> until shutdown.
> 
> Bug: 726937
> Change-Id: I2ce529a410382e7199558a0ed95476767b14786b
> Reviewed-on: https://chromium-review.googlesource.com/919866
> Commit-Queue: François Doray <fdoray@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Tommy Martino <tmartino@chromium.org>
> Reviewed-by: Robert Liao <robliao@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#537862}

TBR=gab@chromium.org,jam@chromium.org,fdoray@chromium.org,robliao@chromium.org,tmartino@chromium.org

Change-Id: Ibc7b70e5bdebd953ad6397507bd1f0673622942f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 726937
Reviewed-on: https://chromium-review.googlesource.com/927282
Reviewed-by: Andrei Kapishnikov <kapishnikov@chromium.org>
Commit-Queue: Andrei Kapishnikov <kapishnikov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537943}
[modify] https://crrev.com/bc8ef3f79ae198d7527e10dedbae19a76772826c/base/base_switches.cc
[modify] https://crrev.com/bc8ef3f79ae198d7527e10dedbae19a76772826c/base/base_switches.h
[modify] https://crrev.com/bc8ef3f79ae198d7527e10dedbae19a76772826c/base/task_scheduler/task_tracker.cc
[modify] https://crrev.com/bc8ef3f79ae198d7527e10dedbae19a76772826c/base/task_scheduler/task_tracker.h
[modify] https://crrev.com/bc8ef3f79ae198d7527e10dedbae19a76772826c/base/task_scheduler/task_tracker_posix.cc
[modify] https://crrev.com/bc8ef3f79ae198d7527e10dedbae19a76772826c/base/task_scheduler/task_tracker_posix.h
[modify] https://crrev.com/bc8ef3f79ae198d7527e10dedbae19a76772826c/chrome/browser/ui/startup/bad_flags_prompt.cc
[modify] https://crrev.com/bc8ef3f79ae198d7527e10dedbae19a76772826c/content/browser/browser_child_process_host_impl.cc
[modify] https://crrev.com/bc8ef3f79ae198d7527e10dedbae19a76772826c/content/browser/renderer_host/render_process_host_impl.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b3b1d34a763ebf8a5153c74d4917fcb09b40a2e5

commit b3b1d34a763ebf8a5153c74d4917fcb09b40a2e5
Author: Francois Doray <fdoray@chromium.org>
Date: Thu Apr 05 14:31:23 2018

Initialize CommandLine before TaskScheduler in quic.

TaskScheduler needs access to command line arguments in an
upcoming CL that allows disabling background tasks via a command
line argument
(https://chromium-review.googlesource.com/c/chromium/src/+/996439).

The order of initialization matches what is done elsewhere in the
codebase.

Bug: 726937
Change-Id: I4e709db4c7f36c9e062d43aa686f5309cc8f3745
Reviewed-on: https://chromium-review.googlesource.com/996559
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548416}
[modify] https://crrev.com/b3b1d34a763ebf8a5153c74d4917fcb09b40a2e5/net/tools/quic/quic_server_bin.cc
[modify] https://crrev.com/b3b1d34a763ebf8a5153c74d4917fcb09b40a2e5/net/tools/quic/quic_simple_client_bin.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1519479b3d2528f2b526d79665da3725491e5d62

commit 1519479b3d2528f2b526d79665da3725491e5d62
Author: Francois Doray <fdoray@chromium.org>
Date: Thu Apr 05 17:01:09 2018

Add --disable-background-tasks flag (reland).

This flag delays execution of base::TaskPriority::BACKGROUND tasks
until shutdown.

This CL first landed as https://chromium-review.googlesource.com/919866.
It was reverted because the CommandLine wasn't always available during
TaskScheduler initialization (e.g.
https://cs.chromium.org/chromium/src/components/cronet/cronet_global_state_stubs.cc?l=34&rcl=975444866735e5e44b024707a7ff28582b8383cd).
This CL fixes the issue by adding a check for CommandLine
initialization before accessing it during TaskScheduler initialization.

TBR=gab@chromium.org,jam@chromium.org,tmartino@chromium.org

Bug: 726937
Change-Id: I0500c54222234e6cb88a1f2d7e92a7e87b6656fa
Reviewed-on: https://chromium-review.googlesource.com/996439
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548457}
[modify] https://crrev.com/1519479b3d2528f2b526d79665da3725491e5d62/base/base_switches.cc
[modify] https://crrev.com/1519479b3d2528f2b526d79665da3725491e5d62/base/base_switches.h
[modify] https://crrev.com/1519479b3d2528f2b526d79665da3725491e5d62/base/task_scheduler/task_tracker.cc
[modify] https://crrev.com/1519479b3d2528f2b526d79665da3725491e5d62/base/task_scheduler/task_tracker.h
[modify] https://crrev.com/1519479b3d2528f2b526d79665da3725491e5d62/base/task_scheduler/task_tracker_posix.cc
[modify] https://crrev.com/1519479b3d2528f2b526d79665da3725491e5d62/base/task_scheduler/task_tracker_posix.h
[modify] https://crrev.com/1519479b3d2528f2b526d79665da3725491e5d62/chrome/browser/ui/startup/bad_flags_prompt.cc
[modify] https://crrev.com/1519479b3d2528f2b526d79665da3725491e5d62/content/browser/browser_child_process_host_impl.cc
[modify] https://crrev.com/1519479b3d2528f2b526d79665da3725491e5d62/content/browser/renderer_host/render_process_host_impl.cc