perf: Reduce component structure check overhead (#516)

Previously, we issued change events whenever component order changed; this resulted in spurious invocations of listeners,
which when did (potentially very expensive) redundant checks of what components were in their children. In particular, this
was done whenever a _component_ changed; this was probably a bug, as a change event on the _GameObject_ is issued when
component order changes.

This change makes a few adjustments to fix this:
* We record the child component list in the ShadowGameObject, and only generate change events if it actually changes.
* We only perform component structure checks when _game objects_ change, not when the components themselves change.
* We avoid registering complex filters for component structure events; those filters were expensive, frequently redundantly
registered, and are no longer needed with the new ShadowGameObject component tracking.

Closes: #513

perf: Remove redundant listener filters for MonitorGetComponents

Now that the shadow hierarchy is checking for component structure changes, we don't need to perform these checks in SingleObjectQueries.

Avoiding doing these checks there avoids redundant processing, which is particularly important since these will all generally
 converge into the same invalidation.

CHANGELOG.md

@@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 ### Fixed
-- Performance improvements (#515)
+- Performance improvements (#515 #516)
 
 ### Changed
 

Editor/ChangeStream/ChangeStreamMonitor.cs

@@ -277,7 +277,7 @@ private static void OnChangeGameObjectStructure(ChangeGameObjectStructureEventAr
         {
             var instanceId = data.instanceId;
 
-            ObjectWatcher.Instance.Hierarchy.FireStructureChangeEvent(instanceId);
+            ObjectWatcher.Instance.Hierarchy.MaybeFireStructureChangeEvent(instanceId);
         }
 
         private static void OnChangeGameObjectStructureHierarchy(ChangeGameObjectStructureHierarchyEventArgs data)

Editor/ChangeStream/ObjectWatcher.cs

@@ -264,24 +264,24 @@ private static bool InvokeCallback<T>(Func<T, bool> callback, object t) where T
             }
         }
 
-        public C[] MonitorGetComponents<C>(GameObject obj, ComputeContext ctx,
-            Func<C[]> get0, bool includeChildren)
+        public void MonitorGetComponents(GameObject obj, ComputeContext ctx, bool includeChildren)
         {
             var previewScene = NDMFPreviewSceneManager.GetPreviewScene();
-            Func<C[]> get = () => get0().Where(c => (c as Component)?.gameObject.scene != previewScene).ToArray();
-
-            C[] components = get();
 
+            // Ensure component structure data is up-to-date
+            Hierarchy.RequestComponentStructureCheck(obj.GetInstanceID());
+
             var cancel = Hierarchy.RegisterGameObjectListener(obj, e =>
             {
                 if (e == HierarchyEvent.ChildComponentsChanged && !includeChildren) return false;
 
                 switch (e)
                 {
                     case HierarchyEvent.ChildComponentsChanged:
+                        return includeChildren;
                     case HierarchyEvent.SelfComponentsChanged:
                     case HierarchyEvent.ForceInvalidate:
-                        return obj == null || !components.SequenceEqual(get());
+                        return true;
                     default:
                         return false;
                 }
@@ -290,33 +290,8 @@ public C[] MonitorGetComponents<C>(GameObject obj, ComputeContext ctx,
             if (includeChildren) Hierarchy.EnableComponentMonitoring(obj);
 
             BindCancel(ctx, cancel);
-
-            return components;
         }
-
-        public C MonitorGetComponent<C>(GameObject obj, ComputeContext ctx,
-            Func<C> get) where C : class
-        {
-            C component = get();
-
-            var cancel = Hierarchy.RegisterGameObjectListener(obj, e =>
-            {
-                switch (e)
-                {
-                    case HierarchyEvent.SelfComponentsChanged:
-                    case HierarchyEvent.ChildComponentsChanged:
-                    case HierarchyEvent.ForceInvalidate:
-                        return obj == null || !ReferenceEquals(component, get());
-                    default:
-                        return false;
-                }
-            }, ctx);
-
-            BindCancel(ctx, cancel);
-
-            return component;
-        }
-
+
         class WrappedDisposable : IDisposable
         {
             private readonly int _targetThread;

Editor/ChangeStream/PropertyMonitor.cs

@@ -108,6 +108,11 @@ public async Task CheckAllObjects()
 
                     var (instanceId, reg) = pair;
 
+                    if (reg._obj is GameObject)
+                    {
+                        ObjectWatcher.Instance.Hierarchy.RequestComponentStructureCheck(instanceId);
+                    }
+
                     // Wake up all listeners to see if their monitored value has changed
                     Profiler.BeginSample("FirePropsUpdated", EditorUtility.InstanceIDToObject(instanceId));
                     reg._listeners.Fire(PropertyMonitorEvent.PropsUpdated);

Editor/ChangeStream/ShadowGameObject.cs

@@ -5,6 +5,7 @@
 #endif
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Threading;
 using nadena.dev.ndmf.preview;
 using nadena.dev.ndmf.preview.trace;
@@ -445,15 +446,8 @@ internal void FireObjectChangeNotification(int instanceId)
                     shadow.IsActive = shadow.GameObject.activeSelf;
                     FirePathChangeNotifications(shadow);
                 }
-            }
 
-            var component = EditorUtility.InstanceIDToObject(instanceId) as Component;
-            if (component != null)
-            {
-                // This event may have been a component reordering, so trigger a synthetic structure change event.
-                // TODO: Cache component positions?
-                var parentId = component.gameObject.GetInstanceID();
-                FireStructureChangeEvent(parentId);
+                MaybeFireStructureChangeEvent(instanceId);
             }
 
             if (_otherObjects.TryGetValue(instanceId, out var shadowComponent))
@@ -462,6 +456,47 @@ internal void FireObjectChangeNotification(int instanceId)
             }
         }
 
+        private SendOrPostCallback _deferredComponentStructureCheckCb;
+
+        /// <summary>
+        ///     Requests that we (asynchronously) check the order and contents of components on the specified GameObject.
+        ///     If they have changed since the last check, a structure change event will be fired.
+        /// </summary>
+        /// <param name="instanceId"></param>
+        internal void RequestComponentStructureCheck(int instanceId)
+        {
+            if (!_gameObjects.TryGetValue(instanceId, out var shadow))
+            {
+                return;
+            }
+
+            if (shadow.StructureCheckQueued)
+            {
+                return;
+            }
+
+            if (_deferredComponentStructureCheckCb == null)
+            {
+                _deferredComponentStructureCheckCb = DeferredComponentStructureCheck;
+            }
+
+            NDMFSyncContext.Context.Post(_deferredComponentStructureCheckCb, shadow);
+            shadow.StructureCheckQueued = true;
+        }
+
+        internal void DeferredComponentStructureCheck(object erased)
+        {
+            var shadow = (ShadowGameObject)erased;
+            if (!_gameObjects.TryGetValue(shadow.InstanceID, out var currentShadow) || currentShadow != shadow)
+            {
+                return;
+            }
+
+            shadow.StructureCheckQueued = false;
+
+            MaybeFireStructureChangeEvent(shadow.InstanceID);
+        }
+
         /// <summary>
         /// Fires a notification that the specified GameObject has a new parent.
         /// </summary>
@@ -586,7 +621,7 @@ internal void FireReorderNotification(int parentInstanceId)
             FireParentComponentChangeNotifications(shadow);
         }
 
-        internal void FireStructureChangeEvent(int instanceId)
+        internal void MaybeFireStructureChangeEvent(int instanceId)
         {
 #if NDMF_TRACE_SHADOW
             System.Diagnostics.Debug.WriteLine($"[ShadowHierarchy] FireStructureChangeEvent({instanceId})");
@@ -597,6 +632,10 @@ internal void FireStructureChangeEvent(int instanceId)
                 return;
             }
 
+            var newStructure = shadow.CurrentComponentStructure;
+            if (shadow.ComponentStructure.SequenceEqual(newStructure)) return;
+            shadow.ComponentStructure = newStructure;
+
             FireEvent(shadow._listeners, HierarchyEvent.SelfComponentsChanged);
             FireParentComponentChangeNotifications(shadow.Parent);
         }
@@ -720,6 +759,10 @@ internal class ShadowGameObject
         internal bool ComponentMonitoring { get; set; } = false;
         internal bool IsActive { get; set; }
 
+        internal bool StructureCheckQueued;
+
+        internal int[] ComponentStructure;
+
         internal ShadowGameObject Parent
         {
             get => _parent;
@@ -756,6 +799,24 @@ internal ShadowGameObject(GameObject gameObject)
             GameObject = gameObject;
             Scene = gameObject.scene;
             IsActive = gameObject.activeSelf;
+
+            ComponentStructure = CurrentComponentStructure;
+        }
+
+        internal int[] CurrentComponentStructure
+        {
+            get
+            {
+                var comps = GameObject.GetComponents<Component>();
+                var instanceIds = new int[comps.Length];
+
+                for (var i = 0; i < comps.Length; i++)
+                {
+                    instanceIds[i] = comps[i].GetInstanceID();
+                }
+
+                return instanceIds;
+            }
         }
     }
 }

Editor/PreviewSystem/ComputeContext/SingleObjectQueries.cs

@@ -249,8 +249,9 @@ public static C GetComponent<C>(this ComputeContext ctx, GameObject obj,
         {
             if (obj == null) return null;
 
-            return ObjectWatcher.Instance.MonitorGetComponent(obj, ctx,
-                () => obj != null ? InternalGetComponent<C>(obj) : null);
+            ObjectWatcher.Instance.MonitorGetComponents(obj, ctx, false);
+
+            return obj.GetComponent<C>();
         }
 
         public static Component GetComponent(this ComputeContext ctx, GameObject obj, Type type,
@@ -260,8 +261,9 @@ public static Component GetComponent(this ComputeContext ctx, GameObject obj, Ty
         {
             if (obj == null) return null;
 
-            return ObjectWatcher.Instance.MonitorGetComponent(obj, ctx,
-                () => obj != null ? InternalGetComponent(obj, type) : null);
+            ObjectWatcher.Instance.MonitorGetComponents(obj, ctx, false);
+
+            return obj.GetComponent(type);
         }
 
         public static C[] GetComponents<C>(this ComputeContext ctx, GameObject obj,
@@ -271,8 +273,9 @@ public static C[] GetComponents<C>(this ComputeContext ctx, GameObject obj,
         {
             if (obj == null) return Array.Empty<C>();
 
-            return ObjectWatcher.Instance.MonitorGetComponents(obj, ctx,
-                () => obj != null ? obj.GetComponents<C>() : Array.Empty<C>(), false);
+            ObjectWatcher.Instance.MonitorGetComponents(obj, ctx, false);
+
+            return obj.GetComponents<C>();
         }
 
         public static Component[] GetComponents(this ComputeContext ctx, GameObject obj, Type type,
@@ -282,8 +285,9 @@ public static Component[] GetComponents(this ComputeContext ctx, GameObject obj,
         {
             if (obj == null) return Array.Empty<Component>();
 
-            return ObjectWatcher.Instance.MonitorGetComponents(obj, ctx,
-                () => obj != null ? obj.GetComponents(type) : Array.Empty<Component>(), false);
+            ObjectWatcher.Instance.MonitorGetComponents(obj, ctx, false);
+
+            return obj.GetComponents(type);
         }
 
         public static C[] GetComponentsInChildren<C>(this ComputeContext ctx, GameObject obj, bool includeInactive,
@@ -294,8 +298,9 @@ public static C[] GetComponentsInChildren<C>(this ComputeContext ctx, GameObject
         {
             if (obj == null) return Array.Empty<C>();
 
-            return ObjectWatcher.Instance.MonitorGetComponents(obj, ctx,
-                () => obj != null ? obj.GetComponentsInChildren<C>(includeInactive) : Array.Empty<C>(), true);
+            ObjectWatcher.Instance.MonitorGetComponents(obj, ctx, true);
+
+            return obj.GetComponentsInChildren<C>(includeInactive);
         }
 
         public static Component[] GetComponentsInChildren(this ComputeContext ctx, GameObject obj, Type type,
@@ -306,9 +311,9 @@ public static Component[] GetComponentsInChildren(this ComputeContext ctx, GameO
         {
             if (obj == null) return Array.Empty<Component>();
 
-            return ObjectWatcher.Instance.MonitorGetComponents(obj, ctx,
-                () => obj != null ? obj.GetComponentsInChildren(type, includeInactive) : Array.Empty<Component>(),
-                true);
+            ObjectWatcher.Instance.MonitorGetComponents(obj, ctx, true);
+
+            return obj.GetComponentsInChildren(type, includeInactive);
         }
     }
 }

Editor/PreviewSystem/Task/NDMFSyncContext.cs

@@ -1,7 +1,6 @@
 using System;
 using System.Collections.Generic;
 using System.Threading;
-using JetBrains.Annotations;
 using UnityEditor;
 using UnityEngine;
 
@@ -15,7 +14,7 @@ namespace nadena.dev.ndmf.preview
     public static class NDMFSyncContext
     {
         public static SynchronizationContext Context = new Impl();
-        private static Impl InternalContext = (Impl)Context;
+        internal static Impl InternalContext = (Impl)Context;
 
         /// <summary>
         ///    Runs the given function on the unity main thread, passing the given target as an argument.
@@ -61,7 +60,7 @@ public void Dispose()
             }
         }
 
-        private class Impl : SynchronizationContext
+        internal class Impl : SynchronizationContext
         {
             private SynchronizationContext _unityMainThreadContext;
 
@@ -78,7 +77,7 @@ internal Impl()
                 EditorApplication.delayCall += () =>
                 {
                     // Obtain the unity synchronization context
-                    _unityMainThreadContext = SynchronizationContext.Current;
+                    _unityMainThreadContext = Current;
                     unityMainThreadId = Thread.CurrentThread.ManagedThreadId;
 
                     Turn(); // process anything that was queued before we had a chance to initialize
@@ -99,7 +98,8 @@ private void InvokeTurn(object state)
                 ((Impl)state).Turn();
             }
 
-            private void Turn()
+            // visible for tests
+            internal void Turn()
             {
                 lock (_lock)
                 {

UnitTests~/RQ-EditorTests/ShadowHierarchyTest.cs

@@ -214,7 +214,8 @@ public void ComponentChangeNotifications_GeneratedWhenObjectItselfChanges()
                 return false;
             }, ctx);
 
-            shadow.FireStructureChangeEvent(obj.GetInstanceID());
+            obj.AddComponent<MeshFilter>();
+            shadow.MaybeFireStructureChangeEvent(obj.GetInstanceID());
 
             Assert.AreEqual(1, events.Count);
             Assert.AreEqual(HierarchyEvent.SelfComponentsChanged, events[0]);
@@ -239,7 +240,9 @@ public void ComponentChangeNotifications_GeneratedWhenChildChanges()
             }, ctx);
 
             shadow.EnableComponentMonitoring(parent);
-            shadow.FireStructureChangeEvent(child.GetInstanceID());
+
+            child.AddComponent<MeshFilter>();
+            shadow.MaybeFireStructureChangeEvent(child.GetInstanceID());
 
             Assert.AreEqual(1, events.Count);
             Assert.AreEqual(HierarchyEvent.ChildComponentsChanged, events[0]);
@@ -274,8 +277,9 @@ public void ComponentChangeNotifications_FiredAfterReparents()
             Assert.IsTrue(events.Contains(HierarchyEvent.ChildComponentsChanged));
 
             events.Clear();
-
-            shadow.FireStructureChangeEvent(p3.GetInstanceID());
+
+            p3.AddComponent<MeshFilter>();
+            shadow.MaybeFireStructureChangeEvent(p3.GetInstanceID());
 
             // Assert.AreEqual(1, events.Count); - TODO - deduplicate events
             Assert.IsTrue(events.Contains(HierarchyEvent.ChildComponentsChanged));
@@ -466,8 +470,7 @@ public void ComponentReorder_TriggersStructureChange()
             Assert.AreEqual(iid1, iid2a);
             Assert.AreEqual(iid2, iid1a);
 
-            shadow.FireObjectChangeNotification(iid1);
-            shadow.FireObjectChangeNotification(iid2);
+            shadow.FireObjectChangeNotification(o2.GetInstanceID());
 
             Assert.Contains((1, HierarchyEvent.ChildComponentsChanged), events);
             Assert.Contains((2, HierarchyEvent.SelfComponentsChanged), events);