Issue metadata
Sign in to add a comment
|
3.1%-7.7% regression in speedometer at 417843:417850 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Sep 12 2016
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/9001740813842344432
,
Sep 12 2016
=== Auto-CCing suspected CL author fs@opera.com === Hi fs@opera.com, the bisect results pointed to your CL below as possibly causing a regression. Please have a look at this info and see whether your CL be related. ===== BISECT JOB RESULTS ===== Status: completed ===== SUSPECTED CL(s) ===== Subject : Revert of Make canceling Timers fast. (patchset #10 id:180001 of https://codereview.chromium.org/2319053004/ ) Author : fs Commit description: Reason for revert: Wreaks havoc on the ASAN bots: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20ASAN STDERR: ================================================================= STDERR: ==4==ERROR: AddressSanitizer: use-after-poison on address 0x7ed9a5616190 at pc 0x00000768b0da bp 0x7fff4bbca630 sp 0x7fff4bbca628 STDERR: READ of size 8 at 0x7ed9a5616190 thread T0 (content_shell) STDERR: #0 0x768b0d9 in operator-> third_party/WebKit/Source/wtf/RefPtr.h:68:50 STDERR: #1 0x768b0d9 in revokeAll third_party/WebKit/Source/wtf/WeakPtr.h:146:0 STDERR: #2 0x768b344 in ?? third_party/WebKit/Source/platform/Timer.cpp:124:22 STDERR: #3 0x47a4461 in Run base/callback.h:56:12 STDERR: #4 0x47a4461 in RunTask base/debug/task_annotator.cc:54:0 STDERR: #5 0x79e2381 in ProcessTaskFromWorkQueue third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc:316:19 ... STDERR: AddressSanitizer can not describe address in more detail (wild memory access suspected). STDERR: SUMMARY: AddressSanitizer: use-after-poison Appears to be accessing a "user-poison" area, so maybe a timer in something that was swept? (Wild guess.) Original issue's description: > Make canceling Timers fast. > > base::Closure recently got an IsCancelled method. Taking advantage of > that the scheduler can short circuit a bunch of logic for cancelled > tasks and avoid running them and the rest of the task selection > machinery. > > On the new TimerPerfTest benchmark this makes running 10000 cancelled > tasks aprox 50x - 60x faster (measured on Android and Linux). > > Note this patch reverts many of the changes made in > https://codereview.chromium.org/2258713004 in favor of > WeakPtr based cancellation as favored by the base owners. > > BUG= 605718 , 638542 > > Committed: https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1 > Cr-Commit-Position: refs/heads/master@{#417621} TBR=jochen@chromium.org,haraken@chromium.org,skyostil@chromium.org,alexclarke@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 605718 , 638542 Review-Url: https://codereview.chromium.org/2326313003 Cr-Commit-Position: refs/heads/master@{#417846} Commit : bee7b64b4772d14c9cafc00f60629a89e8f5bf81 Date : Sat Sep 10 21:59:53 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@417845 2556.77 46.5827 5 good chromium@417846 2720.83 66.683 5 bad <-- chromium@417847 2707.56 68.0172 5 bad Bisect job ran on: android_nexus7_perf_bisect Bug ID: 645876 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests speedometer Test Metric: FlightJS-TodoMVC/FlightJS-TodoMVC Relative Change: 5.90% Score: 99.5 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus7_perf_bisect/builds/3304 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9001740813842344432 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5782476557910016 | 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!
,
Sep 12 2016
That commit is a revert. I expect these tests will recover when the CL is relanded. Assigning to (original) CL author.
,
Sep 12 2016
Oh interesting. I did hope some of the metrics might improve with the patch but didn't find any till now. I'm hopeful the ASAN issue can be fixed and the patch relanded.
,
Sep 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/77f183aaf924ea7742df562455db8579607224e0 commit 77f183aaf924ea7742df562455db8579607224e0 Author: alexclarke <alexclarke@chromium.org> Date: Mon Sep 12 13:59:45 2016 Make canceling Timers fast. base::Closure recently got an IsCancelled method. Taking advantage of that the scheduler can short circuit a bunch of logic for cancelled tasks and avoid running them and the rest of the task selection machinery. On the new TimerPerfTest benchmark this makes running 10000 cancelled tasks aprox 50x - 60x faster (measured on Android and Linux). Note this patch reverts many of the changes made in https://codereview.chromium.org/2258713004 in favor of WeakPtr based cancellation as favored by the base owners. NOTE it's possible this might break https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20ASAN I was not able to reproduce the ASAN failure locally however I suspect it's due to the Timer getting swept away which should be fixed by the change in the latest patchset. BUG= 605718 , 638542 , 645876 Committed: https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1 Review-Url: https://codereview.chromium.org/2319053004 Cr-Original-Commit-Position: refs/heads/master@{#417621} Cr-Commit-Position: refs/heads/master@{#417931} [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/Timer.cpp [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/Timer.h [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_perftest.cc [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/time_domain.cc [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/work_queue.cc [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/work_queue.h [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/work_queue_sets_unittest.cc [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/work_queue_unittest.cc [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/child/compositor_worker_scheduler.cc [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/child/web_task_runner_impl.cc [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/child/web_task_runner_impl.h [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/test/fake_web_task_runner.cc [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/test/fake_web_task_runner.h [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/web/BUILD.gn [add] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/web/tests/TimerPerfTest.cpp [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/public/platform/WebTaskRunner.h [modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/public/platform/scheduler/base/task_queue.h
,
Sep 13 2016
I re-landed the patch and the perf regression has gone away. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by nikolaos@chromium.org
, Sep 12 2016