Browse Source

Optimize Node::MarkDirty() implementation by a) relying on the invariant that all children of a dirty node must also be dirty, and that all parents of a clean node must also be clean, b) avoid double-dereferencing Component WeakPtr in listeners, c) use the swap-with-last erase trick to avoid O(n^2) behavior in removing listeners, and d) perform tail call optimization to avoid excessive recursive function calls when marking children dirty. This optimizes the Node::MarkDirty() time in 06_SkeletalAnimation scene with 2000 Jacks from 42.73% down to 16.43%, a net saving of -61.5% less time spent in that call site. Closes #931.

Jukka Jylänki 10 years ago
parent
commit
d168ab9d91
1 changed files with 39 additions and 12 deletions
  1. 39 12
      Source/Urho3D/Scene/Node.cpp

+ 39 - 12
Source/Urho3D/Scene/Node.cpp

@@ -558,23 +558,50 @@ void Node::SetOwner(Connection* owner)
 
 
 void Node::MarkDirty()
 void Node::MarkDirty()
 {
 {
-    dirty_ = true;
+    Node *cur = this;
+    for (;;)
+    {
+        // Precondition:
+        // a) whenever a node is marked dirty, all its children are marked dirty as well.
+        // b) whenever a node is cleared from being dirty, all its parents must have been
+        //    cleared as well.
+        // Therefore if we are recursing here to mark this node dirty, and it already was,
+        // then all children of this node must also be already dirty, and we don't need to
+        // reflag them again.
+        if (cur->dirty_) return;
+        cur->dirty_ = true;
+
+        // Notify listener components first, then mark child nodes
+        for (Vector<WeakPtr<Component> >::Iterator i = cur->listeners_.Begin(); i != cur->listeners_.End();)
+        {
+            Component *c = *i;
+            if (c)
+            {
+                c->OnMarkedDirty(cur);
+                ++i;
+            }
+            // If listener has expired, erase from list (swap with the last element to avoid O(n^2) behavior)
+            else
+            {
+                *i = cur->listeners_.Back();
+                cur->listeners_.Pop();
+            }
+        }
 
 
-    // Notify listener components first, then mark child nodes
-    for (Vector<WeakPtr<Component> >::Iterator i = listeners_.Begin(); i != listeners_.End();)
-    {
-        if (*i)
+        // Tail call optimization: Don't recurse to mark the first child dirty, but
+        // instead process it in the context of the current function. If there are more
+        // than one child, then recurse to the excess children.
+        Vector<SharedPtr<Node> >::Iterator i = cur->children_.Begin();
+        if (i != cur->children_.End())
         {
         {
-            (*i)->OnMarkedDirty(this);
-            ++i;
+            Node *next = *i;
+            for (++i; i != cur->children_.End(); ++i)
+                (*i)->MarkDirty();
+            cur = next;
         }
         }
-        // If listener has expired, erase from list
         else
         else
-            i = listeners_.Erase(i);
+            return;
     }
     }
-
-    for (Vector<SharedPtr<Node> >::Iterator i = children_.Begin(); i != children_.End(); ++i)
-        (*i)->MarkDirty();
 }
 }
 
 
 Node* Node::CreateChild(const String& name, CreateMode mode, unsigned id)
 Node* Node::CreateChild(const String& name, CreateMode mode, unsigned id)