[Webkit-unassigned] [Bug 28455] ThreadTimer: avoid blocking UI when too many timers ready to fire

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 19 11:24:37 PDT 2009


https://bugs.webkit.org/show_bug.cgi?id=28455


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #35123|review?                     |review-
               Flag|                            |




--- Comment #3 from Darin Adler <darin at apple.com>  2009-08-19 11:24:35 PDT ---
(From update of attachment 35123)
The concept of the code is here OK, but the conditionals need to be done
better.

I think you should have the TimerBeingFired struct all the time, and just make
the fire time member be conditional.

Normally we do not use the m_ prefix for struct members, only for classes where
they are private.

> +    for (Vector<TimerBeingFired>::const_iterator i = firingTimers.begin(); i != firingTimers.end(); ++i) {
> +        TimerBase* timer = i->m_timer;

>      for (size_t i = 0; i != size; ++i) {
>          TimerBase* timer = firingTimers[i];

We do not normally use iterators to go through a Vector. Instead we just use
indexing as you would with an array. That's what we did in the existing code,
and there's no reason to change that just because the timers are a struct.

> +        // 30ms
> +        if (currentTime() - startTime > 0.03) {

The time interval should be a constant at the top of the file, and the comment
should explain why that particular interval was chosen.

> +            for (++i; i != firingTimers.end(); ++i) {
> +                timer = i->m_timer;
> +                if (timer->m_nextFireTime == 0 && m_timersReadyToFire.contains(timer))
> +                    timer->setNextFireTime(i->m_fireTime);
> +            }

I wrote the class, and yet I do not understand this code. It may be correct.
There needs to be a comment explaining why this is the right thing to do.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list