From 98b6ba0a259556a87ec27fdca6895ba5a76e1fce Mon Sep 17 00:00:00 2001 From: vis2k Date: Sun, 13 Nov 2022 18:20:39 +0100 Subject: [PATCH] perf: NetworkServer.dirtySpawned via NetworkIdentity.OnBecameDirty callback --- Assets/Mirror/Core/NetworkIdentity.cs | 13 ++ Assets/Mirror/Core/NetworkServer.cs | 174 ++++++++++-------- .../Mirror/Tests/Editor/NetworkServerTest.cs | 10 +- 3 files changed, 120 insertions(+), 77 deletions(-) diff --git a/Assets/Mirror/Core/NetworkIdentity.cs b/Assets/Mirror/Core/NetworkIdentity.cs index 77c488144..7c0920da2 100644 --- a/Assets/Mirror/Core/NetworkIdentity.cs +++ b/Assets/Mirror/Core/NetworkIdentity.cs @@ -852,6 +852,8 @@ internal void OnBecameDirty(NetworkBehaviour component) // OnSerialize will decide how to use them. if (isServer) { + bool clean = (serverOwnerDirtyMask | serverObserversDirtyMask) == 0; + // owner needs to be considered for both SyncModes, because // Observers mode always includes the Owner. // @@ -869,6 +871,14 @@ internal void OnBecameDirty(NetworkBehaviour component) // observers which aren't the owner. if (component.syncMode == SyncMode.Observers) serverObserversDirtyMask |= nthBit; + + // add to dirty spawned, but only when we get dirty the first time. + // don't do a dictionary lookup for every [SyncVar] change... + + // only add if observed. + // otherwise no point in adding + iterating from broadcast. + if (clean && observers.Count > 0) + NetworkServer.dirtySpawned[netId] = this; } if (isClient) { @@ -885,6 +895,9 @@ internal void OnBecameDirty(NetworkBehaviour component) // set the n-th bit if dirty // shifting from small to large numbers is varint-efficient. clientDirtyMask |= (1u << component.ComponentIndex); + + // note: we don't have NetworkClient.dirtySpawned. + // checking the few owned objects is enough. } } } diff --git a/Assets/Mirror/Core/NetworkServer.cs b/Assets/Mirror/Core/NetworkServer.cs index fb33382bc..bac582bac 100644 --- a/Assets/Mirror/Core/NetworkServer.cs +++ b/Assets/Mirror/Core/NetworkServer.cs @@ -51,6 +51,10 @@ public static partial class NetworkServer public static readonly Dictionary spawned = new Dictionary(); + // all spawned which are dirty (= need broadcasting). + internal static readonly Dictionary dirtySpawned = + new Dictionary(); + /// Single player mode can use dontListen to not accept incoming connections // see also: https://github.com/vis2k/Mirror/pull/2595 public static bool dontListen; @@ -91,6 +95,11 @@ public static partial class NetworkServer // capture full Unity update time from before Early- to after LateUpdate public static TimeSample fullUpdateDuration; + // cache broadcast writers. + // don't need to get/return from/to pool for every identity. + static NetworkWriter ownerWriter = new NetworkWriter(); + static NetworkWriter observersWriter = new NetworkWriter(); + // initialization / shutdown /////////////////////////////////////////// static void Initialize() { @@ -1709,6 +1718,93 @@ static NetworkWriter SerializeForConnection(NetworkConnectionToClient connection internal static readonly List connectionsCopy = new List(); + // if we have large worlds with 10k+ Npcs, Items, etc. + // then we can easily ignore all of those which didn't change. + // otherwise this loop would be very costly. + static void BroadcastDirty() + { + // if (dirtySpawned.Count > 0) Debug.Log($"NetworkServer: Broadcasting {dirtySpawned.Count} dirty out of {spawned.Count} spawned."); + + foreach (NetworkIdentity identity in dirtySpawned.Values) + { + // make sure it's not null or destroyed. + // (which can happen if someone uses + // GameObject.Destroy instead of + // NetworkServer.Destroy) + if (identity != null) + { + // only serialize if it has any observers. + // this is checked in NetworkIdentity.OnBecameDirty too. + // however, a connection may have disconnected since then. + if (identity.observers.Count > 0) + BroadcastIdentity(identity); + } + // spawned list should have no null entries because we + // always call Remove in OnObjectDestroy everywhere. + // if it does have null then someone used + // GameObject.Destroy instead of NetworkServer.Destroy. + else Debug.LogWarning($"Found 'null' entry in spawned. Please call NetworkServer.Destroy to destroy networked objects. Don't use GameObject.Destroy."); + } + } + + static void BroadcastIdentity(NetworkIdentity identity) + { + // serialize for owner & observers + ownerWriter.Position = 0; + observersWriter.Position = 0; + identity.SerializeServer(false, ownerWriter, observersWriter); + + // broadcast to each observer connection + foreach (NetworkConnectionToClient connection in identity.observers.Values) + { + // has this connection joined the world yet? + if (connection.isReady) + { + // is this entity owned by this connection? + bool owned = identity.connectionToClient == connection; + + // get serialization for this entity viewed by this connection + // (if anything was serialized this time) + NetworkWriter serialization = SerializeForConnection(connection, owned, ownerWriter, observersWriter); + if (serialization != null) + { + EntityStateMessage message = new EntityStateMessage + { + netId = identity.netId, + payload = serialization.ToArraySegment() + }; + connection.Send(message); + } + } + } + } + + static void FlushConnections() + { + foreach (NetworkConnectionToClient connection in connectionsCopy) + { + // has this connection joined the world yet? + // for each READY connection: + // pull in UpdateVarsMessage for each entity it observes + if (connection.isReady) + { + // send time for snapshot interpolation every sendInterval. + // BroadcastToConnection() may not send if nothing is new. + // + // sent over unreliable. + // NetworkTime / Transform both use unreliable. + // + // make sure Broadcast() is only called every sendInterval, + // even if targetFrameRate isn't set in host mode (!) + // (done via AccurateInterval) + connection.Send(new TimeSnapshotMessage(), Channels.Unreliable); + } + + // update connection to flush out batched messages + connection.Update(); + } + } + static void Broadcast() { // copy all connections into a helper collection so that @@ -1739,83 +1835,15 @@ static void Broadcast() // - need to cache identity.Serialize() so it's only called once. // instead of once per observing. // - using (NetworkWriterPooled ownerWriter = NetworkWriterPool.Get(), - observersWriter = NetworkWriterPool.Get()) - { - // let's use push broadcasting to prepare for dirtyObjects. - foreach (NetworkIdentity identity in spawned.Values) - { - // make sure it's not null or destroyed. - // (which can happen if someone uses - // GameObject.Destroy instead of - // NetworkServer.Destroy) - if (identity != null) - { - // only serialize if it has any observers - // TODO only set dirty if has observers? would be easiest. - if (identity.observers.Count > 0) - { - // serialize for owner & observers - ownerWriter.Position = 0; - observersWriter.Position = 0; - identity.SerializeServer(false, ownerWriter, observersWriter); - // broadcast to each observer connection - foreach (NetworkConnectionToClient connection in identity.observers.Values) - { - // has this connection joined the world yet? - if (connection.isReady) - { - // is this entity owned by this connection? - bool owned = identity.connectionToClient == connection; + // only broadcast dirty entities. + BroadcastDirty(); - // get serialization for this entity viewed by this connection - // (if anything was serialized this time) - NetworkWriter serialization = SerializeForConnection(connection, owned, ownerWriter, observersWriter); - if (serialization != null) - { - EntityStateMessage message = new EntityStateMessage - { - netId = identity.netId, - payload = serialization.ToArraySegment() - }; - connection.Send(message); - } - } - } - } - } - // spawned list should have no null entries because we - // always call Remove in OnObjectDestroy everywhere. - // if it does have null then someone used - // GameObject.Destroy instead of NetworkServer.Destroy. - else Debug.LogWarning($"Found 'null' entry in spawned. Please call NetworkServer.Destroy to destroy networked objects. Don't use GameObject.Destroy."); - } - } + // clear dirty spawned, now that all were broadcasted + dirtySpawned.Clear(); // flush all connection's batched messages - foreach (NetworkConnectionToClient connection in connectionsCopy) - { - // has this connection joined the world yet? - // for each READY connection: - // pull in UpdateVarsMessage for each entity it observes - if (connection.isReady) - { - // send time for snapshot interpolation every sendInterval. - // BroadcastToConnection() may not send if nothing is new. - // - // sent over unreliable. - // NetworkTime / Transform both use unreliable. - // - // make sure Broadcast() is only called every sendInterval, - // even if targetFrameRate isn't set in host mode (!) - // (done via AccurateInterval) - connection.Send(new TimeSnapshotMessage(), Channels.Unreliable); - } - - // update connection to flush out batched messages - connection.Update(); - } + FlushConnections(); // TODO this is way too slow because we iterate ALL spawned :/ // TODO this is way too complicated :/ diff --git a/Assets/Mirror/Tests/Editor/NetworkServerTest.cs b/Assets/Mirror/Tests/Editor/NetworkServerTest.cs index 61cb24506..60d5caaef 100644 --- a/Assets/Mirror/Tests/Editor/NetworkServerTest.cs +++ b/Assets/Mirror/Tests/Editor/NetworkServerTest.cs @@ -1251,17 +1251,18 @@ public void HasExternalConnections_WithHostAndConnection() // should log a warning. someone probably used GameObject.Destroy // instead of NetworkServer.Destroy. [Test] - public void UpdateDetectsNullEntryInObserving() + public void UpdateDetectsNullEntryInDirtySpawned() { // start NetworkServer.Listen(1); ConnectHostClientBlockingAuthenticatedAndReady(); CreateNetworkedAndSpawn(out GameObject go, out NetworkIdentity ni); + NetworkServer.dirtySpawned[ni.netId] = ni; // add manually Assert.That(NetworkServer.spawned.ContainsKey(ni.netId)); // set null - NetworkServer.spawned[ni.netId] = null; + NetworkServer.dirtySpawned[ni.netId] = null; // update LogAssert.Expect(LogType.Warning, new Regex("Found 'null' entry in spawned.*")); @@ -1274,14 +1275,15 @@ public void UpdateDetectsNullEntryInObserving() // // => need extra test because of Unity's custom null check [Test] - public void UpdateDetectsDestroyedSpawned() + public void UpdateDetectsDestroyedDirtySpawned() { // start NetworkServer.Listen(1); ConnectHostClientBlockingAuthenticatedAndReady(); CreateNetworkedAndSpawn(out GameObject go, out NetworkIdentity ni); - Assert.That(NetworkServer.spawned.ContainsKey(ni.netId)); + NetworkServer.dirtySpawned[ni.netId] = ni; // add manually + Assert.That(NetworkServer.dirtySpawned.ContainsKey(ni.netId)); // destroy GameObject.DestroyImmediate(go);