Bug 106586 – NodeRareData and NodeListsNodeData are never deleted until Node is deleted even if they're no longer used

• Status: RESOLVED
• Resolution: WONTFIX
• Component: DOM

[Ryosuke Niwa]

Right now, we never delete NodeRareData and NodeListsNodeData once they’re created. This results in some pathological memory use when the script traverse the entire document using node.childNodes for example because we’ll end up creating childNodes on all nodes temporarily and that results in every node having NodeRareData and NodeListsNodeData.

[Radar WebKit Bug Importer]

<rdar://problem/12990725>

[Ryosuke Niwa]

Also see the bug 79740.

[Elliott Sprehn]

Note that they _are_ deleted, but not until the Node is. The description and title on this bug make it sound like we're leaking memory when we aren't.

[Ryosuke Niwa]

(In reply to comment #3)
> Note that they _are_ deleted, but not until the Node is. The description and title on this bug make it sound like we're leaking memory when we aren't.

That's a good point. Renamed.

[Elliott Sprehn]

I have some fear this will regress performance by creating a lot of memory churn. For instance doing node.childList repeatedly and letting it drop off it will not just allocate the ChildNodeList like it does now, but the NodeRareData, NodeListsNodeData and the ChildNodeList every time.

[Ryosuke Niwa]

(In reply to comment #5)
> I have some fear this will regress performance by creating a lot of memory churn. For instance doing node.childList repeatedly and letting it drop off it will not just allocate the ChildNodeList like it does now, but the NodeRareData, NodeListsNodeData and the ChildNodeList every time.

That should be considered as a bug/performance issue in GC. A good GC shouldn't be dropping childNodes repeatedly like that.

[Elliott Sprehn]

(In reply to comment #6)
> (In reply to comment #5)
> > I have some fear this will regress performance by creating a lot of memory churn. For instance doing node.childList repeatedly and letting it drop off it will not just allocate the ChildNodeList like it does now, but the NodeRareData, NodeListsNodeData and the ChildNodeList every time.
> 
> That should be considered as a bug/performance issue in GC. A good GC shouldn't be dropping childNodes repeatedly like that.

See the discussion on https://bugs.webkit.org/show_bug.cgi?id=106726

It appears that once you have rare data childNodes is noticeably faster, so dropping rare data when the last node list goes away will probably make repeated traversals of the DOM through childNodes slower. I also question the perf impact from adding all of these subobjects into rare data, or dropping them, since it means lots more little allocations.

I'd really like to see some numbers of real world rare data usage and if this is worth it. It doesn't appear this is going to be much of a memory savings, maybe a few KB, and it has a known perf impact now.

[Ryosuke Niwa]

(In reply to comment #7)
> It appears that once you have rare data childNodes is noticeably faster, so dropping rare data when the last node list goes away will probably make repeated traversals of the DOM through childNodes slower. I also question the perf impact from adding all of these subobjects into rare data, or dropping them, since it means lots more little allocations.

Of course, it'll make things slower because it's a trade off between CPU and memory usage. But did you see any perf. regressions from https://bugs.webkit.org/show_bug.cgi?id=107074 ? If not, I highly doubt that deleting one more object will have a significant perf. impact. And 3% memory use reduction is quite significant.

[Elliott Sprehn]

(In reply to comment #8)
> (In reply to comment #7)
> > It appears that once you have rare data childNodes is noticeably faster, so dropping rare data when the last node list goes away will probably make repeated traversals of the DOM through childNodes slower. I also question the perf impact from adding all of these subobjects into rare data, or dropping them, since it means lots more little allocations.
> 
> Of course, it'll make things slower because it's a trade off between CPU and memory usage. But did you see any perf. regressions from https://bugs.webkit.org/show_bug.cgi?id=107074 ? If not, I highly doubt that deleting one more object will have a significant perf. impact. And 3% memory use reduction is quite significant.

Can you explain the 3% improvement? When I looked into this there were about 3000 elements in a large sized web app, if at most 10% have rare data and rare data is 160 bytes (that was months ago, it's smaller now) that's only 47KB. That seems a far cry from 3% of the memory usage of a typical web app.

Can you post some real numbers of rare data usage on pages and how much memory this really saves? It would make sense to instrument rare data usage before we start these kinds of optimizations.

[Maciej Stachowiak]

It seems like it would be easier to evaluate this with a patch that can be read and tested, plus some test data, than in the abstract.