Bug 120611 – Cut down on double hashing and code needlessly using hash table iterators

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Darin Adler]

Cut down on double hashing and code needlessly using hash table iterators

[Darin Adler]

Created attachment 210314 [details]
Patch

[Andreas Kling]

Comment on attachment 210314 [details]
Patch

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

These all look good.

> Source/WebCore/bridge/NP_jsobject.cpp:74
> +        ASSERT(m_map.contains(rootObject));
> +        m_map.remove(rootObject);

It would be cool if we could think of a better pattern that would let us avoid the contains() in debug builds.

> Source/WebCore/inspector/InspectorProfilerAgent.cpp:318
>      if (type == CPUProfileType) {
> -        if (m_profiles.contains(uid))
> -            m_profiles.remove(uid);
> +        m_profiles.remove(uid);
>      } else if (type == HeapProfileType) {
> -        if (m_snapshots.contains(uid))
> -            m_snapshots.remove(uid);
> +        m_snapshots.remove(uid);
>      }

Looks like we don't need curly braces here anymore.

[Andreas Kling]

There's a bug in the DocumentEventQueue change:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x0000000109fe854c WTF::RefPtr<WebCore::EventTarget>::get() const + 12 (RefPtr.h:64)
1   com.apple.WebCore             	0x0000000109fe6d1c WebCore::Event::target() const + 28 (Event.h:99)
2   com.apple.WebCore             	0x000000010a468660 WebCore::DocumentEventQueue::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) + 32 (DocumentEventQueue.cpp:144)
3   com.apple.WebCore             	0x000000010a46860d WebCore::DocumentEventQueue::pendingEventTimerFired() + 525 (DocumentEventQueue.cpp:139)
4   com.apple.WebCore             	0x000000010a46c212 WebCore::DocumentEventQueueTimer::fired() + 98 (DocumentEventQueue.cpp:57)
5   com.apple.WebCore             	0x000000010b86d4b3 WebCore::ThreadTimers::sharedTimerFiredInternal() + 307 (ThreadTimers.cpp:132)
6   com.apple.WebCore             	0x000000010b86d1c9 WebCore::ThreadTimers::sharedTimerFired() + 25 (ThreadTimers.cpp:106)
7   com.apple.WebCore             	0x000000010b606313 WebCore::timerFired(__CFRunLoopTimer*, void*) + 67 (SharedTimerMac.mm:134)

[Andreas Kling]

Comment on attachment 210314 [details]
Patch

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

> Source/WebCore/dom/DocumentEventQueue.cpp:138
> -    while (!m_queuedEvents.isEmpty()) {
> -        ListHashSet<RefPtr<Event>, 16>::iterator iter = m_queuedEvents.begin();
> -        RefPtr<Event> event = *iter;
> -        m_queuedEvents.remove(iter);
> -        if (!event)
> -            break;
> -        dispatchEvent(event.get());
> -    }
> +    while (!m_queuedEvents.isEmpty())
> +        dispatchEvent(m_queuedEvents.takeFirst().get());

Before this code runs, a null Event pointer is inserted in the queue, marking the place to stop.

[Darin Adler]

I’ll land this without any DocumentEventQueue changes.

[Darin Adler]

Comment on attachment 210314 [details]
Patch

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

>> Source/WebCore/dom/DocumentEventQueue.cpp:138
>> +        dispatchEvent(m_queuedEvents.takeFirst().get());
> 
> Before this code runs, a null Event pointer is inserted in the queue, marking the place to stop.

So funny that I missed that. So the while loop is nuts! It says while (!empty) but it’s never going to be empty.

[Darin Adler]

Committed r154967: <http://trac.webkit.org/changeset/154967>

[Simon Fraser (smfr)]

This cause terrible window resize performance: bug 120653.