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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg7_fNtgoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgr66lrAsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgr8CirgkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgz9XUtwkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgr4mcvgkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgz7jspwsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgj_nz-QoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgr86HsQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgj4WdswsM


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

android-nexus5
android-nexus6
android-nexus7v2
chromium-rel-mac-retina
chromium-rel-mac10
chromium-rel-win8-dual

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

=== PERF REGRESSION ===


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

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


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Experiment with redirecting all BrowserThreads (but UI/IO) to TaskScheduler
Author  : gab
Commit description:
  
This is part of the migration phase for TaskScheduler, design doc:
https://docs.google.com/document/d/1S2AAeoo1xa_vsLbDYBsDHCqhrkfiMgoIPlyRi6kxa5k/edit

Also generalizes BrowserThreadImpl away from the underlying thread's MessageLoop
and id in favor of TaskRunners and BrowserThreadState.

Cool side-effect: BrowserThreadImpl::StartWithOptions() no longer blocks on the
underlying thread starting (it used to via GetThreadId()), it now takes a reference
to its TaskRunner and lets the thread itself start on its own schedule :).
This wasn't previously intended to block (BrowserThreadImpl::StartAndWaitForTesting() is
supposed to be used for that) but had unintentionally regressed in http://crrev.com/408295
because GetThreadId() implicitly waits ( http://crbug.com/672977 ).

Another nice improvement is that ~BrowserThreadImpl() now cleans the BrowserThreadGlobals
associated with it, this has the side-effect to force proper destruction order of
TestBrowserThread in tests which brought forth a few precursor cleanups
( https://crbug.com/653916#c24 ) which will from now on be enforced by this CL.

When redirection is disabled, the logic should be the exact same as before.
 - Threads are brought up.
 - Tasks are posted to their MessageLoop (albeit through their TaskRunner now).
 - On shutdown, threads are joined one by one.

When redirection is enabled, we try to mimic this logic.
 - Redirection TaskRunners are only live (|accepting_tasks|) for the same period that the
   matching thread would be.
 - On shutdown, we block on each TaskRunner's queue being flushed one-by-one
   in the same order as in the no-redirection case (this almost identical to
   real threads join % one slight difference documented in detail in
   BrowserThreadImpl::StopRedirectionOfThreadID()).

BUG= 653916 ,  672977 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng

TEST=
 A) "out\Release\chrome.exe"
    Runs/shuts down the exact same as before.

 B) "out\Release\chrome.exe --force-fieldtrials=BrowserScheduler/Enabled/ --force-fieldtrial-params=BrowserScheduler.Enabled:Background/3;3;1;0;10000/BackgroundFileIO/3;3;1;0;10000/Foreground/3;3;1;0;10000/ForegroundFileIO/3;3;1;0;10000/RedirectSequencedWorkerPools/true/RedirectNonUINonIOBrowserThreads/true"
    Runs/shuts down smoothly and chrome://tracing confirms that redirected BrowserThreads are running on TaskSchedulerWorkers.

Review-Url: https://codereview.chromium.org/2464233002
Cr-Commit-Position: refs/heads/master@{#439139}
Commit  : 3ee0e445ae07397a077eb3213fc46318af01b43e
Date    : Fri Dec 16 17:46:20 2016


===== TESTED REVISIONS =====
Revision         Mean       Std Dev   N      Good?
chromium@439131  0.0128222  0.747664  40500  good
chromium@439136  0.012947   0.813303  40500  good
chromium@439138  0.0126153  0.743581  40500  good
chromium@439139  0.0137635  0.915485  40500  bad    <--
chromium@439140  0.0138482  0.924326  40500  bad
chromium@439148  0.0139124  0.925514  40500  bad
chromium@439164  0.0139708  0.936498  40500  bad

Bisect job ran on: mac_retina_perf_bisect
Bug ID: 677036

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=indexeddb.endure.testRandomReadsAndWritesWithoutIndex storage.indexeddb_endure_tracing
Test Metric: idb-gets/Action_Test/indexeddb-endure-testRandomReadsAndWritesWithoutIndex
Relative Change: 8.96%

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_retina_perf_bisect/builds/1865
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8992193080095643904


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5860609508245504

| 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 Tests>AutoBisect.  Thank you!

Mostly just looking for understanding why this was a regression.

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

I launched a trace between 439138 and 439139 to see but I also notice a similar regression in ChromiumPerf/chromium-rel-win8-dual/storage.indexeddb_endure_tracing/idb-cursor-iterations/Action_Test/indexeddb-endure-testWalkingMultipleCursors whose range intersects but doesn't include my CL (Chromium Commit Position range: 439165 - 439283), sent a bisect from that bot.

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

Suspected Commit
  Author : scottmg
  Commit : be0cfa14d77c72864a74d2bcc7910b0b31b7eecb
  Date   : Fri Dec 16 21:06:26 2016
  Subject: Make Crashpad start asynchronous, and move back to chrome_elf

Bisect Details
  Configuration: win_8_perf_bisect
  Benchmark    : storage.indexeddb_endure_tracing
  Metric       : idb-cursor-iterations/Action_Test/indexeddb-endure-testWalkingMultipleCursors
  Change       : 12.07% | 0.0208166666667 -> 0.0233293939394

Revision             Result                     N
chromium@439164      0.0208167 +- 0.32103       3300      good
chromium@439179      0.0208664 +- 0.313847      3300      good
chromium@439187      0.0214227 +- 0.315993      3300      good
chromium@439188      0.0231361 +- 0.314909      3300      bad       <--
chromium@439189      0.0229352 +- 0.317311      3300      bad
chromium@439191      0.0227091 +- 0.308095      3300      bad
chromium@439194      0.0232476 +- 0.313517      3300      bad
chromium@439224      0.0228345 +- 0.313493      3300      bad
chromium@439283      0.0233294 +- 0.315247      3300      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=indexeddb.endure.testWalkingMultipleCursors storage.indexeddb_endure_tracing

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

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


| 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 Tests>AutoBisect.  Thank you!

The bisect tool's automerge tool is a bit buggy because it duplicated this issue against 677034 which was already dupped against this one..!

Scott any chance this is caused by your CL?

Looking at the original link

https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg7_fNtgoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgr66lrAsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgr8CirgkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgz9XUtwkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgr4mcvgkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgz7jspwsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgj_nz-QoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgr86HsQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgj4WdswsM

it includes a regression on Android and Mac, but my CL only touches Windows files, so I don't think so.

Right, I'll launch a few more bisects. From different platforms... My CL is pretty big but *should* merely be a no-op refactoring to use generic TaskRunners instead of MessageLoops (when the experiment is disabled, which it is on trunk).