584287 - Make getting id/class/etc faster

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

A bit of thisType magic on nsIDOMHTMLElement goes a long way.  The element.property subtest of dromaeo's dom-attr test (which just tests getting .id on an HTML element) gets about 30% faster with this patch.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Like so

Comment 2

[Peter Van der Beken [:peterv]]

Comment on attachment 462686 [details] [diff] [review]
Like so

This works because all of the HTML elements just forward these getters/setters to nsGenericHTMLElement. Unfortunately I don't think there's an easy way to enforce that.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Well, the only reason they need to forward is that nsGenericHTMLElement itself isn't what implements nsIDOMHTMLElement.  That seems like a bug in itself... ;)

Comment 4

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/6e97f59bcb1c which broke because apparently on Windows quickstubs includes windows.h, which redefines GetClassName to GetClassNameA.  And since we were now calling a non-virtual method, linking failed.

khuey pushed http://hg.mozilla.org/mozilla-central/rev/373d1b393f28 to fix that.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Some combination of this and bug 584293 gave us a 5% win on Dromaeo (DOM) on tinderbox on Linux and Mac.

Comment 6

[Peter Van der Beken [:peterv]]

(In reply to comment #3)
> Well, the only reason they need to forward is that nsGenericHTMLElement itself
> isn't what implements nsIDOMHTMLElement.

That's because most HTML elements implement interfaces that inherit from nsIDOMHTMLElement, so implementing nsIDOMHTMLElement in nsGenericHTMLElement wouldn't help much.

Comment 7

[Boris Zbarsky [:bzbarsky]]

I think we should think long and hard about whether it makes sense for our C++ class inheritance hierarchy (including for the interface classes) to match the interface inheritance hierarchy defined in the idl...  But that's totally out of scope for this bug.