Issue metadata
Sign in to add a comment
|
20.2% regression in blink_perf.bindings at 474890:474904 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jun 12 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8976996886640299520
,
Jun 13 2017
=== Auto-CCing suspected CL author bmcquade@chromium.org === Hi bmcquade@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 : bmcquade Commit : 42b72063edb66bfd015bd9dea116c3e3e937d3eb Date : Fri May 26 03:14:11 2017 Subject: Provide WebContents::CreateParams to tab helpers. Bisect Details Configuration: android_webview_nexus6_aosp_perf_bisect Benchmark : blink_perf.bindings Metric : get-elements-by-tag-name/get-elements-by-tag-name Change : 22.59% | 133.078095065 -> 103.014572285 Revision Result N chromium@474000 133.078 +- 5.08569 6 good chromium@474452 135.857 +- 5.73003 6 good chromium@474678 130.101 +- 4.53407 5 good chromium@474791 142.163 +- 4.65086 6 good chromium@474848 132.121 +- 5.40164 6 good chromium@474876 126.186 +- 3.26932 6 good chromium@474890 128.163 +- 3.25432 6 good chromium@474894 127.676 +- 3.56766 6 good chromium@474895 106.394 +- 4.99329 6 bad <-- chromium@474896 105.304 +- 8.77445 6 bad chromium@474897 105.302 +- 6.33683 6 bad chromium@474904 103.015 +- 3.32356 6 bad To Run This Test src/tools/perf/run_benchmark -v --browser=android-webview --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests blink_perf.bindings Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8976996886640299520 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=6504581887950848 | 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!
,
Jun 13 2017
This seems like a bad bisect. This change simply passes a new parameter to a few methods, and does nothing interesting / complex with that param.
,
Jun 13 2017
Looking at the test a bit more, it just calls getElementsByTagName in a loop. My change only affects things when a WebContents is created, and even then, its effect is minimal, so I'm very skeptical that this bisect is correct. I'd like to re-run the bisect to double check, but I can't figure out where in the perf dash UI I can do this. :-( Maybe the UI could provide a button for this, as it seems like a very common action users may want to take. In the meantime I'm unassigning this from myself.
,
Jun 16 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8976671876171177120
,
Jun 16 2017
Unassigning with an explanation is the right thing to do, thanks! I reran the bisect. For future reference, you can click the exclamation point in the graph and click the "Bisect" button in the tooltip that comes up to re-run.
,
Jun 17 2017
=== BISECT JOB RESULTS === Perf regression found with culprit Suspected Commit Author : bmcquade Commit : 42b72063edb66bfd015bd9dea116c3e3e937d3eb Date : Fri May 26 03:14:11 2017 Subject: Provide WebContents::CreateParams to tab helpers. Bisect Details Configuration: android_webview_nexus6_aosp_perf_bisect Benchmark : blink_perf.bindings Metric : get-elements-by-tag-name/get-elements-by-tag-name Change : 19.40% | 127.190133976 -> 102.510479811 Revision Result N chromium@474889 127.19 +- 1.03951 6 good chromium@474893 127.795 +- 2.31098 6 good chromium@474894 127.735 +- 2.38563 6 good chromium@474895 105.92 +- 5.73755 6 bad <-- chromium@474897 103.977 +- 5.11399 6 bad chromium@474904 102.51 +- 1.08246 6 bad To Run This Test src/tools/perf/run_benchmark -v --browser=android-webview --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests blink_perf.bindings Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8976671876171177120 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=6232906046898176 | 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!
,
Jun 17 2017
This is quite puzzling - looks like it picked out my change again. I can try to repro this locally next week with and without the patch applied, to see if it repros. If so, I'll debug to see if I can figure out how my change could cause this. Still quite skeptical.
,
Jun 17 2017
+jbroman, owner of blink_perf.bindings benchmark, in case he has any ideas for how bmcquade should investigate.
,
Jun 17 2017
jbroman, if you're willing to take a scan of the blamed change and see if you can see how this might cause this regression, I'd really appreciate it. The 2 look totally unrelated to me, but it's possible I'm missing something. https://codereview.chromium.org/2894973002 Thanks!
,
Jul 27 2017
Explictly assigning. A CL you landed tripped one of the speed metrics we measure in the lab. If this is the first time this has happened to one of your CLs, or if it's been a while, please read: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/addressing_performance_regressions.md We're looking for one of the following: 1. Justification via explanation 2. Plan to revert or fix 3. Angry rage throwing of equipment at my head Just be aware that I'm trained in trumpet playing and First Aid and am not afraid to use it. Note: This was a bulk edit message and not very personal.
,
Jul 27 2017
This change was reverted for other reasons. We think the bisect result is probably wrong, but in any case, the change was reverted, so I'm closing this out. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by toyoshim@chromium.org
, May 30 2017