14001 - getElementById() is really slow

Status: VERIFIED FIXED

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

Description

[(not reading, please use [e-mail] instead)]

to quote alecf:  "getElementById() is really slow in gecko's implementation. it
actually walks the _entire_ DOM"

we are using getElementById() in the js that makes up our app:
http://lxr.mozilla.org/seamonkey/search?string=getElementById

if we can speed it up, it would help all over the place.

Comment 1

[Chris Waterson]

But wait, this should only affect you if you are _not_ using XUL documents.
Where are you using vanilla HTML or XML documents?

Comment 2

[Alec Flett]

I didn't realize they used different codepaths for XUL vs. HTML....it looks like
XUL does hash this stuff...

I originally reported this because I noticed a slight speedup on Linux (not
timed, so maybe I only saw it because I was looking for it) when I started
caching the results of getElementById() in JavaScript...

Updating to reflect the status - HTML only.
Here's the offending code:
http://lxr.mozilla.org/seamonkey/source/layout/html/document/src/nsHTMLDocument.cpp#1827

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Triage: setting this as with a TFV of M11, and a priority P2, since it's an HTML
DOM 0 bug.

Comment 4

[Mike Shaver (:shaver emeritus)]

HTML DOM bugs are M11/P2 for Vidur.

Comment 5

[vidur (gone)]

No question about it - we should be hashing the id. I already do that for named
HTML items and just didn't update the first pass implementation of
getElementById().

Comment 6

[ekrock's old account (dead)]

Moving to M12.

Comment 7

[vidur (gone)]

Moving off M12 radar for the time being. One or more might get back once I get a
chance to really look at them.

Comment 8

[vidur (gone)]

In an attempt to get my bug list in order again, marking all the bugs I have
currently as ASSIGNED.

Comment 9

[leger]

Bulk add of "perf" to new keyword field.  This will replace the [PERF] we were
using in the Status Summary field.

Comment 10

[vidur (gone)]

Couldn't get this done in time. It'll be first up for M14.

Comment 11

[Peter Trudelle]

On behalf of PorkJockeys: putting on beta1 radar, per beta criteria priority #2 
- performance. removing extraneous perf tags, cc waterson.

Comment 12

[leger]

Putting on PDT+ radar for beta1.

Comment 13

[Chris Waterson]

this is not critical for beta1. it only affects large, DOM-intensive, *HTML* 
documents. if somebody thinks otherwise, please re-add the beta1 grafitti.

Comment 14

[vidur (gone)]

This bug shouldn't move any further than M16 (says the man who's going to be 
gone when M16 is worked on :-)). Johnny, look at the implementation for 
NamedItem. That implementation does maintain a hash table that it keeps in sync 
with the document. It doesn't create the hash until the document has completed 
loading, though. Unfortunately, most calls to these methods are probably during 
loading. Maybe we can do better...

Comment 15

[Mike]

M16 has been out for a while now, these bugs target milestones need to be 
updated.

Comment 16

[ekrock's old account (dead)]

This bug has been marked "future" because the original netscape engineer working
on this is over-burdened. If you feel this is an error, that you or another
known resource will be working on this bug, or if it blocks your work in some
way -- please attach your concern to the bug for reconsideration.

Comment 17

[Sebastian Späth]

Considering that getElementById() is the recommended method of mozilla&&netscape 
to overcome IE/NS specific html and will be used a lot more in the future it 
shouldn't put be to far out. Adding mozilla1.1 keyword

Comment 18

[Hixie (not reading bugmail)]

Netscape-Manager-Type-People: Do we need this to meet embedding requirements?

Comment 19

[vidur (gone)]

Fixed by [e-mail] prior to XPCDOM landing. The implementation now uses 
a hash table.

Comment 20

[Prashant Desale]

Updating QA contact to Shivakiran Tummala.

Comment 21

[Sivakiran Tummala]

verified it on both IE and netscape ...did not see any considerable difference 
in the amount of time it took for displaying the Elements-by-Id