[Webkit-unassigned] [Bug 61268] REGRESSION (r45620): Node list caches never deleted

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=61268





--- Comment #5 from Antti Koivisto <koivisto at iki.fi>  2011-05-23 13:37:24 PST ---
(In reply to comment #4)
> (From update of attachment 94468 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=94468&action=review
> 
> How did you test? A while ago Ojan(?) added a harness that could be used for performance tests to prove operations didn’t have an O(n^2) component or something along those lines. Did you check into adding a test using that machinery? I’d like to see at least a manual test.

I checked it fixed the problem in gdb and with a manual performance test case (attached). This is not really O(n^2), more like 2x, but I'll check the harness.

> > Source/WebCore/dom/Node.cpp:1041
> > +void Node::clearNodeListsIfPossible()
> 
> I think this name is confusing. While the function is called “clear node lists”, it does’t actually clear any node lists. In fact it only does work at all when there are no node lists!

This is somewhat consistent with the existing naming (calls RareData::clearNodeLists()). The could certainly use wider renaming.

(It could also use a lot more optimization. The whole thing is pretty primitive.)

> > Source/WebCore/dom/Node.cpp:1047
> > +    NodeRareData* data = rareData();
> > +    if (!data->nodeLists()->isEmpty())
> > +        return;
> 
> Might be nice to inline this part of the function so the common case doesn’t require another level of function call overhead. But maybe not.

True. I'll do that.

> > Source/WebCore/dom/Node.cpp:2540
> > +            m_childNodeListCaches.clear();
> 
> Can this cause another kind of churn as the caches object is deleted and recreated?

I don't think so. Only thing that is actually cached is the last index and the corresponding node. Reconstructing the cache vs. starting with invalidated cache should have little actual cost difference.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.