Last Comment Bug 584287 - Make getting id/class/etc faster
: Make getting id/class/etc faster
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86 Mac OS X
: P1 normal (vote)
: mozilla2.0b4
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
:
Mentors:
Depends on:
Blocks: domperf
  Show dependency treegraph
 
Reported: 2010-08-03 22:38 PDT by Boris Zbarsky [:bz] (still a bit busy)
Modified: 2010-08-06 07:57 PDT (History)
2 users (show)
bzbarsky: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta4+


Attachments
Like so (1.67 KB, patch)
2010-08-03 22:40 PDT, Boris Zbarsky [:bz] (still a bit busy)
peterv: review+
benjamin: approval2.0+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] (still a bit busy) 2010-08-03 22:38:00 PDT
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 [:bz] (still a bit busy) 2010-08-03 22:40:39 PDT
Created attachment 462686 [details] [diff] [review]
Like so
Comment 2 Peter Van der Beken [:peterv] 2010-08-04 01:35:07 PDT
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 [:bz] (still a bit busy) 2010-08-04 01:36:17 PDT
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 [:bz] (still a bit busy) 2010-08-05 19:59:19 PDT
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 [:bz] (still a bit busy) 2010-08-06 06:14:07 PDT
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] 2010-08-06 06:37:30 PDT
(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 [:bz] (still a bit busy) 2010-08-06 07:57:01 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.