476253 - Dynamically pick next polling time for IdleService (instead of polling every 5 sec)

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

[Ed Lee :Mardak]

We can save on unnecessary polls by looking at the expected next here->idle transition time instead of blindly polling every 5 seconds.

(FYI, I'm interested in having the nsIIdleService send an observerService notification, once-a-day-on-idle, which will send that notification when the user goes idle, but will send at most one of these in a 24 hour period. This will require a timer to be available all the time, so I figured it would be good to make the polling "interval" smarter.)

Comment 1

[Ed Lee :Mardak]

Attachment: v1

Keep the minimum polling time at 5 seconds, but if we don't need to poll frequently, just check once every 5 minutes. (Actually we would only really need to check when the next here->idle transition might occur, but this is in preparation for an always-around timer for the once-a-day-on-idle notification.)

Also store the required idle time in milliseconds to not need to calculate * 1000 every time we check away state.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 359887 [details] [diff] [review]
v1

-        {
             mTimer->Cancel();
-            mTimer = nsnull;
-        }

Please don't remove the {} around single statements. That's our coding style last I checked.

Comment 3

[:Gijs (he/him)]

This is going to suck for IM and such applications, where the idle time is actually directly reflected by automatic UI changes or server interaction. I mean, I wouldn't want my instant messaging program to wait 5 minutes before realizing I was back at the machine. :-\

So I'm not sure this is the right thing to do. I realize I don't have a say in this, but I'm a bit confused what your arguments are as to why this is even possible (rather than why it'd be a perf improvement - obviously we could get an even bigger perf improvement if we only polled once a day, but obviously that would also defeat the point).

Comment 4

[Ed Lee :Mardak]

When expecting an idle->back transition, it'll poll at the current 5 seconds. Only if all listeners are waiting for back->idle transition will it poll at 5 minute intervals (or somewhere between 5sec and 5min, e.g., 3 minutes if the next idle listener is looking for idle in 3 more minutes.)

Comment 5

[:Gijs (he/him)]

(In reply to comment #4)
> When expecting an idle->back transition, it'll poll at the current 5 seconds.
> Only if all listeners are waiting for back->idle transition will it poll at 5
> minute intervals (or somewhere between 5sec and 5min, e.g., 3 minutes if the
> next idle listener is looking for idle in 3 more minutes.)

Ah, sorry, that's me not reading the patch (I just woke up). Apologies, and yes, that sounds fine, thanks!

Comment 6

[Ed Lee :Mardak]

Attachment: v1.1

(In reply to comment #2)
> Please don't remove the {} around single statements.
Restored the {}s and added them to the new code.

Comment 7

[Ed Lee :Mardak]

http://hg.mozilla.org/mozilla-central/rev/72d2e6acd6a1

Comment 8

[Ed Lee :Mardak]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ac1562d96d16