Issue metadata
Sign in to add a comment
|
19.3% regression in performance_browser_tests at 489680:489795 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Aug 3 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8972297826296129840
,
Aug 3 2017
=== 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 === Perf regression found with culprit Suspected Commit Author : Gabriel Charette Commit : aa061e550891fab4b55e75d85955e27eed25bf94 Date : Wed Jul 26 20:18:46 2017 Subject: SetupMetrics() in PreCreateThreads() to fix data race in ScopedTaskTracker initialization. Bisect Details Configuration: win_x64_perf_bisect Benchmark : performance_browser_tests Metric : TabCapturePerformance_comp_gpu_webrtc/Capture Change : 26.42% | 19.8044923333 -> 25.0367108333 Revision Result N chromium@489679 19.8045 +- 1.54002 6 good chromium@489737 19.5597 +- 1.0822 6 good chromium@489738 20.1845 +- 2.29619 6 good chromium@489739 24.3906 +- 3.4969 6 bad <-- chromium@489741 25.4363 +- 1.7512 6 bad chromium@489745 25.0487 +- 2.46549 6 bad chromium@489752 25.4734 +- 2.15376 6 bad chromium@489766 24.3128 +- 2.61744 6 bad chromium@489795 25.0367 +- 3.19418 6 bad To Run This Test .\src\out\Release_x64\performance_browser_tests.exe --test-launcher-print-test-stdio=always --enable-gpu More information on addressing performance regressions: http://g.co/ChromePerformanceRegressions Debug information about this bisect: https://chromeperf.appspot.com/buildbucket_job_status/8972297826296129840 For feedback, file a bug with component Speed>Bisection
,
Aug 3 2017
Alexei, any idea why moving SetupMetrics() earlier would have regressed performance_browser_tests TabCapturePerformance_comp_gpu_webrtc/Capture ?
,
Aug 7 2017
I do not. +cc owners of the test in question. Looking at the benchmark code: https://cs.corp.google.com/clankium/src/chrome/browser/extensions/api/tab_capture/tab_capture_performancetest.cc?q=TabCapturePerformance&dr=CSs&l=58 I think it's trying to find durations of "Capture" trace events, which are here: https://cs.chromium.org/chromium/src/media/capture/content/thread_safe_capture_oracle.cc?rcl=f768b8dab842bdde2485d0c65c070ddbf9dff3ae&l=134 I don't see how that would be affected by metrics service. Obviously the CL in question moves around MetricsService::InitializeMetricsRecordingState() which does do some work - but I was trying to look to see if it records a timestamp somewhere that could be used by the benchmark, but didn't see anything like that. It does post tasks and such - and I wonder if it could interact with tab capture tasks timing indirectly? Interestingly, the tab capture perf tests has a constant about events to trim: // Number of events to trim from the begining and end. These events don't // contribute anything toward stable measurements: A brief moment of startup // "jank" is acceptable, and shutdown may result in missing events (since // render widget draws may stop before capture stops). constexpr size_t kTrimEvents = 24; I wonder if the startup jank it's referring to could be at play here - i.e. moving things around at startup could have wiggled things around...
,
Aug 7 2017
For reference, here's the sampling profiler delta between the two versions where that change was landed: https://uma.googleplex.com/callstacks?sid=0b0c82e3d16063d8e9228fb0eb3162f7
,
Aug 7 2017
Hmm...Looking at the graphs, I wonder if this a fluke. FWIW, there's this evidence: 1. All other chromium-rel-win7-x64-dual/performance_browser_tests/*/Capture tests seem unaffected. 2. There was an "unexplained improvement" at r485508, whereby at r489739 simply brought it back to prior levels. 3. Post r489739, performance slightly improved and otherwise seems stable. I also don't see how moving metrics initialization could have an impact on this perf test. Even any unintentional startup latency should not have had an effect since these tests ignore the tracing events during the few seconds of the run to abate that exact issue. So, I think it's safe to WontFix this. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Aug 3 2017