Fast path isDescendantOf for nodes in different documents
Created attachment 176868 [details] Patch
Comment on attachment 176868 [details] Patch How often is this help? It looks interacting nodes are usually living in the same document. In such case this check just adds an extra check.
(In reply to comment #2) > (From update of attachment 176868 [details]) > How often is this help? > It looks interacting nodes are usually living in the same document. > In such case this check just adds an extra check. This happens whenever you move a node from one iframe to another, or the parent document. In those cases we traverse the ancestors when there's no reason to. Looking at places where we use isDescendantOf() it's almost always the same document except in checkAddChild() or checkReplaceChild() where they could be in different documents. The inDocument() check we already do here is similarly as rare. Nodes are almost always already in the document when we do isDesendantOf except during appendChild() and removeChild(). So if this is a regression then we probably shouldn't do the inDocument() check except for appendChild() and removeChild().
Comment on attachment 176868 [details] Patch OK, let's land and see how this goes. Before landing, please check Dromaeo/dom-modify to ensure that this doesn't impact non-iframe case much.
I don't see this landing in Webkit source (from Github) despite being r+ and mentioning about landing in Comment 04. I am not sure whether this patch is applicable today or not but just wanted to leave quick link here: https://github.com/WebKit/WebKit/blob/main/Source/WebCore/dom/Node.cpp#L1025 Appreciate if someone can mark this bug accordingly. Thanks!
This is a won't fix at this point.
Fwiw this perf cliff still exists. Moving nodes between two documents walks up the tree resulting in depth of tree work. If you remove from one document first it avoids that by taking the isConnected fast path because we check the ancestors before removal. So like: deepNode.remove() otherDocNode.appendChild(deepNode) Should be faster than otherDocNode.appendChild(deepNode) The debate was always if one extra branch in the common case (maybe with an UNLIKELY) is worse to guard against the cliff.