Issue metadata
Sign in to add a comment
|
regression in service_worker.service_worker_micro_benchmark at 448622:448664 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Feb 9 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8988172144035664624
,
Feb 9 2017
,
Feb 9 2017
Bisect failed: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus6_perf_bisect/builds/2974 Failure reason: the build has failed due to infrastructure failure.
,
Feb 10 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8988078120060749216
,
Feb 10 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8988078081175046240
,
Feb 10 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8988078049038977552
,
Feb 10 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8988078031414251376
,
Feb 10 2017
=== Auto-CCing suspected CL author csharrison@chromium.org === Hi csharrison@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 : csharrison Commit : 59501351d872c1348665ad6dfd23b09396821056 Date : Tue Feb 07 16:02:43 2017 Subject: Get rid of quadratic behavior in ResourceScheduler Bisect Details Configuration: android_nexus5X_perf_bisect Benchmark : service_worker.service_worker_micro_benchmark Metric : concurrent_100_response_99_percentile/concurrent_100_response_99_percentile Change : 4872.18% | 201.119166667 -> 10000.0 Revision Result N chromium@448619 201.119 +- 16.195 6 good chromium@448628 197.896 +- 7.78132 6 good chromium@448633 200.691 +- 8.75401 9 good chromium@448634 8889.99 +- 9418.71 9 bad <-- chromium@448635 10000.0 +- 0.0 6 bad chromium@448637 10000.0 +- 0.0 6 bad chromium@448654 10000.0 +- 0.0 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 service_worker.service_worker_micro_benchmark Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8988078081175046240 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=5217522833424384 | 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!
,
Feb 10 2017
=== BISECT JOB RESULTS === Perf regression found with culprit Suspected Commit Author : csharrison Commit : 59501351d872c1348665ad6dfd23b09396821056 Date : Tue Feb 07 16:02:43 2017 Subject: Get rid of quadratic behavior in ResourceScheduler Bisect Details Configuration: android_nexus5X_perf_bisect Benchmark : service_worker.service_worker_micro_benchmark Metric : concurrent_100_response_50_percentile/concurrent_100_response_50_percentile Change : 130.39% | 193.026666667 -> 444.7125 Revision Result N chromium@448619 193.027 +- 11.3505 6 good chromium@448628 195.753 +- 10.4888 6 good chromium@448633 193.236 +- 12.4735 9 good chromium@448634 413.031 +- 423.559 9 bad <-- chromium@448635 463.849 +- 39.9343 6 bad chromium@448637 455.648 +- 61.4506 6 bad chromium@448654 444.713 +- 61.369 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 service_worker.service_worker_micro_benchmark Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8988078120060749216 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=5864907280482304 | 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!
,
Feb 10 2017
=== BISECT JOB RESULTS === Perf regression found with culprit Suspected Commit Author : csharrison Commit : 59501351d872c1348665ad6dfd23b09396821056 Date : Tue Feb 07 16:02:43 2017 Subject: Get rid of quadratic behavior in ResourceScheduler Bisect Details Configuration: android_nexus5X_perf_bisect Benchmark : service_worker.service_worker_micro_benchmark Metric : concurrent_10_response_50_percentile/concurrent_10_response_50_percentile Change : 126.11% | 18.3908333333 -> 41.5841666667 Revision Result N chromium@448619 18.3908 +- 0.918951 6 good chromium@448628 18.2683 +- 0.710727 6 good chromium@448633 18.4011 +- 1.05964 9 good chromium@448634 36.2107 +- 45.7572 14 bad <-- chromium@448635 37.1883 +- 35.094 9 bad chromium@448637 41.2833 +- 13.5547 6 bad chromium@448654 41.5842 +- 16.6754 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 service_worker.service_worker_micro_benchmark Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8988078031414251376 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=5334145422589952 | 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!
,
Feb 10 2017
=== BISECT JOB RESULTS === NO Perf regression found Bisect Details Configuration: android_nexus6_perf_bisect Benchmark : service_worker.service_worker_micro_benchmark Metric : concurrent_100_response_90_percentile/concurrent_100_response_90_percentile Revision Result N chromium@448621 302.952 +- 186.166 21 good chromium@448664 5291.49 +- 22633.8 21 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 service_worker.service_worker_micro_benchmark Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8988078049038977552 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=5063073125105664 | 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!
,
Feb 10 2017
Submitted perf trace: https://codereview.chromium.org/2689723002
,
Feb 20 2017
+kinuko I think this is due to aggressively rescheduling start on request removal. Let me try being a bit less aggressive by only doing this rescheduling on reprioritization.
,
Feb 21 2017
Note that some of the graphs are now at "10 sec" which indicates timeout rather than just a perf regression. I.e., not enough requests finished to be able to compute the 99 percentile: https://github.com/amiq11/Service-Worker-Performance/blob/e6b3f604674209a30e4cf416a18cb8be3b991abd/many-registration.html#L108
,
Feb 21 2017
Yeah that aligns with what I'm seeing. I think this can be attributed to hammered IO thread, because during these micro-benchmarks we are CPU bound. I am going to land a patch which removes the post task for RemoveRequest, plus adds a couple extra trace probe points which hopefully will clarify the perf bots traces.
,
Feb 22 2017
Hey Kinuko, using crbug to further discussion to allow for attaching resources. I have attached a trace of https://cr.kungfoo.net/loading/1000spacers/ for Mac canary 58.0.3019.0 The reprioritization pattern can be found at time 1,379.235 ms Many messages are processed in a batch, basically doing no-ops. The first one however posts the (relatively expensive) task to load startable requests which starts at 1,405.721 ms Let me know if this answers your question.
,
Feb 23 2017
Thanks Charlie. I had a concern that the 1000spacers example is a bit too artificial, so I ran some local histograms trying to see how realistic usage could look like (though may not be that of representative users). In short, reprioritization request doesn't come very often (much less than RemoveRequest), and when it comes we could possibly batch them with the current code. In my local build & casual browsing on popular sites I saw roughly 50+% of tasks are batched, each has roughly 2 to 10. On the other hand for RemoveRequest I saw 89.1% cases just ran as an independent task (no coalescing). So I assume the decision to only schedule reprioritization one is probably right, but I might still want to have some histograms.
,
Feb 23 2017
Thanks for the added analysis. I can add a histogram to the CL, with intent to remove the logic entirely if the data doesn't look great. I am also not opposed to just removing the code. From your local analysis it feels maybe not worth the complexity.
,
Feb 23 2017
The added logic is simple, as far as we don't have regression & can expect positive effects I'm fine with having it around.
,
Feb 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a17c6984f4b9369e8ce061018d31e4ecf64f7fdf commit a17c6984f4b9369e8ce061018d31e4ecf64f7fdf Author: csharrison <csharrison@chromium.org> Date: Tue Feb 28 02:18:11 2017 Do not schedule to load pending startable request on RemoveRequest This may be causing too many tasks to be posted in some situations. To ease understanding, this patch also adds two trace events to the relevant methods. BUG= 690290 Review-Url: https://codereview.chromium.org/2704243003 Cr-Commit-Position: refs/heads/master@{#453459} [modify] https://crrev.com/a17c6984f4b9369e8ce061018d31e4ecf64f7fdf/content/browser/loader/resource_scheduler.cc [modify] https://crrev.com/a17c6984f4b9369e8ce061018d31e4ecf64f7fdf/tools/metrics/histograms/histograms.xml
,
Mar 7 2017
,
Mar 7 2017
The graphs look to have recovered after comment 21 <https://codereview.chromium.org/2704243003>: https://chromeperf.appspot.com/group_report?bug_id=690290 However some other tests may have regressed: Issue 698990 . That regression so far is just one bot and one test, and isn't as extreme as the original timeouts, so we may be OK here.
,
Mar 7 2017
Issue 698990 looks like it improved with the change to RemoveRequest, so reverting that in comment #21 brought it back to baseline and triggered the regression notice. The tradeoff is probably worth it, since I guess only one microbenchmark improved with that code :D
,
Apr 11 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8982652020079688848
,
Apr 11 2017
=== Auto-CCing suspected CL author csharrison@chromium.org === Hi csharrison@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 : csharrison Commit : 59501351d872c1348665ad6dfd23b09396821056 Date : Tue Feb 07 16:02:43 2017 Subject: Get rid of quadratic behavior in ResourceScheduler Bisect Details Configuration: android_nexus6_perf_bisect Benchmark : service_worker.service_worker_micro_benchmark Metric : concurrent_100_response_90_percentile/concurrent_100_response_90_percentile Change : 3472.20% | 272.405555556 -> 10000.0 Revision Result N chromium@448621 272.406 +- 115.727 9 good chromium@448632 284.667 +- 103.118 9 good chromium@448633 261.478 +- 125.097 6 good chromium@448634 8415.13 +- 8680.68 6 bad <-- chromium@448635 10000.0 +- 0.0 6 bad chromium@448638 8414.75 +- 8682.8 6 bad chromium@448643 8579.01 +- 13023.7 14 bad chromium@448664 10000.0 +- 0.0 9 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 service_worker.service_worker_micro_benchmark Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8982652020079688848 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=5063073125105664 | 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!
,
Jul 14 2017
I think we're going with WontFix for this? The suspected patch landed in M58.
,
Jul 14 2017
SGTM |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by horo@chromium.org
, Feb 9 2017