Commit 0ed98446 authored by mayankleoboy1's avatar mayankleoboy1 Committed by mayankleoboy1@gmail.com
Browse files

Bug 2000355 - Skip to the lowest observed ancestor on repeated mutations r=smaug 

ForEachAncestorObserver walks the inclusive ancestor chain on every mutation,
which is O(depth). To improve performance, nsINode memoizes one walk's start
node and the lowest ancestor found to hold an observer, and a later walk from
the same node starts there.

The memo only picks where a walk starts, never what it returns. It is dropped
when an observer is registered anywhere, or when certain mutations occur.

Testcase goes 8346 -> 2342 samples, with the walk down from 53% to 0.8%.
Speedometer3 is neutral. mach try auto is clean.

Differential Revision: https://phabricator.services.mozilla.com/D314591
parent c083282e

dom/base/MutationObservers.cpp

+36 −1
Changes for dom/base/MutationObservers.cpp: 36 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -47,12 +47,42 @@ template <typename NotifyObserver> 
static inline nsINode* ForEachAncestorObserver(nsINode* aNode,

                                               NotifyObserver& aFunc,

                                               uint32_t aCallback) {

  // Nodes below the lowest observed ancestor cost a slots load and yield

  // nothing, so a previous walk from this node lets us start above them.

  nsINode* node = nsINode::ObserverChainSkipTo(aNode);

#ifdef DEBUG

  if (node) {

    nsINode* debugNode = aNode;

    nsINode* debugLast = nullptr;

    while (debugNode && debugNode != node) {

      if (debugNode->GetMutationObservers()) {

        MOZ_ASSERT(false, "skipped nodes must not have observers");

      }

      debugLast = debugNode;

      if (!(debugNode = debugNode->GetParentNode())) {

        if (ShadowRoot* shadow = ShadowRoot::FromNode(debugLast)) {

          debugNode = shadow->GetHost();

        }

      }

    }

    MOZ_ASSERT(debugNode == node,

               "cached skip-to node must be a valid ancestor");

  }

#endif

  bool memoize = !node && aNode->IsInComposedDoc();

  if (!node) {

    node = aNode;

  }



  nsINode* last;

  nsINode* node = aNode;

  do {

    mozilla::SafeDoublyLinkedList<nsIMutationObserver>* observers =

        node->GetMutationObservers();

    if (observers) {

      if (memoize && node != aNode) {

        nsINode::NoteObserverChain(aNode, node);

      }

      memoize = false;

      for (auto iter = observers->begin(); iter != observers->end(); ++iter) {

        if (iter->IsCallbackEnabled(aCallback)) {

          aFunc(&*iter);
@@ -66,6 +96,11 @@ static inline nsINode* ForEachAncestorObserver(nsINode* aNode, 
      }

    }

  } while (node);



  // Nothing observed anything, so a later walk can go straight to the end.

  if (memoize && last != aNode) {

    nsINode::NoteObserverChain(aNode, last);

  }

  return last;

}

dom/base/MutationObservers.h

+2 −0
Changes for dom/base/MutationObservers.h: 2 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -115,6 +115,8 @@ class MutationObservers { 
   * @see nsIMutationObserver::ParentChainChanged

   */

  static inline void NotifyParentChainChanged(nsIContent* aContent) {

    // The only way a chain changes without a child list changing.

    nsINode::ForgetObserverChainIfCached(aContent);

    mozilla::SafeDoublyLinkedList<nsIMutationObserver>* observers =

        aContent->GetMutationObservers();

    if (observers) {

dom/base/nsINode.cpp

+8 −0
Changes for dom/base/nsINode.cpp: 8 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -585,9 +585,12 @@ class ChildIndexCache { 
nsClassHashtable<nsPtrHashKey<const nsINode>, ChildIndexCache::Entry>

    ChildIndexCache::sCache;

const nsINode* ChildIndexCache::sLastAccessedParent = nullptr;

const nsINode* nsINode::sObserverChainStart = nullptr;

nsINode* nsINode::sObserverChainSkipTo = nullptr;

ChildIndexCache::Entry* ChildIndexCache::sLastAccessedEntry = nullptr;



nsINode::~nsINode() {

  ForgetObserverChainIfCached(this);

  MOZ_ASSERT(!ChildIndexCache::Contains(this),

             "Node still in ChildIndexCache at destruction?");

  MOZ_ASSERT(ChildIndexCache::LastAccessedParent() != this,
@@ -2368,6 +2371,11 @@ void nsINode::DisconnectChild(nsIContent* aKid) { 
  MOZ_ASSERT(GetChildCount() > 0);



  RemoveFromCache(this);

  // Clear the cache if there is a chance the start node is in the disconnected

  // subtree.

  if (!nsINode::IsObserverChainStart(this) && IsInComposedDoc()) {

    nsINode::ForgetObserverChain();

  }

  ChildIndexCache::Invalidate(this, aKid);



  nsIContent* previousSibling = aKid->GetPreviousSibling();

dom/base/nsINode.h

+37 −0
Changes for dom/base/nsINode.h: 37 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -1589,6 +1589,7 @@ class nsINode : public mozilla::dom::EventTarget { 
                   "Observer already in the list");



      s->mMutationObservers.pushBack(aMutationObserver);

      ForgetObserverChain();

    }

  }
@@ -1606,6 +1607,7 @@ class nsINode : public mozilla::dom::EventTarget { 
    if (aMutationObserver &&

        !s->mMutationObservers.contains(aMutationObserver)) {

      s->mMutationObservers.pushBack(aMutationObserver);

      ForgetObserverChain();

    }

  }
@@ -1630,11 +1632,42 @@ class nsINode : public mozilla::dom::EventTarget { 
   * Removes a mutation observer.

   */

  void RemoveMutationObserver(nsIMutationObserver* aMutationObserver) {

    // We do not need to invalidate the observer chain cache here. The cache

    // points to the lowest ancestor that had an observer. If an observer is

    // removed, the cached skip-to node might become overly conservative, but

    // it will never skip over new observers, so correctness is preserved.

    if (nsSlots* s = GetExistingSlots()) {

      s->mMutationObservers.remove(aMutationObserver);

    }

  }



  /**

   * Memoizes one ForEachAncestorObserver walk's start node and the lowest

   * ancestor holding an observer, so repeated mutations against the same node

   * skip the stretch between them. Only picks where a walk starts, never what

   * it returns, so a stale skip costs iteration and memory accesses rather than

   * correctness.

   */

  static bool IsObserverChainStart(const nsINode* aNode) {

    return aNode == sObserverChainStart;

  }

  static nsINode* ObserverChainSkipTo(const nsINode* aNode) {

    return aNode == sObserverChainStart ? sObserverChainSkipTo : nullptr;

  }

  static void NoteObserverChain(const nsINode* aStart, nsINode* aSkipTo) {

    sObserverChainStart = aStart;

    sObserverChainSkipTo = aSkipTo;

  }

  static void ForgetObserverChain() {

    sObserverChainStart = nullptr;

    sObserverChainSkipTo = nullptr;

  }

  static void ForgetObserverChainIfCached(const nsINode* aNode) {

    if (aNode == sObserverChainStart || aNode == sObserverChainSkipTo) {

      ForgetObserverChain();

    }

  }



  void RemoveMutationObserver(nsMultiMutationObserver* aMultiMutationObserver);



  mozilla::SafeDoublyLinkedList<nsIMutationObserver>* GetMutationObservers();
@@ -3230,6 +3263,10 @@ class nsINode : public mozilla::dom::EventTarget { 
  // EventListenerManager*, which is null for most nodes; otherwise a non-null

  // nsSlots*, which then owns the manager.  See SetSlots.

  uintptr_t mSlotsOrListenerManager = kListenerManagerBit;



  // See ObserverChainSkipTo.

  static const nsINode* sObserverChainStart;

  static nsINode* sObserverChainSkipTo;

};



NON_VIRTUAL_ADDREF_RELEASE(nsINode)