Bug 251697

Summary: EventListenerMap's m_entries vector wastes a lot of vector capacity
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: DOMAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, esprehn+autocc, ews-watchlist, kangil.han, rniwa, simon.fraser, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch ews-feeder: commit-queue-

Description Simon Fraser (smfr) 2023-02-03 10:03:10 PST 
Using the patch in bug 186698, and testing on a wikipedia page, we see that EventListenerMap::m_entries is the second-most wasteful site for vector capacity:

Wasted capacity: 65640 bytes (used 6680 of 72320 bytes, utilization: 9.24%) - 1923 allocations
1   0x2fc39b217 WebCore::EventListenerMap::EventListenerMap()
2   0x2fcd93008 WebCore::RenderObject::RenderObject(WebCore::Node&)
3   0x2fcceef18 WebCore::RenderElement::RenderElement(WebCore::Element&, WebCore::RenderStyle&&, unsigned int)
4   0x2fcd69351 WebCore::RenderLayerModelObject::RenderLayerModelObject(WebCore::Element&, WebCore::RenderStyle&&, unsigned int)
5   0x2fccbe2b1 WebCore::RenderBoxModelObject::RenderBoxModelObject(WebCore::Element&, WebCore::RenderStyle&&, unsigned int)
6   0x2fcd287b3 WebCore::RenderInline::RenderInline(WebCore::Element&, WebCore::RenderStyle&&)

Wasted capacity: 7960 bytes (used 1000 of 8960 bytes, utilization: 11.16%) - 278 allocations
1   0x2fc39b217 WebCore::EventListenerMap::EventListenerMap()
2   0x2fcd93008 WebCore::RenderObject::RenderObject(WebCore::Node&)
3   0x2fcceef18 WebCore::RenderElement::RenderElement(WebCore::Element&, WebCore::RenderStyle&&, unsigned int)
4   0x2fcd69351 WebCore::RenderLayerModelObject::RenderLayerModelObject(WebCore::Element&, WebCore::RenderStyle&&, unsigned int)
5   0x2fccbe261 WebCore::RenderBox::RenderBox(WebCore::Element&, WebCore::RenderStyle&&, unsigned int)

Wasted capacity: 3400 bytes (used 440 of 3840 bytes, utilization: 11.46%) - 7 allocations
1   0x2fc39b217 WebCore::EventListenerMap::EventListenerMap()
2   0x2fc5e7fb0 WebCore::CheckboxInputType::CheckboxInputType(WebCore::HTMLInputElement&)
3   0x2fc5e73e8 WTF::Ref<WebCore::InputType, WTF::RawPtrTraits<WebCore::InputType> > WebCore::createInputType<WebCore::CheckboxInputType>(WebCore::HTMLInputElement&)
4   0x2fc5e0772 WebCore::InputType::createIfDifferent(WebCore::HTMLInputElement&, WTF::AtomString const&, WebCore::InputType*)
Comment 1 Radar WebKit Bug Importer 2023-02-03 10:03:46 PST 
<rdar://problem/105010273>
Comment 2 Chris Dumez 2023-02-03 10:04:58 PST 
This is hot code and this vector was tweaked recently for speedometer gains. If we make changes, we should run speedometer to make sure we don't regress.
Comment 3 Simon Fraser (smfr) 2023-02-03 20:30:23 PST 
I ran Speedometer with some instrumentation. The max number of entries in the vector is 25, and the mean is 1.63.

Here's a table of size frequencies (excluding zeros):

Size  Instance count
 1     25751 
 2      3388  
 6      2010  
 3      2010  
 4      1020  
 5        50  
 25       20  
 9        18  
 17        8
Comment 4 Simon Fraser (smfr) 2023-02-03 20:45:37 PST 
Going to perf-test a patch that gets this down to:

Wasted capacity: 11400 bytes (used 6680 of 18080 bytes, utilization: 36.95%) - 1959 allocations
1   0x3343c9df7 WebCore::EventListenerMap::EventListenerMap()
2   0x334dc5218 WebCore::RenderObject::RenderObject(WebCore::Node&)
3   0x334d20f58 WebCore::RenderElement::RenderElement(WebCore::Element&, WebCore::RenderStyle&&, unsigned int)
4   0x334d9b561 WebCore::RenderLayerModelObject::RenderLayerModelObject(WebCore::Element&, WebCore::RenderStyle&&, unsigned int)
5   0x334cf0281 WebCore::RenderBoxModelObject::RenderBoxModelObject(WebCore::Element&, WebCore::RenderStyle&&, unsigned int)
Comment 5 Simon Fraser (smfr) 2023-02-03 20:48:22 PST 
diff --git a/Source/WebCore/dom/EventListenerMap.h b/Source/WebCore/dom/EventListenerMap.h
index 25a4340b6014f9e2bd2f1cd1b19760c86c30a6d7..55e619529189ad8bf80753514035d50be115c49c 100644
--- a/Source/WebCore/dom/EventListenerMap.h
+++ b/Source/WebCore/dom/EventListenerMap.h
@@ -74,7 +74,7 @@ public:
     Lock& lock() { return m_lock; }
 
 private:
-    Vector<std::pair<AtomString, EventListenerVector>> m_entries;
+    Vector<std::pair<AtomString, EventListenerVector>, 0, CrashOnOverflow, 4> m_entries;
     Lock m_lock;
 };
Comment 6 Simon Fraser (smfr) 2023-02-06 11:12:46 PST 
Created attachment 464867 [details]
Patch
Comment 7 EWS 2023-02-06 12:33:45 PST 
Commit message contains (OOPS!) and no reviewer found, blocking PR #None
Comment 8 Simon Fraser (smfr) 2023-02-06 14:13:14 PST 
https://github.com/WebKit/WebKit/pull/9713
Comment 9 EWS 2023-02-06 15:33:47 PST 
Committed 259920@main (ad21476ae97e): <https://commits.webkit.org/259920@main>

Reviewed commits have been landed. Closing PR #9713 and removing active labels.