Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(769)

Issue 407073004: Revert of High resolution timer fix for Windows (Closed)

Created:
6 years, 5 months ago by Kunihiko Sakamoto
Modified:
6 years, 5 months ago
CC:
chromium-reviews, erikwright+watch_chromium.org, sadrul, tonyg
Project:
chromium
Visibility:
Public.

Description

Revert of High resolution timer fix for Windows (https://codereview.chromium.org/395913006/) Reason for revert: This patch seems to make following browser_tests flakey PPAPINaClNewlibTest.Graphics2D_FlushOffscreenUpdate NetInternalsTest.netInternalsHSTSViewAddOverwrite NetInternalsTest.netInternalsHSTSViewAddDelete NetInternalsTest.netInternalsHSTSViewAddTwice http://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%282%29/builds/34734 http://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%281%29/builds/32086 http://build.chromium.org/p/chromium.win/builders/Vista%20Tests%20%281%29/builds/47545 Original issue's description: > This is jamesr@ code I am landing. > > On Windows the message pump code tried to manage the systemwide timer resolution to fire delayed tasks with better than 15ms resolution but it was buggy: > > 1- A short task that was not followed by any other task will leave the systemwide timer pegged to 1ms > > 2- After we decided to crank up the timer we would 'lease' the timer for 1 second, for no good reason. > > Both issues are detrimental to battery power. > > The source of both problems is that we tried to decide with incomplete information. This patch solves that by having 1 bit for each pending task that requires a high resolution timer and a sum of the number of tasks that require high res timers. > > BUG=153139 > TEST=included here, also see the bug for manual testing. > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284625 TBR=jamesr@chromium.org,darin@chromium.org,cpu@chromium.org NOTREECHECKS=true NOTRY=true BUG=153139 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284664

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+101 lines, -87 lines) Patch
M base/message_loop/incoming_task_queue.h View 2 chunks +10 lines, -10 lines 0 comments Download
M base/message_loop/incoming_task_queue.cc View 4 chunks +52 lines, -25 lines 0 comments Download
M base/message_loop/message_loop.h View 3 chunks +5 lines, -9 lines 0 comments Download
M base/message_loop/message_loop.cc View 7 chunks +5 lines, -27 lines 0 comments Download
M base/message_loop/message_loop_unittest.cc View 1 chunk +17 lines, -7 lines 0 comments Download
M base/message_loop/message_pump.h View 1 chunk +1 line, -2 lines 0 comments Download
M base/pending_task.h View 2 chunks +3 lines, -3 lines 0 comments Download
M base/pending_task.cc View 2 chunks +8 lines, -4 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Kunihiko Sakamoto
Created Revert of High resolution timer fix for Windows
6 years, 5 months ago (2014-07-22 11:30:22 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ksakamoto@chromium.org/407073004/1
6 years, 5 months ago (2014-07-22 11:31:15 UTC) #2
commit-bot: I haz the power
Change committed as 284664
6 years, 5 months ago (2014-07-22 11:32:47 UTC) #3
sof
Another data point -- win_blink_rel bots cycled green as a result of this one, http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel
6 years, 5 months ago (2014-07-22 12:37:51 UTC) #4
johnme
6 years, 5 months ago (2014-07-22 13:29:30 UTC) #5
Message was sent while issue was closed.
On 2014/07/22 12:37:51, sof wrote:
> Another data point -- win_blink_rel bots cycled green as a result of this one,
> 
> http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel

And WebKit Win7 also turned red when this patch landed and cycled green when the
revert landed:
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win7

It had been failing 30-100 tests (the number varied on each build) with the
following CHECK:
[3148:3056:0722/044517:1329752:FATAL:web_test_proxy.cc(557)] Check failed:
web_widget_->isAcceleratedCompositingActive().

Powered by Google App Engine
This is Rietveld 408576698