Bug 89036

Summary: Shrink NodeListsNodeData
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Severity: Normal CC: adamk, arko, darin, kling, koivisto, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 88825, 89603    
Bug Blocks: 73853    
Description Flags
work in progress
work in progress 2
Patch kling: review+

Description Ryosuke Niwa 2012-06-13 14:02:09 PDT
It's wasteful for NodeListsNodeData to have HashMap, HashSet, of all node list types.
This will become a serious problem when we try to make HTMLCollection behave more like DynamicNodeList.
Comment 1 Ryosuke Niwa 2012-06-13 14:03:21 PDT
Created attachment 147405 [details]
work in progress
Comment 2 Ryosuke Niwa 2012-06-13 14:05:08 PDT
Another thing I want to do this in patch is to move more code from Node/NodeListsNodeData to subclasses of DynamicNodeList so that we don't have modify so much code to add a new type of DynamicNodeList.
Comment 3 Ryosuke Niwa 2012-06-20 13:40:13 PDT
Created attachment 148646 [details]
work in progress 2

Here, I've replaced HashMaps by a Vector of DynamicNodeListCache objects. But I think this incurs too much overhead and is somewhat over-engineered. I'll take some goodies that came out of this patch and fix in separate bugs.
Comment 4 Ryosuke Niwa 2012-06-20 20:31:14 PDT
Created attachment 148721 [details]
Comment 5 Ryosuke Niwa 2012-06-20 20:34:14 PDT
Comment on attachment 148721 [details]

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

> Source/WebCore/dom/NodeRareData.h:57
> +    template<typename T>
> +    PassRefPtr<T> addCacheWithAtomicName(Node* node, DynamicNodeList::NodeListType listType, const AtomicString& name)

Should we use some sort of traits to avoid manually passing list type here?
Comment 6 Andreas Kling 2012-06-21 16:22:19 PDT
Comment on attachment 148721 [details]

Comment 7 Ryosuke Niwa 2012-06-21 16:38:01 PDT
Committed r120979: <http://trac.webkit.org/changeset/120979>