Bug 109630 - Avoid updating timer heap when nothing changes
Summary: Avoid updating timer heap when nothing changes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-12 16:40 PST by Antti Koivisto
Modified: 2013-02-13 12:42 PST (History)
3 users (show)

See Also:


Attachments
patch (5.24 KB, patch)
2013-02-12 16:57 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch2 (5.88 KB, patch)
2013-02-13 08:21 PST, Antti Koivisto
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2013-02-12 16:40:33 PST
When the fire time of a Timer is changed we remove it from the timer heap and reinsert it. This is pretty slow. Turns out that in over 80% of cases the insertion position is the same as the original position. We can check if anything is actually going to change before doing this work.
Comment 1 Antti Koivisto 2013-02-12 16:57:29 PST
Created attachment 187963 [details]
patch
Comment 2 WebKit Review Bot 2013-02-12 16:59:40 PST
Attachment 187963 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/Timer.cpp', u'Source/WebCore/platform/Timer.h']" exit_code: 1
Source/WebCore/platform/Timer.cpp:354:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/platform/Timer.cpp:356:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Antti Koivisto 2013-02-13 08:21:32 PST
Created attachment 188085 [details]
patch2

With stronger validity assertions, some cleanups.
Comment 4 Antti Koivisto 2013-02-13 11:32:48 PST
http://trac.webkit.org/changeset/142764
Comment 5 Darin Adler 2013-02-13 12:31:39 PST
Comment on attachment 188085 [details]
patch2

View in context: https://bugs.webkit.org/attachment.cgi?id=188085&action=review

> Source/WebCore/platform/Timer.h:87
> -    const Vector<TimerBase*>& timerHeap() const { ASSERT(m_cachedThreadGlobalTimerHeap); return *m_cachedThreadGlobalTimerHeap; }
> -    Vector<TimerBase*>& timerHeap() { ASSERT(m_cachedThreadGlobalTimerHeap); return *m_cachedThreadGlobalTimerHeap; }
> +    Vector<TimerBase*>& timerHeap() const { ASSERT(m_cachedThreadGlobalTimerHeap); return *m_cachedThreadGlobalTimerHeap; }

Why this change?
Comment 6 Antti Koivisto 2013-02-13 12:42:12 PST
(In reply to comment #5)
> Why this change?

Alexey pointed out in Bug 109597 that it didn't make too much sense to have both as the vector items are mutable anyway.