[webkit-dev] scoping Node destruction during DOM modifications
ojan at chromium.org
Thu Mar 1 16:50:26 PST 2012
We have a lot of code (e.g. in ContainerNode.cpp or any of the editing
code) that needs to RefPtr nodes to make sure they're not destroyed due to
synchronous events like blur, mutation events, etc. For example,
ContainerNode::removeChild needs to RefPtr the parent to make it's not
destroyed during a sync event.
Currently, we need to write careful code that knows whether any methods
it's calling can fire sync events and RefPtrs all the appropriate nodes. I
have a proposal to automate this in
https://bugs.webkit.org/show_bug.cgi?id=80041. It certainly makes the code
cleaner and safer, but it comes at a performance cost in some cases.
Basically, any scope that needs to be careful of sync events adds a
DOMRemovalScope at the top which keeps nodes from getting destroyed until
the scope is destroyed (DOMRemovalScope adds them to a
Vector<RefPtr<Node>>). In cases where we don't delete nodes, there's no
performance impact. In cases where we delete nodes, this is always slower.
I uploaded two performance tests to that bug:
1. Set innerHTML = "" on a node that has half a million children. This test
goes from ~166ms to ~172ms on my machine. A 3.6% regression.
2. Destroy half a million nodes *during* a synchronous event (uses
range.deleteContents). Goes from ~284ms to ~342ms. A 20% regression.
So destroying Nodes during synchronous events fired during a DOM mutation
is significantly slower. This case is rare though. I had to think hard to
come up with a case where we would hit this. For the most part, nodes don't
actually get destroyed due to JS until a garbage collection occurs, which
is usually after the event has completed and the DOMRemovalScope has been
The advantage of this is that it gives a simple way to address a common
class of crash and potentially security bugs. The disadvantage of course is
the possible performance hit.
What do you think? Is this worth trying? Are there better ideas?
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the webkit-dev