perf: remove O(N) ClearDirtyComponentsDirtyBits calls in NetworkServer.Broadcast() and NetworkClient.Broadcast() (#3575)

* perf: remove O(N) ClearDirtyComponentsDirtyBits calls in NetworkServer.Broadcast() and NetworkClient.Broadcast()

* only for delta [imer]

Assets/Mirror/Core/NetworkClient.cs

@@ -1458,9 +1458,6 @@ static void Broadcast()
                                 payload = writer.ToArraySegment()
                             };
                             Send(message);
-
-                            // reset dirty bits so it's not resent next time.
-                            identity.ClearDirtyComponentsDirtyBits();
                         }
                     }
                 }

Assets/Mirror/Core/NetworkIdentity.cs

@@ -960,6 +960,19 @@ internal void SerializeServer(bool initialState, NetworkWriter ownerWriter, Netw
                             if (ownerDirty) ownerWriter.WriteBytes(segment.Array, segment.Offset, segment.Count);
                             if (observersDirty) observersWriter.WriteBytes(segment.Array, segment.Offset, segment.Count);
                         }
+
+                        // clear dirty bits for the components that we serialized.
+                        // do not clear for _all_ components, only the ones that
+                        // were dirty and had their syncInterval elapsed.
+                        //
+                        // we don't want to clear bits before the syncInterval
+                        // was elapsed, as then they wouldn't be synced.
+                        //
+                        // only clear for delta, not for full (spawn messages).
+                        // otherwise if a player joins, we serialize monster,
+                        // and shouldn't clear dirty bits not yet synced to
+                        // other players.
+                        if (!initialState) comp.ClearAllDirtyBits();
                     }
                 }
             }
@@ -1009,6 +1022,14 @@ internal void SerializeClient(NetworkWriter writer)
                         // serialize into writer.
                         // server always knows initialState, we never need to send it
                         comp.Serialize(writer, false);
+
+                        // clear dirty bits for the components that we serialized.
+                        // do not clear for _all_ components, only the ones that
+                        // were dirty and had their syncInterval elapsed.
+                        //
+                        // we don't want to clear bits before the syncInterval
+                        // was elapsed, as then they wouldn't be synced.
+                        comp.ClearAllDirtyBits();
                     }
                 }
             }
@@ -1107,27 +1128,6 @@ internal NetworkIdentitySerialization GetServerSerializationAtTick(int tick)
                                 lastSerialization.ownerWriter,
                                 lastSerialization.observersWriter);
 
-                // clear dirty bits for the components that we serialized.
-                // previously we did this in NetworkServer.BroadcastToConnection
-                // for every connection, for every entity.
-                // but we only serialize each entity once, right here in this
-                // 'lastSerialization.tick != tick' scope.
-                // so only do it once.
-                //
-                // NOTE: not in SerializeAll as that should only do one
-                //       thing: serialize data.
-                //
-                //
-                // NOTE: DO NOT clear ALL component's dirty bits, because
-                //       components can have different syncIntervals and we
-                //       don't want to reset dirty bits for the ones that were
-                //       not synced yet.
-                //
-                // NOTE: this used to be very important to avoid ever growing
-                //       SyncList changes if they had no observers, but we've
-                //       added SyncObject.isRecording since.
-                ClearDirtyComponentsDirtyBits();
-
                 // set tick
                 lastSerialization.tick = tick;
                 //Debug.Log($"{name} (netId={netId}) serialized for tick={tickTimeStamp}");
@@ -1137,23 +1137,6 @@ internal NetworkIdentitySerialization GetServerSerializationAtTick(int tick)
             return lastSerialization;
         }
 
-        // Clear only dirty component's dirty bits. ignores components which
-        // may be dirty but not ready to be synced yet (because of syncInterval)
-        //
-        // NOTE: this used to be very important to avoid ever
-        //       growing SyncList changes if they had no observers,
-        //       but we've added SyncObject.isRecording since.
-        internal void ClearDirtyComponentsDirtyBits()
-        {
-            foreach (NetworkBehaviour comp in NetworkBehaviours)
-            {
-                if (comp.IsDirty())
-                {
-                    comp.ClearAllDirtyBits();
-                }
-            }
-        }
-
         internal void AddObserver(NetworkConnectionToClient conn)
         {
             if (observers.ContainsKey(conn.connectionId))

Assets/Mirror/Tests/Editor/NetworkIdentity/NetworkIdentityTests.cs

@@ -591,38 +591,6 @@ public void ClearObservers()
             Assert.That(identity.observers.Count, Is.EqualTo(0));
         }
 
-        [Test]
-        public void ClearDirtyComponentsDirtyBits()
-        {
-            CreateNetworked(out GameObject _, out NetworkIdentity identity,
-                out NetworkBehaviourMock compA,
-                out NetworkBehaviourMock compB);
-
-            // set syncintervals so one is always dirty, one is never dirty
-            compA.syncInterval = 0;
-            compB.syncInterval = Mathf.Infinity;
-
-            // set components dirty bits
-            compA.SetSyncVarDirtyBit(0x0001);
-            compB.SetSyncVarDirtyBit(0x1001);
-            // dirty because interval reached and mask != 0
-            Assert.That(compA.IsDirty(), Is.True);
-            // not dirty because syncinterval not reached
-            Assert.That(compB.IsDirty(), Is.False);
-
-            // call identity.ClearDirtyComponentsDirtyBits
-            identity.ClearDirtyComponentsDirtyBits();
-            // should be cleared now
-            Assert.That(compA.IsDirty(), Is.False);
-            // should be untouched
-            Assert.That(compB.IsDirty(), Is.False);
-
-            // set compB syncinterval to 0 to check if the masks were untouched
-            // (if they weren't, then it should be dirty now)
-            compB.syncInterval = 0;
-            Assert.That(compB.IsDirty(), Is.True);
-        }
-
         [Test]
         public void ClearAllComponentsDirtyBits()
         {

Assets/Mirror/Tests/Editor/SyncVars/SyncVarAttributeTest.cs

@@ -123,32 +123,6 @@ public void TestSettingStruct()
             Assert.That(player.IsDirty(), "Clearing struct should mark object as dirty");
         }
 
-        [Test]
-        public void TestSyncIntervalAndClearDirtyComponents()
-        {
-            CreateNetworked(out _, out _, out MockPlayer player);
-            player.lastSyncTime = NetworkTime.localTime;
-            // synchronize immediately
-            player.syncInterval = 1f;
-
-            player.guild = new MockPlayer.Guild
-            {
-                name = "Back street boys"
-            };
-
-            Assert.That(player.IsDirty(), Is.False, "Sync interval not met, so not dirty yet");
-
-            // ClearDirtyComponents should do nothing since syncInterval is not
-            // elapsed yet
-            player.netIdentity.ClearDirtyComponentsDirtyBits();
-
-            // set lastSyncTime far enough back to be ready for syncing
-            player.lastSyncTime = NetworkTime.localTime - player.syncInterval;
-
-            // should be dirty now
-            Assert.That(player.IsDirty(), Is.True, "Sync interval met, should be dirty");
-        }
-
         [Test]
         public void TestSyncIntervalAndClearAllComponents()
         {