diff --git a/Assets/Mirror/Core/NetworkBehaviour.cs b/Assets/Mirror/Core/NetworkBehaviour.cs index e189d98ab..888430b37 100644 --- a/Assets/Mirror/Core/NetworkBehaviour.cs +++ b/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) diff --git a/Assets/Mirror/Core/NetworkIdentity.cs b/Assets/Mirror/Core/NetworkIdentity.cs index 06733b921..7ad6e7bf4 100644 --- a/Assets/Mirror/Core/NetworkIdentity.cs +++ b/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. diff --git a/Assets/Mirror/Core/NetworkServer.cs b/Assets/Mirror/Core/NetworkServer.cs index f1693a41d..2dace1b3d 100644 --- a/Assets/Mirror/Core/NetworkServer.cs +++ b/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 here, faster than a Dictionary with enumerators. - internal static readonly List dirtySpawned = - new List(); + internal static readonly HashSet dirtySpawned = + new HashSet(); /// Single player mode can use dontListen to not accept incoming connections // see also: https://github.com/vis2k/Mirror/pull/2595 @@ -1665,6 +1662,7 @@ static NetworkWriter SerializeForConnection( return null; } + static readonly List dirtyRemoved = new List(); 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 diff --git a/Assets/Mirror/Core/Tools/Extensions.cs b/Assets/Mirror/Core/Tools/Extensions.cs index 3093f5892..02407e09f 100644 --- a/Assets/Mirror/Core/Tools/Extensions.cs +++ b/Assets/Mirror/Core/Tools/Extensions.cs @@ -60,20 +60,5 @@ public static bool TryDequeue(this Queue 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(this List 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); - } } }