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

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

Description Boris Zbarsky [:bz] 2010-08-03 22:38:00 PDT
A bit of thisType magic on nsIDOMHTMLElement goes a long way.  The 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] 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] 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] 2010-08-05 19:59:19 PDT
Pushed 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 to fix that.
Comment 5 Boris Zbarsky [:bz] 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] 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.