[webkit-reviews] review denied: [Bug 65734] Timer scheduling leads to poor framerate and responsiveness on timer-heavy pages : [Attachment 103008] Patch

[bugzilla-daemon at webkit.org]

Dmitry Titov <dimich at chromium.org> has denied James Robinson
<jamesr at chromium.org>'s request for review:
Bug 65734: Timer scheduling leads to poor framerate and responsiveness on
timer-heavy pages
https://bugs.webkit.org/show_bug.cgi?id=65734

Attachment 103008: Patch
https://bugs.webkit.org/attachment.cgi?id=103008&action=review

------- Additional Comments from Dmitry Titov <dimich at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=103008&action=review


A general observation: This change has potential to cause regressions or at
least behavioral chnages (because it indeed can change the ordering of some
tasks/timers). Please give it a Chromium try run before landing.
If there is a real scenario that is broken and this is the fix for it then it
makes sense to mention it here simply so the next person who will tweak this
code has the data.
Otherwise, if we are doing it to fix potential FIFO ordering between timers and
tasks, we should add this specific info into the bug and ChangeLog.

r- due to following:

> Source/WebCore/platform/ThreadTimers.cpp:48
> +static const double maxDurationOfFiringTimers = 0; 

If you want to ensure that the loop only fires one timer in Chromium, it's
better to explicitly exit it in case of Chromium instead of using 0 time. It
would be easier to understand what's going on. Could you move the #ifdef into
the firing function itself?