From 91b0d430297d9d8fd4574cb824cbc8c33eafbf04 Mon Sep 17 00:00:00 2001 From: vis2k Date: Sat, 12 Nov 2022 11:34:38 +0100 Subject: [PATCH] perf: NetworkIdentity dirty masks via callbacks instead of iteration --- Assets/Mirror/Core/NetworkBehaviour.cs | 13 +++ Assets/Mirror/Core/NetworkIdentity.cs | 90 +++++++++++-------- .../NetworkIdentitySerializationTests.cs | 45 +++++++--- 3 files changed, 99 insertions(+), 49 deletions(-) diff --git a/Assets/Mirror/Core/NetworkBehaviour.cs b/Assets/Mirror/Core/NetworkBehaviour.cs index 9202c4842..d87117fba 100644 --- a/Assets/Mirror/Core/NetworkBehaviour.cs +++ b/Assets/Mirror/Core/NetworkBehaviour.cs @@ -150,10 +150,21 @@ protected void SetSyncVarHookGuard(ulong dirtyBit, bool value) syncVarHookGuard &= ~dirtyBit; } + // callback for both SyncObject and SyncVar dirty bit setters. + // 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(this); + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] void SetSyncObjectDirtyBit(ulong dirtyBit) { + bool clean = syncObjectDirtyBits == 0; syncObjectDirtyBits |= dirtyBit; + if (clean) OnBecameDirty(); } /// Set as dirty so that it's synced to clients again. @@ -161,7 +172,9 @@ void SetSyncObjectDirtyBit(ulong dirtyBit) [MethodImpl(MethodImplOptions.AggressiveInlining)] public void SetSyncVarDirtyBit(ulong dirtyBit) { + bool clean = syncVarDirtyBits == 0; syncVarDirtyBits |= dirtyBit; + if (clean) OnBecameDirty(); } /// Set as dirty to trigger OnSerialize & send. Dirty bits are cleared after the send. diff --git a/Assets/Mirror/Core/NetworkIdentity.cs b/Assets/Mirror/Core/NetworkIdentity.cs index 3977454ba..af7851966 100644 --- a/Assets/Mirror/Core/NetworkIdentity.cs +++ b/Assets/Mirror/Core/NetworkIdentity.cs @@ -218,6 +218,14 @@ public static Dictionary spawned // which means we can't allow > 64 components (it's enough). const int MaxNetworkBehaviours = 64; + // NetworkBehaviour's dirty masks. + // set from NetworkBehaviour.OnBecameDirty callback. + ulong serverOwnerInitialMask; // all initial components' bits are set + ulong serverOwnerDirtyMask; // only dirty components' bits are set + ulong serverObserversDirtyMask; // all initial components' bits are set + ulong serverObserversInitialMask; // only dirty components' bits are set + ulong clientDirtyMask; // only dirty components. client doesn't send initial. + // current visibility // // Default = use interest management @@ -313,12 +321,30 @@ internal void InitializeNetworkBehaviours() NetworkBehaviours = GetComponents(); ValidateComponents(); + // reset the masks before initializing them. + // during tests, we may create an identity, then change sync mode, + // then call Awake() to initialize the masks properly. + // if we don't reset them first, then we OR into the first state. + serverOwnerInitialMask = serverObserversInitialMask = 0; + // initialize each one for (int i = 0; i < NetworkBehaviours.Length; ++i) { NetworkBehaviour component = NetworkBehaviours[i]; component.netIdentity = this; component.ComponentIndex = (byte)i; + + ulong nthBit = (1u << component.ComponentIndex); + + // build a mask with all owner components' bits set for initialState + // -> for initial, all components are synced to owner no matter what. + // -> so simply set a bit for every component index + serverOwnerInitialMask |= nthBit; + + // build a mask with all observer components' bits set initialState + // -> for initial, only SyncMode.Observers components are synced + if (component.syncMode == SyncMode.Observers) + serverObserversInitialMask |= nthBit; } } @@ -833,29 +859,28 @@ internal void OnStopAuthority() } } - // build dirty mask for server owner & observers (= all dirty components). - // faster to do it in one iteration instead of iterating separately. - (ulong, ulong) ServerDirtyMasks(bool initialState) + // NetworkBehaviour OnBecameDirty calls NetworkIdentity callback with index + internal void OnBecameDirty(NetworkBehaviour component) { - ulong ownerMask = 0; - ulong observerMask = 0; + ulong nthBit = (1u << component.ComponentIndex); - NetworkBehaviour[] components = NetworkBehaviours; - for (int i = 0; i < components.Length; ++i) + // 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?"); + + // set for server & client both. + // OnSerialize will decide how to use them. + if (isServer) { - NetworkBehaviour component = components[i]; - - bool dirty = component.IsDirty(); - ulong nthBit = (1u << i); - // owner needs to be considered for both SyncModes, because // Observers mode always includes the Owner. // // for initial, it should always sync owner. // for delta, only for ServerToClient and only if dirty. // ClientToServer comes from the owner client. - if (initialState || (component.syncDirection == SyncDirection.ServerToClient && dirty)) - ownerMask |= nthBit; + if (component.syncDirection == SyncDirection.ServerToClient) + serverOwnerDirtyMask |= nthBit; // observers need to be considered only in Observers mode // @@ -863,21 +888,10 @@ internal void OnStopAuthority() // for delta, only if dirty. // SyncDirection is irrelevant, as both are broadcast to // observers which aren't the owner. - if (component.syncMode == SyncMode.Observers && (initialState || dirty)) - observerMask |= nthBit; + if (component.syncMode == SyncMode.Observers) + serverObserversDirtyMask |= nthBit; } - - return (ownerMask, observerMask); - } - - // build dirty mask for client. - // server always knows initialState, so we don't need it here. - ulong ClientDirtyMask() - { - ulong mask = 0; - - NetworkBehaviour[] components = NetworkBehaviours; - for (int i = 0; i < components.Length; ++i) + if (isClient) { // on the client, we need to consider different sync scenarios: // @@ -887,16 +901,13 @@ ulong ClientDirtyMask() // serialize only if owned. // on client, only consider owned components with SyncDirection to server - NetworkBehaviour component = components[i]; if (isOwned && component.syncDirection == SyncDirection.ClientToServer) { // set the n-th bit if dirty // shifting from small to large numbers is varint-efficient. - if (component.IsDirty()) mask |= (1u << i); + clientDirtyMask |= (1u << component.ComponentIndex); } } - - return mask; } // check if n-th component is dirty. @@ -920,12 +931,9 @@ internal void SerializeServer(bool initialState, NetworkWriter ownerWriter, Netw // check which components are dirty for owner / observers. // this is quite complicated with SyncMode + SyncDirection. - // see the function for explanation. - // - // instead of writing a 1 byte index per component, - // we limit components to 64 bits and write one ulong instead. - // the ulong is also varint compressed for minimum bandwidth. - (ulong ownerMask, ulong observerMask) = ServerDirtyMasks(initialState); + // see InitializeNetworkBehaviours and OnBecameDirty for explanations. + ulong ownerMask = initialState ? serverOwnerInitialMask : serverOwnerDirtyMask; + ulong observerMask = initialState ? serverObserversInitialMask : serverObserversDirtyMask; // if nothing dirty, then don't even write the mask. // otherwise, every unchanged object would send a 1 byte dirty mask! @@ -985,7 +993,7 @@ internal void SerializeClient(NetworkWriter writer) // instead of writing a 1 byte index per component, // we limit components to 64 bits and write one ulong instead. // the ulong is also varint compressed for minimum bandwidth. - ulong dirtyMask = ClientDirtyMask(); + ulong dirtyMask = clientDirtyMask; // varint compresses the mask to 1 byte in most cases. // instead of writing an 8 byte ulong. @@ -1325,6 +1333,10 @@ internal void Reset() // clear all component's dirty bits no matter what internal void ClearAllComponentsDirtyBits() { + serverOwnerDirtyMask = 0; + serverObserversDirtyMask = 0; + clientDirtyMask = 0; + foreach (NetworkBehaviour comp in NetworkBehaviours) { comp.ClearAllDirtyBits(); diff --git a/Assets/Mirror/Tests/Editor/NetworkIdentitySerializationTests.cs b/Assets/Mirror/Tests/Editor/NetworkIdentitySerializationTests.cs index d9546d0d5..20245a601 100644 --- a/Assets/Mirror/Tests/Editor/NetworkIdentitySerializationTests.cs +++ b/Assets/Mirror/Tests/Editor/NetworkIdentitySerializationTests.cs @@ -44,10 +44,18 @@ public void SerializeAndDeserializeAll() serverOwnerComp.syncMode = clientOwnerComp.syncMode = SyncMode.Owner; serverObserversComp.syncMode = clientObserversComp.syncMode = SyncMode.Observers; + // syncMode was changed after spawning. + // need to reinitialize the initial masks. + serverIdentity.InitializeNetworkBehaviours(); + // set unique values on server components serverOwnerComp.value = "42"; serverObserversComp.value = 42; + // TODO FIX + // ownerWriter: has owner comp and observers comp (since it's for owner) + // observers writer: only has the comp for observers + // serialize server object serverIdentity.SerializeServer(true, ownerWriter, observersWriter); @@ -87,7 +95,11 @@ public void SerializationException() // set sync modes serverCompExc.syncMode = clientCompExc.syncMode = SyncMode.Observers; - serverComp2.syncMode = clientComp2.syncMode = SyncMode.Owner; + serverComp2.syncMode = clientComp2.syncMode = SyncMode.Owner; + + // syncMode was changed after spawning. + // need to reinitialize the initial masks. + serverIdentity.InitializeNetworkBehaviours(); // set unique values on server components serverComp2.value = "42"; @@ -270,30 +282,35 @@ public void SerializeAndDeserialize_ClientToServer_NOT_OWNED() [Test] public void SerializeServer_OwnerMode_ClientToServer() { - CreateNetworked(out GameObject _, out NetworkIdentity identity, - out SyncVarTest1NetworkBehaviour comp); + CreateNetworkedAndSpawn( + out _, out NetworkIdentity serverIdentity, out SyncVarTest1NetworkBehaviour serverComp, + out _, out NetworkIdentity clientIdentity, out SyncVarTest1NetworkBehaviour clientComp); // pretend to be owned - identity.isOwned = true; - comp.syncMode = SyncMode.Owner; + serverIdentity.isOwned = true; + serverComp.syncMode = SyncMode.Owner; // set to CLIENT with some unique values // and set connection to server to pretend we are the owner. - comp.syncDirection = SyncDirection.ClientToServer; - comp.value = 12345; + serverComp.syncDirection = SyncDirection.ClientToServer; + serverComp.value = 12345; + + // syncMode was changed after spawning. + // need to reinitialize the initial masks. + serverIdentity.InitializeNetworkBehaviours(); // initial: should still write for owner - identity.SerializeServer(true, ownerWriter, observersWriter); + serverIdentity.SerializeServer(true, ownerWriter, observersWriter); Debug.Log("initial ownerWriter: " + ownerWriter); Debug.Log("initial observerWriter: " + observersWriter); Assert.That(ownerWriter.Position, Is.GreaterThan(0)); Assert.That(observersWriter.Position, Is.EqualTo(0)); // delta: ClientToServer comes from the client - ++comp.value; // change something + ++serverComp.value; // change something ownerWriter.Position = 0; observersWriter.Position = 0; - identity.SerializeServer(false, ownerWriter, observersWriter); + serverIdentity.SerializeServer(false, ownerWriter, observersWriter); Debug.Log("delta ownerWriter: " + ownerWriter); Debug.Log("delta observersWriter: " + observersWriter); Assert.That(ownerWriter.Position, Is.EqualTo(0)); @@ -309,6 +326,7 @@ public void SerializeServer_ObserversMode_ClientToServer() out SyncVarTest1NetworkBehaviour comp); // pretend to be owned + identity.isServer = true; identity.isOwned = true; comp.syncMode = SyncMode.Observers; @@ -317,6 +335,10 @@ public void SerializeServer_ObserversMode_ClientToServer() comp.syncDirection = SyncDirection.ClientToServer; comp.value = 12345; + // syncMode was changed after spawning. + // need to reinitialize the initial masks. + identity.InitializeNetworkBehaviours(); + // initial: should write something for owner and observers identity.SerializeServer(true, ownerWriter, observersWriter); Debug.Log("initial ownerWriter: " + ownerWriter); @@ -324,6 +346,9 @@ public void SerializeServer_ObserversMode_ClientToServer() Assert.That(ownerWriter.Position, Is.GreaterThan(0)); Assert.That(observersWriter.Position, Is.GreaterThan(0)); + // reset dirty bits after serializing + identity.ClearAllComponentsDirtyBits(); + // delta: should only write for observers ++comp.value; // change something ownerWriter.Position = 0;