Reduce alloc and copying in AsyncNodeDeletionQueue #47198
Conversation
|
EWS run on previous version of this PR (hash 94d615d) |
| @@ -43,7 +43,7 @@ class AsyncNodeDeletionQueue { | |||
| ALWAYS_INLINE static bool isNodeLikelyLarge(const Node&); | |||
|
|
|||
| private: | |||
| Vector<Ref<Node>> m_queue; | |||
| Vector<Vector<Ref<Node>>> m_queue; | |||
There was a problem hiding this comment.
This is not right. We need to use NodeVector, which has inline capacity of 11.
There was a problem hiding this comment.
Good catch!
I assume my patch is destructing Vector<Ref<Node>, 11> and constructing Vector<Ref<Node>, 0> right now then...
There was a problem hiding this comment.
Hmm, this fix seems to hurt perf, which seems very strange to me.
I stepped through to make sure it's behaving as expected after this fix and can see that we aren't moving the NodeVector contents.
Any idea what could be going on?
There was a problem hiding this comment.
Vectors with inline buffers are not cheap to move. Here NodeVector has an inline cache of 11. So until it reaches 12 nodes in it, moving it is actually a copy. This may be related.
The move I'm referring to is here:
m_queue.append(WTFMove(children));
Most of the time it will just copy all children's into a few vector instead of just quickly "moving" a VectorBuffer.
There was a problem hiding this comment.
Oh right.
Inline contents are moved...
https://bugs.webkit.org/show_bug.cgi?id=294987 rdar://154311875 Reviewed by NOBODY (OOPS!). Instead of moving the contents of the NodeVector to m_queue, just move the inline part of the NodeVector so that we don't need to grow m_queue as many times, can destruct the NodeVector opportunistically, and do less copying. * Source/WebCore/dom/AsyncNodeDeletionQueue.h: * Source/WebCore/dom/AsyncNodeDeletionQueueInlines.h: (WebCore::AsyncNodeDeletionQueue::addIfSubtreeSizeIsUnderLimit):
94d615d to
e131d10
Compare
|
EWS run on current version of this PR (hash e131d10) |
e131d10
e131d10