Bug 75069 - Make elements with attributes smaller by eliminating the m_element back pointer in NamedNodeMap
Summary: Make elements with attributes smaller by eliminating the m_element back point...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Caio Marcelo de Oliveira Filho
URL:
Keywords:
Depends on: 77233 77674 77800 79963 80169 80188 80522 80541
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-21 23:38 PST by Darin Adler
Modified: 2019-02-06 09:03 PST (History)
10 users (show)

See Also:


Attachments
Patch (32.34 KB, patch)
2012-03-07 20:00 PST, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff
Patch (33.57 KB, patch)
2012-03-08 05:38 PST, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2011-12-21 23:38:12 PST
The back pointer from the heap-allocated vector of attributes to the element is a waste of space. We should move the data from the current NamedNodeMap class into a new heap-allocated class that is independent of the DOM.

The NamedNodeMap class should then just be a class with an Element* in it. The pointer to NamedNodeMap could go into ElementRareData or some other map, and all the operations could be done based on data stored in the element and its data structures. NamedNodeMap would no longer have any data of its own.
Comment 1 Caio Marcelo de Oliveira Filho 2012-01-27 11:54:28 PST
I intend to implement this in steps. The first one is bug 77233 which would add ElementAttributeData, that will be the internal attribute storage. In that patch it is still part of NamedNodeMap, so no real change happen yet. It already contains the style related members that were in NamedNodeMap.

The next step is moving m_attributes to the ElementAttributeData, and the last step would be moving ElementAttributeData to Element and moving NamedNodeMap inside ElementRareData.
Comment 2 Caio Marcelo de Oliveira Filho 2012-03-07 20:00:36 PST
Created attachment 130754 [details]
Patch
Comment 3 Caio Marcelo de Oliveira Filho 2012-03-07 20:04:23 PST
It took more steps than planned because I made them smaller. In the end this made things easier to review. Tomorrow I'm planning to improve the ChangeLog with some empirical data of memory usage.
Comment 4 Ryosuke Niwa 2012-03-07 20:14:43 PST
Comment on attachment 130754 [details]
Patch

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

> Source/WebCore/dom/Element.cpp:203
> +NamedNodeMap* Element::attributes() const

I don't think this function should have const qualifier.
Comment 5 WebKit Review Bot 2012-03-07 21:34:15 PST
Comment on attachment 130754 [details]
Patch

Attachment 130754 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11861141
Comment 6 Caio Marcelo de Oliveira Filho 2012-03-08 05:38:35 PST
Created attachment 130810 [details]
Patch
Comment 7 Caio Marcelo de Oliveira Filho 2012-03-08 05:41:37 PST
Comment on attachment 130754 [details]
Patch

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

>> Source/WebCore/dom/Element.cpp:203
>> +NamedNodeMap* Element::attributes() const
> 
> I don't think this function should have const qualifier.

What about Node::attributes()?
Comment 8 Caio Marcelo de Oliveira Filho 2012-03-08 09:44:28 PST
Committed r110172: <http://trac.webkit.org/changeset/110172>
Comment 9 Julien Chaffraix 2012-03-14 08:59:23 PDT
Comment on attachment 130810 [details]
Patch

Clearing review flag from an old attachment so that it doesn't show on the review queue.
Comment 10 Lucas Forschler 2019-02-06 09:03:27 PST
Mass moving XML DOM bugs to the "DOM" Component.