From a868368ecac7574410bed15476ef03db3b2559df Mon Sep 17 00:00:00 2001 From: vis2k Date: Sat, 12 Mar 2022 18:26:00 +0800 Subject: [PATCH] perf: Rpcs/Cmds functionHash bandwidth reduced from 4 => 2 bytes (with the potential for 1 byte VarInt) (#3119) * perf: Rpcs/Cmds functionHash bandwidth reduced from 4 => 2 bytes (with the potential for 1 byte VarInt) * tests --- Assets/Mirror/Runtime/Messages.cs | 8 ++- Assets/Mirror/Runtime/NetworkBehaviour.cs | 10 ++- Assets/Mirror/Runtime/NetworkClient.cs | 2 +- Assets/Mirror/Runtime/NetworkIdentity.cs | 8 +-- Assets/Mirror/Runtime/NetworkServer.cs | 4 +- Assets/Mirror/Runtime/RemoteCalls.cs | 81 +++++++++++++++++++---- Assets/Mirror/Tests/Common/MirrorTest.cs | 5 ++ 7 files changed, 90 insertions(+), 28 deletions(-) diff --git a/Assets/Mirror/Runtime/Messages.cs b/Assets/Mirror/Runtime/Messages.cs index d3816f8ac..2ec729d12 100644 --- a/Assets/Mirror/Runtime/Messages.cs +++ b/Assets/Mirror/Runtime/Messages.cs @@ -28,7 +28,9 @@ public struct CommandMessage : NetworkMessage { public uint netId; public byte componentIndex; - public int functionHash; + // NOTE: this could be 1 byte most of the time via VarInt! + // but requires custom serialization for Command/RpcMessages. + public ushort functionIndex; // the parameters for the Cmd function // -> ArraySegment to avoid unnecessary allocations public ArraySegment payload; @@ -38,7 +40,9 @@ public struct RpcMessage : NetworkMessage { public uint netId; public byte componentIndex; - public int functionHash; + // NOTE: this could be 1 byte most of the time via VarInt! + // but requires custom serialization for Command/RpcMessages. + public ushort functionIndex; // the parameters for the Cmd function // -> ArraySegment to avoid unnecessary allocations public ArraySegment payload; diff --git a/Assets/Mirror/Runtime/NetworkBehaviour.cs b/Assets/Mirror/Runtime/NetworkBehaviour.cs index adb0367f5..25d390637 100644 --- a/Assets/Mirror/Runtime/NetworkBehaviour.cs +++ b/Assets/Mirror/Runtime/NetworkBehaviour.cs @@ -3,6 +3,7 @@ using System.ComponentModel; using System.Runtime.CompilerServices; using UnityEngine; +using Mirror.RemoteCalls; namespace Mirror { @@ -235,8 +236,7 @@ protected void SendCommandInternal(string functionFullName, NetworkWriter writer { netId = netId, componentIndex = (byte)ComponentIndex, - // type+func so Inventory.RpcUse != Equipment.RpcUse - functionHash = functionFullName.GetStableHashCode(), + functionIndex = RemoteProcedureCalls.GetIndexFromFunctionHash(functionFullName), // segment to avoid reader allocations payload = writer.ToArraySegment() }; @@ -271,8 +271,7 @@ protected void SendRPCInternal(string functionFullName, NetworkWriter writer, in { netId = netId, componentIndex = (byte)ComponentIndex, - // type+func so Inventory.RpcUse != Equipment.RpcUse - functionHash = functionFullName.GetStableHashCode(), + functionIndex = RemoteProcedureCalls.GetIndexFromFunctionHash(functionFullName), // segment to avoid reader allocations payload = writer.ToArraySegment() }; @@ -319,8 +318,7 @@ protected void SendTargetRPCInternal(NetworkConnection conn, string functionFull { netId = netId, componentIndex = (byte)ComponentIndex, - // type+func so Inventory.RpcUse != Equipment.RpcUse - functionHash = functionFullName.GetStableHashCode(), + functionIndex = RemoteProcedureCalls.GetIndexFromFunctionHash(functionFullName), // segment to avoid reader allocations payload = writer.ToArraySegment() }; diff --git a/Assets/Mirror/Runtime/NetworkClient.cs b/Assets/Mirror/Runtime/NetworkClient.cs index 4357b53e2..4ca4d143c 100644 --- a/Assets/Mirror/Runtime/NetworkClient.cs +++ b/Assets/Mirror/Runtime/NetworkClient.cs @@ -1264,7 +1264,7 @@ static void OnRPCMessage(RpcMessage message) if (spawned.TryGetValue(message.netId, out NetworkIdentity identity)) { using (NetworkReaderPooled networkReader = NetworkReaderPool.Get(message.payload)) - identity.HandleRemoteCall(message.componentIndex, message.functionHash, RemoteCallType.ClientRpc, networkReader); + identity.HandleRemoteCall(message.componentIndex, message.functionIndex, RemoteCallType.ClientRpc, networkReader); } } diff --git a/Assets/Mirror/Runtime/NetworkIdentity.cs b/Assets/Mirror/Runtime/NetworkIdentity.cs index 6e5fbaf2d..9865d7985 100644 --- a/Assets/Mirror/Runtime/NetworkIdentity.cs +++ b/Assets/Mirror/Runtime/NetworkIdentity.cs @@ -1061,12 +1061,12 @@ internal void OnDeserializeAllSafely(NetworkReader reader, bool initialState) } // Helper function to handle Command/Rpc - internal void HandleRemoteCall(byte componentIndex, int functionHash, RemoteCallType remoteCallType, NetworkReader reader, NetworkConnectionToClient senderConnection = null) + internal void HandleRemoteCall(byte componentIndex, ushort functionIndex, RemoteCallType remoteCallType, NetworkReader reader, NetworkConnectionToClient senderConnection = null) { // check if unity object has been destroyed if (this == null) { - Debug.LogWarning($"{remoteCallType} [{functionHash}] received for deleted object [netId={netId}]"); + Debug.LogWarning($"{remoteCallType} [{functionIndex}] received for deleted object [netId={netId}]"); return; } @@ -1078,9 +1078,9 @@ internal void HandleRemoteCall(byte componentIndex, int functionHash, RemoteCall } NetworkBehaviour invokeComponent = NetworkBehaviours[componentIndex]; - if (!RemoteProcedureCalls.Invoke(functionHash, remoteCallType, reader, invokeComponent, senderConnection)) + if (!RemoteProcedureCalls.Invoke(functionIndex, remoteCallType, reader, invokeComponent, senderConnection)) { - Debug.LogError($"Found no receiver for incoming {remoteCallType} [{functionHash}] on {gameObject.name}, the server and client should have the same NetworkBehaviour instances [netId={netId}]."); + Debug.LogError($"Found no receiver for incoming {remoteCallType} [{functionIndex}] on {gameObject.name}, the server and client should have the same NetworkBehaviour instances [netId={netId}]."); } } diff --git a/Assets/Mirror/Runtime/NetworkServer.cs b/Assets/Mirror/Runtime/NetworkServer.cs index 45d00c491..f208f3c3b 100644 --- a/Assets/Mirror/Runtime/NetworkServer.cs +++ b/Assets/Mirror/Runtime/NetworkServer.cs @@ -966,7 +966,7 @@ static void OnCommandMessage(NetworkConnectionToClient conn, CommandMessage msg, // Commands can be for player objects, OR other objects with client-authority // -> so if this connection's controller has a different netId then // only allow the command if clientAuthorityOwner - bool requiresAuthority = RemoteProcedureCalls.CommandRequiresAuthority(msg.functionHash); + bool requiresAuthority = RemoteProcedureCalls.CommandRequiresAuthority(msg.functionIndex); if (requiresAuthority && identity.connectionToClient != conn) { Debug.LogWarning($"Command for object without authority [netId={msg.netId}]"); @@ -976,7 +976,7 @@ static void OnCommandMessage(NetworkConnectionToClient conn, CommandMessage msg, // Debug.Log($"OnCommandMessage for netId:{msg.netId} conn:{conn}"); using (NetworkReaderPooled networkReader = NetworkReaderPool.Get(msg.payload)) - identity.HandleRemoteCall(msg.componentIndex, msg.functionHash, RemoteCallType.Command, networkReader, conn as NetworkConnectionToClient); + identity.HandleRemoteCall(msg.componentIndex, msg.functionIndex, RemoteCallType.Command, networkReader, conn as NetworkConnectionToClient); } // spawning //////////////////////////////////////////////////////////// diff --git a/Assets/Mirror/Runtime/RemoteCalls.cs b/Assets/Mirror/Runtime/RemoteCalls.cs index 0005287cf..18dfa172e 100644 --- a/Assets/Mirror/Runtime/RemoteCalls.cs +++ b/Assets/Mirror/Runtime/RemoteCalls.cs @@ -30,10 +30,44 @@ public bool AreEqual(Type componentType, RemoteCallType remoteCallType, RemoteCa /// Used to help manage remote calls for NetworkBehaviours public static class RemoteProcedureCalls { - // one lookup for all remote calls. - // allows us to easily add more remote call types without duplicating code. - // note: do not clear those with [RuntimeInitializeOnLoad] - static readonly Dictionary remoteCallDelegates = new Dictionary(); + // sending rpc/cmd function hash would require 4 bytes each time. + // instead, let's only send the index to save bandwidth. + // => 1 byte index with 255 rpcs in total would not be enough. + // => 1 byte index with 255 rpcs per type is doable but lookup is hard, + // because an rpc might be in the actual type or in the base type etc + // => 2 byte index allows for 64k Rpcs and is very easy to implement + // with a SortedList + .IndexOfKey. + // + // NOTE: this could be 1 byte most of the time via VarInt! + // but requires custom serialization for Command/RpcMessages. + // + // SortedList still doesn't allow duplicate keys, which is good. + // But it allows accessing keys by index. + static readonly SortedList remoteCallDelegates = new SortedList(); + + // hash -> index reverse lookup to cache .IndexOfKey() binary search. + static readonly Dictionary remoteCallIndexLookup = new Dictionary(); + + // helper function to get rpc/cmd index from function name / hash. + internal static ushort GetIndexFromFunctionHash(string functionFullName) + { + int hash = functionFullName.GetStableHashCode(); + + // IndexOfKey runs a binary search. + // cache results in lookup. + // IMPORTANT: can't cache results when registering rpcs/cmds as the + // indices would only be valid after ALL were registered. + // return (ushort)remoteCallDelegates.IndexOfKey(hash); + + // reuse cached index if possible + if (remoteCallIndexLookup.TryGetValue(hash, out ushort index)) + return index; + + // otherwise search and cache + ushort searchedIndex = (ushort)remoteCallDelegates.IndexOfKey(hash); + remoteCallIndexLookup[hash] = searchedIndex; + return searchedIndex; + } static bool CheckIfDelegateExists(Type componentType, RemoteCallType remoteCallType, RemoteCallDelegate func, int functionHash) { @@ -64,6 +98,7 @@ internal static int RegisterDelegate(Type componentType, string functionFullName if (CheckIfDelegateExists(componentType, remoteCallType, func, hash)) return hash; + // register invoker by hash remoteCallDelegates[hash] = new Invoker { callType = remoteCallType, @@ -93,18 +128,30 @@ internal static void RemoveDelegate(int hash) => // note: no need to throw an error if not found. // an attacker might just try to call a cmd with an rpc's hash etc. // returning false is enough. - static bool GetInvokerForHash(int functionHash, RemoteCallType remoteCallType, out Invoker invoker) => - remoteCallDelegates.TryGetValue(functionHash, out invoker) && - invoker != null && - invoker.callType == remoteCallType; + static bool GetInvoker(ushort functionIndex, RemoteCallType remoteCallType, out Invoker invoker) + { + // valid index? + if (functionIndex <= remoteCallDelegates.Count) + { + // get key by index + int functionHash = remoteCallDelegates.Keys[functionIndex]; + invoker = remoteCallDelegates[functionHash]; + // check rpc type. don't allow calling cmds from rpcs, etc. + return invoker != null && + invoker.callType == remoteCallType; + } + invoker = null; + return false; + } // InvokeCmd/Rpc Delegate can all use the same function here - internal static bool Invoke(int functionHash, RemoteCallType remoteCallType, NetworkReader reader, NetworkBehaviour component, NetworkConnectionToClient senderConnection = null) + // => invoke by index to save bandwidth (2 bytes instead of 4 bytes) + internal static bool Invoke(ushort functionIndex, RemoteCallType remoteCallType, NetworkReader reader, NetworkBehaviour component, NetworkConnectionToClient senderConnection = null) { // IMPORTANT: we check if the message's componentIndex component is // actually of the right type. prevents attackers trying // to invoke remote calls on wrong components. - if (GetInvokerForHash(functionHash, remoteCallType, out Invoker invoker) && + if (GetInvoker(functionIndex, remoteCallType, out Invoker invoker) && invoker.componentType.IsInstanceOfType(component)) { // invoke function on this component @@ -115,8 +162,8 @@ internal static bool Invoke(int functionHash, RemoteCallType remoteCallType, Net } // check if the command 'requiresAuthority' which is set in the attribute - internal static bool CommandRequiresAuthority(int cmdHash) => - GetInvokerForHash(cmdHash, RemoteCallType.Command, out Invoker invoker) && + internal static bool CommandRequiresAuthority(ushort cmdIndex) => + GetInvoker(cmdIndex, RemoteCallType.Command, out Invoker invoker) && invoker.cmdRequiresAuthority; /// Gets the handler function by hash. Useful for profilers and debuggers. @@ -124,6 +171,14 @@ public static RemoteCallDelegate GetDelegate(int functionHash) => remoteCallDelegates.TryGetValue(functionHash, out Invoker invoker) ? invoker.function : null; + + // RuntimeInitializeOnLoadMethod -> fast playmode without domain reload + [RuntimeInitializeOnLoadMethod] + internal static void ResetStatics() + { + // clear rpc lookup every time. + // otherwise tests may have issues. + remoteCallIndexLookup.Clear(); + } } } - diff --git a/Assets/Mirror/Tests/Common/MirrorTest.cs b/Assets/Mirror/Tests/Common/MirrorTest.cs index 9559b9e2f..fbf3620a6 100644 --- a/Assets/Mirror/Tests/Common/MirrorTest.cs +++ b/Assets/Mirror/Tests/Common/MirrorTest.cs @@ -1,6 +1,7 @@ // base class for networking tests to make things easier. using System.Collections.Generic; using System.Linq; +using Mirror.RemoteCalls; using NUnit.Framework; using UnityEngine; @@ -48,6 +49,10 @@ public virtual void TearDown() GameObject.DestroyImmediate(transport.gameObject); Transport.activeTransport = null; NetworkManager.singleton = null; + + // clear rpc lookup caches. + // this can cause problems in tests otherwise. + RemoteProcedureCalls.ResetStatics(); } // create a tracked GameObject for tests without Networkidentity