1831014 - Allow (fix) early firing of timers in TimerThread

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Justin Link]

The purpose of this work is to allow timers to fire slightly ahead of their scheduled time.

This addresses the frequent problem that I have observed that TimerThread gets woken up before it is supposed to (by an average of ~0.4ms on Windows). Without early firing, most of these early wake-ups is a wasted wake-up as we frequently go back to sleep just to wake up again shortly thereafter to actually fire the timers that we had intended to.

There is already functionality in place (in TimerThread) to allow early firing but:

1. it only applies to timers after the first timer in a wake-up
2. the tolerance is calculated at run-time based on half of the minimum resolution of PRIntervalTime. This made sense at one point long ago when we used PRIntervals for sleeping but this is no longer the case. This method of determining the early firing tolerance also means that we could have different tolerances on different platforms.

This change needs to precede https://bugzilla.mozilla.org/show_bug.cgi?id=1826224. If we increase the timer resolution, which potentially increases the frequency at which we wake up, without first addressing this problem we will end up with an avalanche of wasted wake-ups.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Need to be careful with early firing. The spec lets UA to fire setTimeout timers late, but not early, if interpreted strictly.

Comment 2

[Justin Link]

We should definitely be on the watch for any problematic changes in behavior with this change. The catch though is that we are already willing to fire timers early so I'm not really expecting that to happen. (Or, if there were going to be any problems, this would probably just be making them more consistent and reproducible.)

In the future if we decide that we do need to be more strict with JS timeouts then we could add the option for individual timers to not fire early but that would be a much bigger change to the behavior.

Comment 3

[Justin Link]

Attachment: Bug 1831014: Allowed the first timer in a wake-up to fire slightly early as well. r=florian,smaug

• Prior to this change, we allowed any timer except the first to fire early. Being woken up
  slightly before our first timer is due was resulting in a lot of "wasted" wake-ups (where we wake
  up and do nothing other than go back to sleep).
• This change results in waking up TimerThread 10% less frequently in my test case of watching a
  YouTube video.
• Made the allowed early firing tolerance be a constant value rather than calculated at run-time.
• Also cleaned up some includes.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/61d2e733a10c
Allowed the first timer in a wake-up to fire slightly early as well. r=smaug

Comment 5

[Cristian Tuns]

Backed out for causing build bustages in TimerThread.h

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/xpcom/threads/TimerThread.h:26:34: error: base class has incomplete type

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bc2094412009
Allowed the first timer in a wake-up to fire slightly early as well. r=smaug

Comment 7

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/bc2094412009