perf: push->pull broadcasting part 1: feature parity

Assets/Mirror/Runtime/NetworkIdentity.cs

@@ -260,7 +260,6 @@ public NetworkBehaviour[] NetworkBehaviours
             }
         }
 
-
 #pragma warning disable 618
         NetworkVisibility visibilityCache;
         [Obsolete(NetworkVisibilityObsoleteMessage.Message)]
@@ -1277,70 +1276,6 @@ internal void Reset()
             }
         }
 
-        /// <summary>
-        /// Invoked by NetworkServer.Update during LateUpdate
-        /// </summary>
-        internal void ServerUpdate()
-        {
-            if (observers != null && observers.Count > 0)
-            {
-                SendUpdateVarsMessage();
-            }
-            else
-            {
-                // clear all component's dirty bits.
-                // it would be spawned on new observers anyway.
-                ClearAllComponentsDirtyBits();
-            }
-        }
-
-        void SendUpdateVarsMessage()
-        {
-            // one writer for owner, one for observers
-            using (PooledNetworkWriter ownerWriter = NetworkWriterPool.GetWriter(), observersWriter = NetworkWriterPool.GetWriter())
-            {
-                // serialize all the dirty components and send
-                OnSerializeAllSafely(false, ownerWriter, out int ownerWritten, observersWriter, out int observersWritten);
-                if (ownerWritten > 0 || observersWritten > 0)
-                {
-                    UpdateVarsMessage varsMessage = new UpdateVarsMessage
-                    {
-                        netId = netId
-                    };
-
-                    // send ownerWriter to owner
-                    // (only if we serialized anything for owner)
-                    // (only if there is a connection (e.g. if not a monster),
-                    //  and if connection is ready because we use SendToReady
-                    //  below too)
-                    if (ownerWritten > 0)
-                    {
-                        varsMessage.payload = ownerWriter.ToArraySegment();
-                        if (connectionToClient != null && connectionToClient.isReady)
-                            NetworkServer.SendToClientOfPlayer(this, varsMessage);
-                    }
-
-                    // send observersWriter to everyone but owner
-                    // (only if we serialized anything for observers)
-                    if (observersWritten > 0)
-                    {
-                        varsMessage.payload = observersWriter.ToArraySegment();
-                        NetworkServer.SendToReady(this, varsMessage, false);
-                    }
-
-                    // clear dirty bits only for the components that we serialized
-                    // DO NOT clean 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.
-                    // (we serialized only the IsDirty() components, or all of
-                    //  them if initialState. clearing the dirty ones is enough.)
-                    ClearDirtyComponentsDirtyBits();
-                }
-            }
-        }
-
-
         /// <summary>
         /// clear all component's dirty bits no matter what
         /// </summary>

Assets/Mirror/Runtime/NetworkServer.cs

@@ -459,6 +459,19 @@ internal static void NetworkEarlyUpdate()
                 Transport.activeTransport.ServerEarlyUpdate();
         }
 
+        // cache NetworkIdentity serializations
+        // Update() shouldn't serialize multiple times for multiple connections
+        struct Serialization
+        {
+            public PooledNetworkWriter ownerWriter;
+            public PooledNetworkWriter observersWriter;
+            // TODO there is probably a more simple way later
+            public int ownerWritten;
+            public int observersWritten;
+        }
+        static Dictionary<NetworkIdentity, Serialization> serializations =
+            new Dictionary<NetworkIdentity, Serialization>();
+
         // NetworkLateUpdate called after any Update/FixedUpdate/LateUpdate
         // (we add this to the UnityEngine in NetworkLoop)
         internal static void NetworkLateUpdate()
@@ -468,22 +481,137 @@ internal static void NetworkLateUpdate()
             {
                 // Check for dead clients but exclude the host client because it
                 // doesn't ping itself and therefore may appear inactive.
+                // TODO move this into the below foreach
                 CheckForInactiveConnections();
 
-                // update all server objects
+                // for each READY connection:
-                foreach (KeyValuePair<uint, NetworkIdentity> kvp in NetworkIdentity.spawned)
+                //   pull in UpdateVarsMessage for each entity it observes
+                foreach (NetworkConnectionToClient conn in connections.Values)
                 {
-                    NetworkIdentity identity = kvp.Value;
+                    // only if this connection has joined the world yet
+                    if (conn.isReady)
+                    {
+                        // for each entity that this connection is seeing
+                        foreach (NetworkIdentity identity in conn.observing)
+                        {
+                            // make sure it's not null or destroyed.
+                            // (which can happen if someone uses
+                            //  GameObject.Destroy instead of
+                            //  NetworkServer.Destroy)
                             if (identity != null)
                             {
-                        identity.ServerUpdate();
+                                // multiple connections might be observed by the
+                                // same NetworkIdentity, but we don't want to
+                                // serialize them multiple times. look it up first.
+                                //
+                                // IMPORTANT: don't forget to return them to pool!
+                                // TODO make this easier later. for now aim for
+                                //      feature parity to not break projects.
+                                if (!serializations.ContainsKey(identity))
+                                {
+                                    // serialize all the dirty components.
+                                    // one version for owner, one for observers.
+                                    PooledNetworkWriter ownerWriter = NetworkWriterPool.GetWriter();
+                                    PooledNetworkWriter observersWriter = NetworkWriterPool.GetWriter();
+                                    identity.OnSerializeAllSafely(false, ownerWriter, out int ownerWritten, observersWriter, out int observersWritten);
+                                    serializations[identity] = new Serialization
+                                    {
+                                        ownerWriter = ownerWriter,
+                                        observersWriter = observersWriter,
+                                        ownerWritten = ownerWritten,
+                                        observersWritten = observersWritten
+                                    };
+
+                                    // clear dirty bits only for the components that we serialized
+                                    // DO NOT clean 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.
+                                    // (we serialized only the IsDirty() components, or all of
+                                    //  them if initialState. clearing the dirty ones is enough.)
+                                    //
+                                    // NOTE: this is what we did before push->pull
+                                    //       broadcasting. let's keep doing this for
+                                    //       feature parity to not break anyone's project.
+                                    //       TODO make this more simple / unnecessary later.
+                                    identity.ClearDirtyComponentsDirtyBits();
+                                }
+
+                                // get serialization
+                                Serialization serialization = serializations[identity];
+
+                                // is this entity owned by this connection?
+                                bool owned = identity.connectionToClient == conn;
+
+                                // send serialized data
+                                // owner writer if owned
+                                if (owned)
+                                {
+                                    // was it dirty / did we actually serialize anything?
+                                    if (serialization.ownerWritten > 0)
+                                    {
+                                        UpdateVarsMessage message = new UpdateVarsMessage
+                                        {
+                                            netId = identity.netId,
+                                            payload = serialization.ownerWriter.ToArraySegment()
+                                        };
+                                        conn.Send(message);
+                                    }
+                                }
+                                // observers writer if not owned
+                                else
+                                {
+                                    // was it dirty / did we actually serialize anything?
+                                    if (serialization.observersWritten > 0)
+                                    {
+                                        UpdateVarsMessage message = new UpdateVarsMessage
+                                        {
+                                            netId = identity.netId,
+                                            payload = serialization.observersWriter.ToArraySegment()
+                                        };
+                                        conn.Send(message);
+                                    }
+                                }
                             }
                             // spawned list should have no null entries because we
                             // always call Remove in OnObjectDestroy everywhere.
-                    else Debug.LogWarning("Found 'null' entry in spawned list for netId=" + kvp.Key + ". Please call NetworkServer.Destroy to destroy networked objects. Don't use GameObject.Destroy.");
+                            // if it does have null then someone used
+                            // GameObject.Destroy instead of NetworkServer.Destroy.
+                            else Debug.LogWarning("Found 'null' entry in observing list for connectionId=" + conn.connectionId + ". Please call NetworkServer.Destroy to destroy networked objects. Don't use GameObject.Destroy.");
+                        }
+                    }
+                }
+
+                // return serialized writers to pool, clear set
+                // TODO this is for feature parity before push->pull change.
+                //      make this more simple / unnecessary later.
+                foreach (Serialization entry in serializations.Values)
+                {
+                    NetworkWriterPool.Recycle(entry.ownerWriter);
+                    NetworkWriterPool.Recycle(entry.observersWriter);
+                }
+                serializations.Clear();
+
+                // TODO this unfortunately means we still need to iterate ALL
+                //      spawned and not just the ones with observers. figure
+                //      out a way to get rid of this.
+                //
+                // for each spawned:
+                //   clear dirty bits if it has no observers.
+                //   we did this before push->pull broadcasting so let's keep
+                //   doing this for now.
+                foreach (NetworkIdentity identity in NetworkIdentity.spawned.Values)
+                {
+                    if (identity.observers == null || identity.observers.Count == 0)
+                    {
+                        // clear all component's dirty bits.
+                        // it would be spawned on new observers anyway.
+                        identity.ClearAllComponentsDirtyBits();
+                    }
                 }
 
                 // update all connections to send out batched messages in interval
+                // TODO move this into the above foreach
                 foreach (NetworkConnectionToClient conn in connections.Values)
                 {
                     conn.Update();

Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs

@@ -1224,83 +1224,5 @@ public void HandleRpc()
             NetworkIdentity.spawned.Clear();
             RemoteCallHelper.RemoveDelegate(registeredHash);
         }
-
-
-        [Test]
-        public void ServerUpdate()
-        {
-            // add components
-            SerializeTest1NetworkBehaviour compA = gameObject.AddComponent<SerializeTest1NetworkBehaviour>();
-            // test value
-            compA.value = 1337;
-            // set syncInterval so IsDirty passes the interval check
-            compA.syncInterval = 0;
-            // one needs to sync to owner
-            compA.syncMode = SyncMode.Owner;
-            SerializeTest2NetworkBehaviour compB = gameObject.AddComponent<SerializeTest2NetworkBehaviour>();
-            // test value
-            compB.value = "test";
-            // set syncInterval so IsDirty passes the interval check
-            compB.syncInterval = 0;
-            // one needs to sync to owner
-            compB.syncMode = SyncMode.Observers;
-
-            // call OnStartServer once so observers are created
-            identity.OnStartServer();
-
-            // set it dirty
-            compA.SetDirtyBit(ulong.MaxValue);
-            compB.SetDirtyBit(ulong.MaxValue);
-            Assert.That(compA.IsDirty(), Is.True);
-            Assert.That(compB.IsDirty(), Is.True);
-
-            // calling update without observers should clear all dirty bits.
-            // it would be spawned on new observers anyway.
-            identity.ServerUpdate();
-            Assert.That(compA.IsDirty(), Is.False);
-            Assert.That(compB.IsDirty(), Is.False);
-
-            // add an owner connection that will receive the updates
-            LocalConnectionToClient owner = new LocalConnectionToClient();
-            // for syncing
-            owner.isReady = true;
-            // add a client to server connection + handler to receive syncs
-            owner.connectionToServer = new LocalConnectionToServer();
-            int ownerCalled = 0;
-            owner.connectionToServer.SetHandlers(new Dictionary<int, NetworkMessageDelegate>
-            {
-                { MessagePacking.GetId<UpdateVarsMessage>(), ((conn, reader, channelId) => ++ownerCalled) }
-            });
-            identity.connectionToClient = owner;
-
-            // add an observer connection that will receive the updates
-            LocalConnectionToClient observer = new LocalConnectionToClient();
-            // we only sync to ready observers
-            observer.isReady = true;
-            // add a client to server connection + handler to receive syncs
-            observer.connectionToServer = new LocalConnectionToServer();
-            int observerCalled = 0;
-            observer.connectionToServer.SetHandlers(new Dictionary<int, NetworkMessageDelegate>
-            {
-                { MessagePacking.GetId<UpdateVarsMessage>(), ((conn, reader, channelId) => ++observerCalled) }
-            });
-            identity.observers[observer.connectionId] = observer;
-
-            // set components dirty again
-            compA.SetDirtyBit(ulong.MaxValue);
-            compB.SetDirtyBit(ulong.MaxValue);
-
-            // calling update should serialize all components and send them to
-            // owner/observers
-            identity.ServerUpdate();
-
-            // update connections once so that messages are processed
-            owner.connectionToServer.Update();
-            observer.connectionToServer.Update();
-
-            // was it received on the clients?
-            Assert.That(ownerCalled, Is.EqualTo(1));
-            Assert.That(observerCalled, Is.EqualTo(1));
-        }
     }
 }

Assets/Mirror/Tests/Editor/NetworkServerTest.cs

@@ -1108,42 +1108,47 @@ public void NoConnectionsTest_WithHostAndConnection()
             NetworkServer.RemoveLocalConnection();
         }
 
-        // updating NetworkServer with a null entry in NetworkIdentity.spawned
+        // updating NetworkServer with a null entry in connection.observing
-        // should log a warning.
+        // should log a warning. someone probably used GameObject.Destroy
+        // instead of NetworkServer.Destroy.
         [Test]
-        public void UpdateDetectsNullEntryInSpawned()
+        public void UpdateDetectsNullEntryInObserving()
         {
             // start
             NetworkServer.Listen(1);
 
-            // add null
+            // add a connection that is observed by a null entity
-            NetworkIdentity.spawned[42] = null;
+            NetworkServer.connections[42] = new FakeNetworkConnection{isReady=true};
+            NetworkServer.connections[42].observing.Add(null);
 
             // update
-            LogAssert.Expect(LogType.Warning, new Regex("Found 'null' entry in spawned list.*"));
+            LogAssert.Expect(LogType.Warning, new Regex("Found 'null' entry in observing list.*"));
             NetworkServer.NetworkLateUpdate();
 
             // clean up
             NetworkServer.Shutdown();
         }
 
-        // updating NetworkServer with a null entry in NetworkIdentity.spawned
+        // updating NetworkServer with a null entry in connection.observing
-        // should log a warning.
+        // should log a warning. someone probably used GameObject.Destroy
+        // instead of NetworkServer.Destroy.
+        //
         // => need extra test because of Unity's custom null check
         [Test]
-        public void UpdateDetectsDestroyedEntryInSpawned()
+        public void UpdateDetectsDestroyedEntryInObserving()
         {
             // start
             NetworkServer.Listen(1);
 
-            // add destroyed
+            // add a connection that is observed by a destroyed entity
             GameObject go = new GameObject();
             NetworkIdentity ni = go.AddComponent<NetworkIdentity>();
-            NetworkIdentity.spawned[42] = ni;
+            NetworkServer.connections[42] = new FakeNetworkConnection{isReady=true};
+            NetworkServer.connections[42].observing.Add(ni);
             GameObject.DestroyImmediate(go);
 
             // update
-            LogAssert.Expect(LogType.Warning, new Regex("Found 'null' entry in spawned list.*"));
+            LogAssert.Expect(LogType.Warning, new Regex("Found 'null' entry in observing list.*"));
             NetworkServer.NetworkLateUpdate();
 
             // clean up

Assets/Mirror/Tests/Performance/Editor/NetworkIdentityPerformance.cs

@@ -1,90 +0,0 @@
-#if !UNITY_2019_2_OR_NEWER || UNITY_PERFORMANCE_TESTS_1_OR_OLDER
-using NUnit.Framework;
-using Unity.PerformanceTesting;
-using UnityEngine;
-
-namespace Mirror.Tests.Performance
-{
-    public class Health : NetworkBehaviour
-    {
-        [SyncVar] public int health = 10;
-
-        public void Update()
-        {
-            health = (health + 1) % 10;
-        }
-    }
-    [Category("Performance")]
-    [Category("Benchmark")]
-    public class NetworkIdentityPerformance
-    {
-        GameObject gameObject;
-        NetworkIdentity identity;
-        Health health;
-
-
-        [SetUp]
-        public void SetUp()
-        {
-            gameObject = new GameObject();
-            identity = gameObject.AddComponent<NetworkIdentity>();
-            identity.observers = new System.Collections.Generic.Dictionary<int, NetworkConnection>();
-            identity.connectionToClient = new FakeNetworkConnection(1);
-            identity.observers.Add(1, identity.connectionToClient);
-            health = gameObject.AddComponent<Health>();
-            health.syncMode = SyncMode.Owner;
-            health.syncInterval = 0f;
-        }
-        [TearDown]
-        public void TearDown()
-        {
-            UnityEngine.Object.DestroyImmediate(gameObject);
-        }
-
-        [Test]
-#if UNITY_2019_2_OR_NEWER
-        [Performance]
-#else
-        [PerformanceTest]
-#endif
-        public void NetworkIdentityServerUpdateIsDirty()
-        {
-            Measure.Method(RunServerUpdateIsDirty)
-                .WarmupCount(10)
-                .MeasurementCount(100)
-                .Run();
-        }
-
-        void RunServerUpdateIsDirty()
-        {
-            for (int j = 0; j < 10000; j++)
-            {
-                health.SetDirtyBit(1UL);
-                identity.ServerUpdate();
-            }
-        }
-
-        [Test]
-#if UNITY_2019_2_OR_NEWER
-        [Performance]
-#else
-        [PerformanceTest]
-#endif
-        public void NetworkIdentityServerUpdateNotDirty()
-        {
-            Measure.Method(RunServerUpdateNotDirty)
-                .WarmupCount(10)
-                .MeasurementCount(100)
-                .Run();
-        }
-
-        void RunServerUpdateNotDirty()
-        {
-            for (int j = 0; j < 10000; j++)
-            {
-                identity.ServerUpdate();
-            }
-        }
-    }
-}
-#endif

Assets/Mirror/Tests/Performance/Editor/NetworkIdentityPerformance.cs.meta

@@ -1,11 +0,0 @@
-fileFormatVersion: 2
-guid: 8bf0e9ffea6328a42aac1c6bbfe410ec
-MonoImporter:
-  externalObjects: {}
-  serializedVersion: 2
-  defaultReferences: []
-  executionOrder: 0
-  icon: {instanceID: 0}
-  userData: 
-  assetBundleName: 
-  assetBundleVariant: 

Assets/Mirror/Tests/Performance/Editor/NetworkIdentityPerformanceWithMultipleBehaviour.cs

@@ -1,89 +0,0 @@
-#if !UNITY_2019_2_OR_NEWER || UNITY_PERFORMANCE_TESTS_1_OR_OLDER
-using NUnit.Framework;
-using Unity.PerformanceTesting;
-using UnityEngine;
-
-namespace Mirror.Tests.Performance
-{
-    [Category("Performance")]
-    [Category("Benchmark")]
-    public class NetworkIdentityPerformanceWithMultipleBehaviour
-    {
-        const int healthCount = 32;
-        GameObject gameObject;
-        NetworkIdentity identity;
-        Health[] health;
-
-
-        [SetUp]
-        public void SetUp()
-        {
-            gameObject = new GameObject();
-            identity = gameObject.AddComponent<NetworkIdentity>();
-            identity.observers = new System.Collections.Generic.Dictionary<int, NetworkConnection>();
-            identity.connectionToClient = new FakeNetworkConnection(1);
-            identity.observers.Add(1, identity.connectionToClient);
-            health = new Health[healthCount];
-            for (int i = 0; i < healthCount; i++)
-            {
-                health[i] = gameObject.AddComponent<Health>();
-                health[i].syncMode = SyncMode.Owner;
-                health[i].syncInterval = 0f;
-            }
-        }
-        [TearDown]
-        public void TearDown()
-        {
-            UnityEngine.Object.DestroyImmediate(gameObject);
-        }
-
-        [Test]
-#if UNITY_2019_2_OR_NEWER
-        [Performance]
-#else
-        [PerformanceTest]
-#endif
-        public void ServerUpdateIsDirty()
-        {
-            Measure.Method(RunServerUpdateIsDirty)
-                .WarmupCount(10)
-                .MeasurementCount(100)
-                .Run();
-        }
-
-        void RunServerUpdateIsDirty()
-        {
-            for (int j = 0; j < 10000; j++)
-            {
-                for (int i = 0; i < healthCount; i++)
-                {
-                    health[i].SetDirtyBit(1UL);
-                }
-                identity.ServerUpdate();
-            }
-        }
-
-        [Test]
-#if UNITY_2019_2_OR_NEWER
-        [Performance]
-#else
-        [PerformanceTest]
-#endif
-        public void ServerUpdateNotDirty()
-        {
-            Measure.Method(RunServerUpdateNotDirty)
-                .WarmupCount(10)
-                .MeasurementCount(100)
-                .Run();
-        }
-
-        void RunServerUpdateNotDirty()
-        {
-            for (int j = 0; j < 10000; j++)
-            {
-                identity.ServerUpdate();
-            }
-        }
-    }
-}
-#endif

Assets/Mirror/Tests/Performance/Editor/NetworkIdentityPerformanceWithMultipleBehaviour.cs.meta

@@ -1,11 +0,0 @@
-fileFormatVersion: 2
-guid: f690766cb23aca74d86925a64b233ca1
-MonoImporter:
-  externalObjects: {}
-  serializedVersion: 2
-  defaultReferences: []
-  executionOrder: 0
-  icon: {instanceID: 0}
-  userData: 
-  assetBundleName: 
-  assetBundleVariant: