Bug 89036 - Shrink NodeListsNodeData
Summary: Shrink NodeListsNodeData
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on: 88825 89603
Blocks: 73853
  Show dependency tree / graph
 
Reported: 2012-06-13 14:02 PDT by Ryosuke Niwa
Modified: 2012-06-22 18:56 PDT (History)
6 users (show)

See Also:

Attachments
work in progress (27.55 KB, patch)
2012-06-13 14:03 PDT, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
work in progress 2 (54.15 KB, patch)
2012-06-20 13:40 PDT, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Patch (41.69 KB, patch)
2012-06-20 20:31 PDT, Ryosuke Niwa 		kling: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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]
Patch
Comment 5 Ryosuke Niwa 2012-06-20 20:34:14 PDT 
Comment on attachment 148721 [details]
Patch

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]
Patch

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