Bug 92438 – Teach Timer how to handle timer priorities.

• Status: NEW
• Component: DOM

[Kwang Yul Seo]

Currently, HTMLParserScheduler checks if the layout timer is active (a layout is scheduled) and defer its timer. This policy is hardcoded in the parser code. We can handle this automatically by teaching Timer how to handle timer priorities.
    
Added an optional TimerPriority argument to the Timer constructor. When fired, the timer checks if there is other active timer with higher priority. If there is one, the timer defers itself automatically. This is slightly more efficient because the timer can query the other timer's nextFireInterval and defers itself by that amount instead of polling with 0 interval.
    
Priority value can be one of TimerPriority enum values: TimerPriorityNone, TimerPriorityLow, TimerPriorityMedium and TimerPriorityHigh.
TimerPriorityNone is special and ignores the priority. Because TimerPriorityNone is the default value, existing Timer code paths are not affected by this change.
    
Gave the layout timer higher priority than the parser scheduler timer. Then the parser scheduler timer is defered until there is no active layout timer.

[Kwang Yul Seo]

Created attachment 154789 [details]
Patch

[Eric Seidel (no email)]

I'm skeptical that we need this complexity.

[Build Bot]

Comment on attachment 154789 [details]
Patch

Attachment 154789 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13361873

[Kwang Yul Seo]

(In reply to comment #2)
> I'm skeptical that we need this complexity.

Yes, this patch became more complex than expected. Let's listen to other opinions before I work on fixing mac build.

[Sam Weinig]

What is the benefit of this patch?  What else will these priorities be used for.  The added complexity and increased size of Timer objects seems questionable.

[Kwang Yul Seo]

(In reply to comment #5)
> What is the benefit of this patch? What else will these priorities be used for. The added complexity and increased size of Timer objects seems questionable.

Because WebKit is mostly single-threaded, timers are used as a task scheduler. When the shared timer is fired, there are usually many competing timers that need to be fired. Because there is no concept of timer priority in WebKit timers, a random timer among candidates is chosen and fired.

The benefit of timer priority is quite similar to process priority of operating systems. This patch shows one such example. When both the layout timer and the parser scheduler timer are active, we need to pick and fire the layout timer because the layout timer has higher priority. Currently, the parser scheduler timer manually yields when the layout timer is active. This reminds me of cooperative multi-threading used in old days.

I haven't surveyed all the timers in WebKit, but I guess we can improve responsiveness by giving higher priority to user events processing or repaint tasks. I know this sounds quite hypothetical, but I don't have hard evidence supporting this theory :(

Thanks for the comments. I won't be sad even if you give me r-.

[Sam Weinig]

(In reply to comment #6)
> (In reply to comment #5)
> > What is the benefit of this patch? What else will these priorities be used for. The added complexity and increased size of Timer objects seems questionable.
> 
> Because WebKit is mostly single-threaded, timers are used as a task scheduler. When the shared timer is fired, there are usually many competing timers that need to be fired. Because there is no concept of timer priority in WebKit timers, a random timer among candidates is chosen and fired.
> 
> The benefit of timer priority is quite similar to process priority of operating systems. This patch shows one such example. When both the layout timer and the parser scheduler timer are active, we need to pick and fire the layout timer because the layout timer has higher priority. Currently, the parser scheduler timer manually yields when the layout timer is active. This reminds me of cooperative multi-threading used in old days.
> 
> I haven't surveyed all the timers in WebKit, but I guess we can improve responsiveness by giving higher priority to user events processing or repaint tasks. I know this sounds quite hypothetical, but I don't have hard evidence supporting this theory :(
> 
> Thanks for the comments. I won't be sad even if you give me r-.

I certainly like the idea of prioritizing user events, but I would need to see data indicating it sped something up before we went ahead and did this.

[Kwang Yul Seo]

(In reply to comment #7)
> I certainly like the idea of prioritizing user events, but I would need to see data indicating it sped something up before we went ahead and did this.

Okay. I will see if I can show you the benefits with real data. Thanks.

[Eric Seidel (no email)]

Comment on attachment 154789 [details]
Patch

Removing this from the review queue for now.