ok let's go

Assets/Mirror/Core/NetworkBehaviour.cs

@@ -159,7 +159,17 @@ protected void SetSyncVarHookGuard(ulong dirtyBit, bool value)
         // called once it becomes dirty, not called again while already dirty.
         // we only want to follow the .netIdentity memory indirection once.
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        void OnBecameDirty() => netIdentity.OnBecameDirty();
+        void OnBecameDirty()
+        {
+            // previously, NetworkBehaviour.OnBecameDirty would call
+            // NetworkIdentity.OnBecameDirty, which would insert into dirtyObjects.
+            //
+            // instead, let's try to insert directly.
+            // avoids the NetworkIdentity indirection.
+            // the only indirection is NetworkServer.dirtyObjects.
+            // we insert 'netIdentity', which is a reference.
+            NetworkServer.dirtySpawned.Add(netIdentity);
+        }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         void SetSyncObjectDirtyBit(ulong dirtyBit)

Assets/Mirror/Core/NetworkIdentity.cs

@@ -806,36 +806,6 @@ internal void OnStopLocalPlayer()
             }
         }
 
-        // NetworkBehaviour OnBecameDirty calls NetworkIdentity callback with index
-        bool addedToDirtySpawned = false;
-        internal void OnBecameDirty()
-        {
-            // ensure either isServer or isClient are set.
-            // ensures tests are obvious. without proper setup, it should throw.
-            if (!isClient && !isServer)
-                Debug.LogWarning("NetworkIdentity.OnBecameDirty(): neither isClient nor isServer are true. Improper setup?");
-
-
-            if (isServer)
-            {
-                // only add to dirty spawned once.
-                // don't run the insertion twice.
-                if (!addedToDirtySpawned)
-                {
-                    // insert into server dirty objects if not inserted yet
-                    // TODO keep a bool so we don't insert all the time?
-
-                    // only add if observed.
-                    // otherwise no point in adding + iterating from broadcast.
-                    if (observers.Count > 0)
-                    {
-                        NetworkServer.dirtySpawned.Add(this);
-                        addedToDirtySpawned = true;
-                    }
-                }
-            }
-        }
-
         // build dirty mask for server owner & observers (= all dirty components).
         // faster to do it in one iteration instead of iterating separately.
         (ulong, ulong, ulong) ServerDirtyMasks(bool initialState)
@@ -1000,10 +970,6 @@ internal void SerializeServer(bool initialState, NetworkWriter ownerWriter, Netw
             // the NetworkIdentity from dirtyObjects just yet.
             // otherwise if we remove before it was synced, we would miss a sync.
             pendingDirty = pendingMask != 0;
-
-            // if none are still pending, this will be removed from dirtyObjects.
-            // in that case, clear our flag (the flag is only for performance).
-            if (!pendingDirty) addedToDirtySpawned = false;
         }
 
         // serialize components into writer on the client.

Assets/Mirror/Core/NetworkServer.cs

@@ -56,11 +56,8 @@ public static partial class NetworkServer
         // otherwise each NetworkBehaviour would need an Update() to wait until
         // syncInterval is elapsed, which is more expansive then simply adding
         // a few false positives here.
-        //
-        // NetworkIdentity adds itself to dirtySpawned exactly once.
-        // we can safely use a List<T> here, faster than a Dictionary with enumerators.
-        internal static readonly List<NetworkIdentity> dirtySpawned =
-            new List<NetworkIdentity>();
+        internal static readonly HashSet<NetworkIdentity> dirtySpawned =
+            new HashSet<NetworkIdentity>();
 
         /// <summary>Single player mode can use dontListen to not accept incoming connections</summary>
         // see also: https://github.com/vis2k/Mirror/pull/2595
@@ -1665,6 +1662,7 @@ static NetworkWriter SerializeForConnection(
             return null;
         }
 
+        static readonly List<NetworkIdentity> dirtyRemoved = new List<NetworkIdentity>();
         static void BroadcastDirtySpawned()
         {
             // PULL-Broadcasting vs. PUSH-Broadcasting:
@@ -1691,10 +1689,8 @@ static void BroadcastDirtySpawned()
                 // only iterate NetworkIdentities which we know to be dirty.
                 // for example, in an MMO we don't need to iterate NPCs,
                 // item drops, idle monsters etc. every Broadcast.
-                for (int i = 0; i < dirtySpawned.Count; ++i)
+                foreach (NetworkIdentity identity in dirtySpawned)
                 {
-                    NetworkIdentity identity = dirtySpawned[i];
-
                     // make sure it's not null or destroyed.
                     // (which can happen if someone uses
                     //  GameObject.Destroy instead of
@@ -1741,8 +1737,7 @@ static void BroadcastDirtySpawned()
                                 // List.RemoveAt(i) is O(N).
                                 // instead, use O(1) swap-remove from Rust.
                                 // dirtySpawned.RemoveAt(i);
-
-                                dirtySpawned.SwapRemove(i);
+                                dirtyRemoved.Add(identity);
 
                                 // the last element was moved to 'i'.
                                 // count was reduced by 1.
@@ -1757,6 +1752,11 @@ static void BroadcastDirtySpawned()
                     else Debug.LogWarning($"Found 'null' entry in dirtySpawned. Please call NetworkServer.Destroy to destroy networked objects. Don't use GameObject.Destroy.");
                 }
             }
+
+            // safely remove after iterating
+            foreach (NetworkIdentity identity in dirtyRemoved)
+                dirtySpawned.Remove(identity);
+            dirtyRemoved.Clear();
         }
 
         // helper function to check a connection for inactivity and disconnect if necessary

Assets/Mirror/Core/Tools/Extensions.cs

@@ -60,20 +60,5 @@ public static bool TryDequeue<T>(this Queue<T> source, out T element)
             return false;
         }
 #endif
-
-        // List.RemoveAt is O(N).
-        // implement Rust's swap-remove O(1) removal technique.
-        public static void SwapRemove<T>(this List<T> list, int index)
-        {
-            // we can only swap if we have at least two elements
-            if (list.Count >= 2)
-            {
-                // copy last element to index
-                list[index] = list[list.Count - 1];
-            }
-
-            // remove last element
-            list.RemoveAt(list.Count - 1);
-        }
     }
 }