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

Issue 1311863002: Reland of move 1 ms rounding up of task delays on Windows (Closed)

Created:
5 years, 4 months ago by brucedawson
Modified:
5 years, 4 months ago
CC:
chromium-reviews, sadrul, ananta
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Reland of move 1 ms rounding up of task delays on Windows (patchset #1 id:1 of https://codereview.chromium.org/1304293002/ ) Reason for revert: The original change was reverted in error. The Webkit Win10 builder has been hitting multiple failures for a long time. Neither my change nor the revert affected this. And, the linked build failures (builds/71) were before my change landed. By reverting the revert of my undo of a previous change we are putting the code back to its original state (see original description) Original issue's description: > Revert of Remove 1 ms rounding up of task delays on Windows (patchset #1 id:1 of https://codereview.chromium.org/1305873002/ ) > > Reason for revert: > Figuring out if this could cause Win10 failures: http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win10/builds/71 > > Original issue's description: > > Remove 1 ms rounding up of task delays on Windows > > > > Prior to a couple of attempts at fixing bug 487724 the message pump > > would sometimes end up spinning for up to a ms when waiting for a > > message's time to arrive. This was unintentional and non-obvious. > > > > The eventual fix was to round up the delay time to the next ms. > > Unfortunately this caused a regression in smoothness.top_25_smooth > > which has not been resolved (investigation was delayed due to some > > confusion about the results). Therefore this change reverts both > > fix attempts in order to leave time for a proper investigation and > > any necessary fixes. > > > > It is worth mentioning that the delay introduced by rounding up to 1 > > ms is actually worse than expected on some operating systems. On Windows > > 7 a call to Sleep(1) will, if the timer frequency is raised, return > > within a ms. On Windows 8.1 it will often take two ms to return, which > > increases the delay. If the timer frequency is at its default interval > > of 15.625 ms then the delay can be longer. > > > > BUG=487724, 497536 > > > > Committed: https://crrev.com/2b1bf190a079709a1a6a769bf86e782423d23121 > > Cr-Commit-Position: refs/heads/master@{#344688} > > TBR=rvargas@chromium.org,brucedawson@chromium.org > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=487724, 497536 > > Committed: https://crrev.com/189fc2149ec370a0a42b1bb63e9b49fa579aa1d7 > Cr-Commit-Position: refs/heads/master@{#344798} TBR=rvargas@chromium.org,pfeldman@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=487724, 497536 Committed: https://crrev.com/3e74c7c6e2c4d3ca1e9876d81d95c66b7be512b5 Cr-Commit-Position: refs/heads/master@{#345110}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -9 lines) Patch
M base/message_loop/message_pump_default.cc View 2 chunks +0 lines, -9 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
brucedawson
Created Reland of move 1 ms rounding up of task delays on Windows
5 years, 4 months ago (2015-08-24 17:36:57 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1311863002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1311863002/1
5 years, 4 months ago (2015-08-24 17:37:19 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 4 months ago (2015-08-24 17:38:15 UTC) #3
commit-bot: I haz the power
5 years, 4 months ago (2015-08-24 17:39:12 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/3e74c7c6e2c4d3ca1e9876d81d95c66b7be512b5
Cr-Commit-Position: refs/heads/master@{#345110}

Powered by Google App Engine
This is Rietveld 408576698