Bug 14977 - Hixie's DOM Core performance test shows insert >10x slower than append
: Hixie's DOM Core performance test shows insert >10x slower than append
Status: RESOLVED FIXED
: WebKit
HTML DOM
: 523.x (Safari 3)
: Macintosh Mac OS X 10.4
: P3 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2007-08-15 13:38 PST by
Modified: 2007-11-22 22:44 PST (History)


Attachments
first cut at a fix (7.35 KB, patch)
2007-08-15 13:44 PST, Darin Adler
no flags Review Patch | Details | Formatted Diff | Diff
first cut at a fix (uploaded wrong the first time) (11.23 KB, patch)
2007-08-15 13:46 PST, Darin Adler
oliver: review-
Review Patch | Details | Formatted Diff | Diff
proposed fix (3.81 KB, patch)
2007-11-22 10:16 PST, Alexey Proskuryakov
mjs: review+
Review Patch | Details | Formatted Diff | Diff


Note

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


Description From 2007-08-15 13:38:10 PST
I noticed that the performance test indicated that insert was 10x slower than append.

Then I figured out why. Unnecessary churn in the NodeList code. I have a patch that fixes it, but it could possibly slow down other things so it's not quite ready for review.

I didn't make a regression test yet.
------- Comment #1 From 2007-08-15 13:44:27 PST -------
Created an attachment (id=15985) [details]
first cut at a fix
------- Comment #2 From 2007-08-15 13:46:27 PST -------
Created an attachment (id=15986) [details]
first cut at a fix (uploaded wrong the first time)
------- Comment #3 From 2007-09-05 22:06:32 PST -------
(From update of attachment 15986 [details])
+private:
+    virtual bool nodeMatches(Node*) const;

private virtual?

Nothing else leaps out at me as being odd, but i don't feel qualified to comment on risk or correctness :(

My ad hoc benchmarking of this show Insertion time (from hixies DOM Core test) go from ~1100ms (avg. of 3 runs) to ~110ms (3 runs)
------- Comment #4 From 2007-09-05 23:57:04 PST -------
(From update of attachment 15986 [details])
Okay, after repeated runs (mostly cleaning my tree) i don't think this is safe, it's a huge regression for the Indexd portion of Hixies DOM Core tests
------- Comment #5 From 2007-09-05 23:58:52 PST -------
http://nontroppo.org/timer/Hixie_DOM.html

The index test goes from 26ms to >1000ms
------- Comment #6 From 2007-11-22 10:16:28 PST -------
Created an attachment (id=17443) [details]
proposed fix

This doesn't cause regressions in other tests.

I couldn't fully understand the original patch, so I started from scratch. It's possible that I missed some additional optimization opportunities that were present there.
------- Comment #7 From 2007-11-22 10:19:08 PST -------
> Unnecessary churn in the NodeList code.

In other words, the problem had nothing to do with insertion performance - it's the previous test (index) that was making us register too many NodeLists, and thus slowed down subsequent DOM mutations.
------- Comment #8 From 2007-11-22 19:33:44 PST -------
(From update of attachment 17443 [details])
r=me
------- Comment #9 From 2007-11-22 22:44:20 PST -------
Committed revision 27983.