Issue metadata
Sign in to add a comment
|
159ms startup performance regression in base::Time::ActivateHighResolutionTimer() |
||||||||||||||||||||
Issue descriptionData from the UMA Sampling Profiler shows that base::Time::ActivateHighResolutionTimer() regressed by 159ms during GPU process compositor thread startup on 64-bit Chrome on Windows. This occurred in the 69.0.3489.0 Canary release. The responsible CL appears to be https://chromium.googlesource.com/chromium/src.git/+/62973d1b3f85cbc12f3f6e255e8d5dd1f91225b4 Execution profile difference of Canary 69.0.3488.0 vs. 69.0.3489.0: https://uma.googleplex.com/p/chrome/callstacks?sid=3aba0fb9c8f865a26a958c7ea308c043 This also shows up in the periodic data as +0.3% increase.
,
Jul 17
Thanks, revert incoming, diagnosis @ https://bugs.chromium.org/p/chromium/issues/detail?id=854237#c9
,
Jul 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/050feaa75927b7216306c6a12084cb64920f9e78 commit 050feaa75927b7216306c6a12084cb64920f9e78 Author: Gabriel Charette <gab@chromium.org> Date: Wed Jul 18 19:16:12 2018 Revert "[MessageLoop] Disable hi-res timers when not sleeping" This reverts commit 62973d1b3f85cbc12f3f6e255e8d5dd1f91225b4. Reason for revert: results in more churn in base::Time::ActivateHighResolutionTimer() without much benefits (decreased 97th and 98th percentile of Windows.HighResolutionTimerUsage from 100% to 97% and 99% respectively). See crbug.com/863938 Original change's description: > [MessageLoop] Disable hi-res timers when not sleeping > > Time::ActivateHighResolutionTimer(bool activating) is a per-thread vote > for a system-wide side-effect. For a given thread, hi-res timers are > only useful when going to sleep (if it has pending hi-res tasks). > > Deactivating a thread's vote while it's active will prevent other > threads on the system which do not have hi-res requirements from > being forced to use hi-res timers in that period. > > Bug: 854237 > Change-Id: I1393e184cac6c9321d13b92b6077a38c62b1f590 > Reviewed-on: https://chromium-review.googlesource.com/1107110 > Reviewed-by: danakj <danakj@chromium.org> > Reviewed-by: kylechar <kylechar@chromium.org> > Commit-Queue: Gabriel Charette <gab@chromium.org> > Cr-Commit-Position: refs/heads/master@{#574259} TBR=danakj@chromium.org,gab@chromium.org,kylechar@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 854237 , 863938 Change-Id: I7c26646cac8548ae7b02c90e045bc857ae890ce7 Reviewed-on: https://chromium-review.googlesource.com/1140753 Reviewed-by: Gabriel Charette <gab@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#576157} [modify] https://crrev.com/050feaa75927b7216306c6a12084cb64920f9e78/base/message_loop/message_loop.cc [modify] https://crrev.com/050feaa75927b7216306c6a12084cb64920f9e78/base/message_loop/message_loop.h [modify] https://crrev.com/050feaa75927b7216306c6a12084cb64920f9e78/base/message_loop/message_loop_unittest.cc
,
Jul 18
Should be fixed by revert. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by gab@chromium.org
, Jul 17