Bug 31308 – NodeLists generated by the same queries should be cached.

• Component: WebCore Misc.
• Status: RESOLVED DUPLICATE

[Jens Alfke]

V8DOMWrapper caches V8 proxy objects for most native DOM objects, but it doesn't do so for NodeLists. This means that every time a JS function like getElementsByTagName is called, a new JS object has to be created for the return value. This has a pretty noticeable effect in the Dromaeo DOM Query test — I used Shark to sample it on OS X and it's spending 1/3 of its time in V8DOMWrapper::convertToV8Object, most of which is spent in instantiateV8Object. There's a lot of GC going on too.

I _think_ it would be straightforward to extend V8DOMWrapper to cache NodeLists; but I'm not familiar with the details of how the caching is done.

[Jens Alfke]

Created attachment 42887 [details]
Shark profile of Dromaeo DOM Query test

[Sam Weinig]

This would actually change functionality as the behavior of == would change.  You need to check what other browsers do before making this change.

[Jens Alfke]

Good point. I'm pretty sure that Safari caches NodeLists, because it scores twice as high on that benchmark and I think the reason why is that it's not constantly creating new objects. But this needs confirmation, of course.

[Sam Weinig]

That is easy to test.  

javascript:alert(document.getElementsByTagName('a') == document.getElementsByTagName('a'))

I get false in Safari.

[Sam Weinig]

It does, however, return true for Firefox.

[Sam Weinig]

If we do decide to do NodeList caching, it should probably be done in the DOM, and not in the wrappers.  That is to say, calling Document::getElementsByTagName('a') twice should return the same C++ NodeList object.

[Oliver Hunt]

Retitling this bug as the caching is in the DOM implementation so (one would hope) it will be entirely JS engine independent.  I believe maciej bought this up on the whatwg list recently, i'm not sure what the outcome was

[Alexey Proskuryakov]

This specific case was fixed in bug 33696.

*** This bug has been marked as a duplicate of bug 33696 ***