Closed
Bug 243727
Opened 20 years ago
Closed 11 years ago
Consider storing nsIDOMElement* in the getElementById hashtable
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: bzbarsky, Unassigned)
References
()
Details
(Keywords: perf)
See profile data in bug 243726. On that DHTML testcase, 50% of the time spent under GetElementById is spent under QI of the nsIContent to nsIDOMElement. Now granted, the time spent under GetElementById is only 0.2% of the total time taken. So maybe this is a non-issue (and in fact, maybe this is not worth the perf hit of QIing on addition to the table and the cleanliness hit of using a weak ptr to an interface we got via QI -- we're assuming it's not a tearoff). jst, what do you think?
Comment 1•20 years ago
|
||
Hmm, I guess I'm not entirely against it, but I don't really like the idea either. Maybe we could reorganize the order of the IIDs in the QI implementations to get a hit for nsIDOMElement sooner in stead? Or are we just looking at function call overhead here and not that QI on a HTML element is slow?
Reporter | ||
Comment 2•20 years ago
|
||
I don't actually know how to evaluate which it is... I suspect function call overhead is the real problem; optimizing the QI order based on one testcase isn't really right, imo.
Comment 3•20 years ago
|
||
You're absolutely right, but looking at the current QI, we check IIDs in this order (for a generic HTML element): nsIHTMLContent nsIContent nsIStyledContent nsIDOM3Node nsIDOMEventReceiver nsIDOMEventTarget nsIDOM3EventTarget nsISupports nsIDOMNode nsIDOMElement nsIDOMHTMLElement nsIDOMNSHTMLElement nsIDOMElementCSSInlineStyle *element class interfaces* nsIClassInfo *XBL* And while one testcase isn't enough to show much real world data, I'm fairly sure we could improve things in *that* list :-)
Reporter | ||
Comment 4•20 years ago
|
||
hmm... yeah....
Comment 5•17 years ago
|
||
Looking at just the getElementByID performance for HTML at the testcase in the URL: Minefield: 36ms Safari 3.0.0.4: 4ms Opera 9.5b1: 6ms So 6-9x delta. Gotta be some low hanging fruit there :-).
Reporter | ||
Comment 6•17 years ago
|
||
Mike, one question. How does that testcase scale if you go to 10000 nodes instead of 1000? It might actually scale linearly for us, depending on whether all those nodes the testcase creates are ending up in the hashtable (which I suspect they are). And frankly, I doubt this bug is the issue on that testcase, though I would welcome a profile proving me wrong. I'll bet that particular testcase is dominated by xpconnect and js code.
I actually think we should WONTFIX this. While the QI might be a large portion of time spent in getElementById, I've never seen the getElementById time be very large. Usually we're spending way more time actually getting to the getElementById implementation, or on completely separate things. See comment 0 where we're only spending a total of .2% of the time inside getElementById.
Flags: blocking1.9? → blocking1.9-
Comment 8•11 years ago
|
||
I don't think we QI to nsIDOMElement anymore.
You need to log in
before you can comment on or make changes to this bug.
Description
•