NEW 103571
some checkReplaceChild() can be skipped if there are no event listener fired.
https://bugs.webkit.org/show_bug.cgi?id=103571
Summary some checkReplaceChild() can be skipped if there are no event listener fired.
Hajime Morrita
Reported 2012-11-28 16:08:25 PST
Suggested at Bug 103372. ContainerNode::replaceChild() calls multiple checkReplaceChild() for preventing manifold tree. But we can skip the one other than the first if we know there is no event listener fired since the last checkReplaceChild() call. That omission will improve replaceChild() performance.
Attachments
Patch (2.87 KB, patch)
2012-11-29 03:08 PST, Hajime Morrita
no flags
Hajime Morrita
Comment 1 2012-11-29 03:08:56 PST
Hajime Morrita
Comment 2 2012-11-29 03:11:21 PST
I tried, but didn't go faster and went even slower. It looks our fast path does fast enough. I'd close this bug unless there are something I overlooked.
Ojan Vafai
Comment 3 2012-11-29 17:06:25 PST
What did you use to measure performance here? The main performance bottleneck in checkAcceptChild I think is the contains call, which is O(n) in the depth of the tree. I would expect a test case with a deep tree to show improvement.
Hajime Morrita
Comment 4 2012-11-29 17:16:18 PST
(In reply to comment #3) > What did you use to measure performance here? The main performance bottleneck in checkAcceptChild I think is the contains call, which is O(n) in the depth of the tree. I would expect a test case with a deep tree to show improvement. Ah, that's right. What I used is dom-modify. We need some better benchmark here.
Kentaro Hara
Comment 5 2012-11-29 17:18:39 PST
(In reply to comment #4) > We need some better benchmark We're always saying this:D
Ojan Vafai
Comment 6 2012-12-01 10:33:37 PST
(In reply to comment #5) > (In reply to comment #4) > > We need some better benchmark > > We're always saying this:D lol. It's always true! I don't think that the benchmarks we have generally don't test deep DOM trees, so we don't get good performance testing of things that are O(n) in DOM depth.
Note You need to log in before you can comment on or make changes to this bug.