Issue metadata
Sign in to add a comment
|
18.2% regression in blink_perf.bindings at 470680:470770 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
May 18 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8979252435819307024
,
May 18 2017
=== Auto-CCing suspected CL author tapted@chromium.org === Hi tapted@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 : tapted Commit : 335d78a0e0299bda3465cb7e303b79e043f680da Date : Thu May 11 01:19:30 2017 Subject: Mac[Views]: Make native menus more responsive by pumping private runloop modes. Bisect Details Configuration: mac_pro_perf_bisect Benchmark : blink_perf.bindings Metric : post-message/post-message Change : 15.94% | 41.8826666667 -> 48.5578333333 Revision Result N chromium@470679 41.8827 +- 3.36277 6 good chromium@470725 41.9442 +- 3.13283 6 good chromium@470748 41.8755 +- 2.24871 6 good chromium@470759 42.7934 +- 6.49976 9 good chromium@470765 41.3965 +- 1.76488 6 good chromium@470768 41.5225 +- 2.67095 6 good chromium@470769 47.4807 +- 4.37355 6 bad <-- chromium@470770 48.5578 +- 6.48938 6 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 blink_perf.bindings Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8979252435819307024 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=5625762968764416 | 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!
,
May 18 2017
+cc some folks for any tips (as often seems to be the case with these perf bugs) I'm struggling to find what the test is actually doing, or how to run the test to see what code from that CL is involved. "blink_perf.bindings" isn't a great key to search for. I think it's related to https://cs.chromium.org/chromium/src/third_party/WebKit/PerformanceTests/Bindings/ that has a bunch of html files with javascript. I'm guessing they are all run? Is there a way to see whether a _particular_ one of these html has blown out its runtime? It's hard to see how https://codereview.chromium.org/2852233002 plays a role, but perhaps Robert's observation at https://codereview.chromium.org/2852233002/diff/350001/base/message_loop/message_pump_mac.mm#newcode44 is related. i.e. there's an overhead for adding these runloop observers. Perhaps this perf test makes sufficient tasks that it gets noticed. If we can add those observers just to the main UI thread perhaps we succeed in cutting out effects on javascript. Worth a try implementing the suggestion there I guess. I'll do that.
,
May 19 2017
I'm not an expert on the tests, but some details from the bisect comment: It posted the command you can run to run the test: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests blink_perf.bindings It listed the metric as post-message/post-message. That's this file in the directory you linked (which is correct): https://cs.chromium.org/chromium/src/third_party/WebKit/PerformanceTests/Bindings/post-message.html
,
May 19 2017
Thanks for the pointers! Yeah - I didn't link up that Metric being an actual test case. CL that potentially fixes this: https://codereview.chromium.org/2889293002 running $ tools/perf/run_benchmark try mac-pro blink_perf.bindings ("chromium-rel-mac11-pro" wasn't an option, so mac-pro it is)
,
May 19 2017
Seems to be the ticket: https://console.developers.google.com/m/cloudstorage/b/chromium-telemetry/o/html-results/results-2017-05-18_19-45-51 has post-message going 50.101 ms -> 39.631 ms
,
May 19 2017
There are definitely costs (in terms of mutexes) to adding CFRunLoop work sources to more modes. The renderers run a NSRunLoop-backed main thread, so if we limit the additional modes to just the NSApplication-backed MessagePump, this should hopefully be fixed.
,
May 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/80575da0e067c7c941edfd106287dd58ed578f8f commit 80575da0e067c7c941edfd106287dd58ed578f8f Author: tapted <tapted@chromium.org> Date: Sat May 20 00:08:14 2017 Filter run loop modes in message_pump_mac.mm There's a non-trivial overhead for the machinery to run posted tasks in a given run loop mode. Only observe modes known to be run for a particular thread. BUG=640466, 724076 Review-Url: https://codereview.chromium.org/2889293002 Cr-Commit-Position: refs/heads/master@{#473391} [modify] https://crrev.com/80575da0e067c7c941edfd106287dd58ed578f8f/base/message_loop/message_pump_mac.h [modify] https://crrev.com/80575da0e067c7c941edfd106287dd58ed578f8f/base/message_loop/message_pump_mac.mm
,
May 21 2017
Looks recovered |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by tdres...@chromium.org
, May 18 2017