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

Issue 1304293002: Revert of Remove 1 ms rounding up of task delays on Windows (Closed)

Created:
5 years, 4 months ago by pfeldman
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

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}

Patch Set 1 #

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

Messages

Total messages: 6 (0 generated)
pfeldman
Created Revert of Remove 1 ms rounding up of task delays on Windows
5 years, 4 months ago (2015-08-21 17:09:23 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1304293002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1304293002/1
5 years, 4 months ago (2015-08-21 17:10:50 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 4 months ago (2015-08-21 17:12:24 UTC) #3
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/189fc2149ec370a0a42b1bb63e9b49fa579aa1d7 Cr-Commit-Position: refs/heads/master@{#344798}
5 years, 4 months ago (2015-08-21 17:13:29 UTC) #4
brucedawson
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/1311863002/ by brucedawson@chromium.org. ...
5 years, 4 months ago (2015-08-24 17:36:57 UTC) #5
pfeldman
5 years, 4 months ago (2015-08-24 17:38:39 UTC) #6
Message was sent while issue was closed.
Sorry about that, at some point it seemed like the 1ms revert did help Win 10.

Powered by Google App Engine
This is Rietveld 408576698