perf!: reduce linq, cleanup connectedclients (#1196)

This is really two separate things:

    refactoring how the NetworkManager,ConnectedClients stuff works
    reducing linq / gc allocs in the hotpath
    The reason why they are in the same PR is because it would have been really hard to make one change without the other.

Refactoring ConnectedClients

Before whenever ConnectedClientIds from NetworkManager was called we would allocate a new list and use a linq .where call to fill it :this_is_fine: (this was really problematic for instance sending a ClientRpc uses this property causing 2 allocs 🔥 )
The new code keeps an additinal id list cached.

In addition on the client side ConnectedClients is no longer partially broken by just containing the local client. The new system works like the following:

    NetworkManager.ConnectedClients / ConnectedClientsList / ConnectedClientsIds can only be accessed by the server. If a non-server tries to access them an exception is thrown.
    These lists are now exposed as properties and they are IReadOnlyLists because they really should not be modifiable.
    There is a new NetworkManager.LocalClient property which can be used to access the local client's NetworkClient. This works on clients/host.

Author: LukeStampfli

com.unity.netcode.adapter.utp/Runtime/UnityTransport.cs

@@ -1,9 +1,7 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
-using System.Linq;
 using UnityEngine;
-
 using NetcodeNetworkEvent = Unity.Netcode.NetworkEvent;
 using TransportNetworkEvent = Unity.Networking.Transport.NetworkEvent;
 using Unity.Collections.LowLevel.Unsafe;
@@ -472,12 +470,21 @@ public override void DisconnectRemoteClient(ulong clientId)
                 }
 
                 // we need to cleanup any SendQueues for this connectionID;
-                var keys = m_SendQueue.Keys.Where(k => k.ClientId == clientId).ToList();
+                var keys = new NativeList<SendTarget>(16, Allocator.Temp); // use nativelist and manual foreach to avoid allocations
+                foreach (var key in m_SendQueue.Keys)
+                {
+                    if (key.ClientId == clientId)
+                    {
+                        keys.Add(key);
+                    }
+                }
+
                 foreach (var queue in keys)
                 {
                     m_SendQueue[queue].Dispose();
                     m_SendQueue.Remove(queue);
                 }
+                keys.Dispose();
             }
         }
 

com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs

@@ -1,8 +1,8 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using UnityEngine;
 using System.Reflection;
-using System.Linq;
 using Unity.Collections;
 
 namespace Unity.Netcode
@@ -131,14 +131,7 @@ internal unsafe void __sendClientRpc(FastBufferWriter writer, uint rpcMethodId,
 
             if (rpcParams.Send.TargetClientIds != null)
             {
-                // Copy into a localArray because SendMessage doesn't take IEnumerable, only IReadOnlyList
-                ulong* localArray = stackalloc ulong[rpcParams.Send.TargetClientIds.Count()];
-                var index = 0;
-                foreach (var clientId in rpcParams.Send.TargetClientIds)
-                {
-                    localArray[index++] = clientId;
-                }
-                messageSize = NetworkManager.SendMessage(message, networkDelivery, localArray, index, true);
+                messageSize = NetworkManager.SendMessage(message, networkDelivery, in rpcParams.Send.TargetClientIds, true);
             }
             else if (rpcParams.Send.TargetClientIdsNativeArray != null)
             {

com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs

@@ -1,7 +1,6 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
-using System.Linq;
 using Unity.Collections;
 using Unity.Collections.LowLevel.Unsafe;
 using UnityEngine;
@@ -251,20 +250,61 @@ internal set
 
         private ulong m_LocalClientId;
 
+        private Dictionary<ulong, NetworkClient> m_ConnectedClients = new Dictionary<ulong, NetworkClient>();
+
+        private List<NetworkClient> m_ConnectedClientsList = new List<NetworkClient>();
+
+        private List<ulong> m_ConnectedClientIds = new List<ulong>();
+
         /// <summary>
-        /// Gets a dictionary of connected clients and their clientId keys. This is only populated on the server.
+        /// Gets a dictionary of connected clients and their clientId keys. This is only accessible on the server.
         /// </summary>
-        public readonly Dictionary<ulong, NetworkClient> ConnectedClients = new Dictionary<ulong, NetworkClient>();
+        public IReadOnlyDictionary<ulong, NetworkClient> ConnectedClients
+        {
+            get
+            {
+                if (IsServer == false)
+                {
+                    throw new NotServerException($"{nameof(ConnectedClients)} should only be accessed on server.");
+                }
+                return m_ConnectedClients;
+            }
+        }
 
         /// <summary>
-        /// Gets a list of connected clients. This is only populated on the server.
+        /// Gets a list of connected clients. This is only accessible on the server.
         /// </summary>
-        public readonly List<NetworkClient> ConnectedClientsList = new List<NetworkClient>();
+        public IReadOnlyList<NetworkClient> ConnectedClientsList
+        {
+            get
+            {
+                if (IsServer == false)
+                {
+                    throw new NotServerException($"{nameof(ConnectedClientsList)} should only be accessed on server.");
+                }
+                return m_ConnectedClientsList;
+            }
+        }
 
         /// <summary>
-        /// Gets a list of just the IDs of all connected clients.
+        /// Gets a list of just the IDs of all connected clients. This is only accessible on the server.
         /// </summary>
-        public ulong[] ConnectedClientsIds => ConnectedClientsList.Select(c => c.ClientId).ToArray();
+        public IReadOnlyList<ulong> ConnectedClientsIds
+        {
+            get
+            {
+                if (IsServer == false)
+                {
+                    throw new NotServerException($"{nameof(m_ConnectedClientIds)} should only be accessed on server.");
+                }
+                return m_ConnectedClientIds;
+            }
+        }
+
+        /// <summary>
+        /// Gets the local <see cref="NetworkClient"/> for this client.
+        /// </summary>
+        public NetworkClient LocalClient { get; internal set; }
 
         /// <summary>
         /// Gets a dictionary of the clients that have been accepted by the transport but are still pending by the Netcode. This is only populated on the server.
@@ -468,8 +508,10 @@ private void Initialize(bool server)
             LocalClientId = ulong.MaxValue;
 
             PendingClients.Clear();
-            ConnectedClients.Clear();
-            ConnectedClientsList.Clear();
+            m_ConnectedClients.Clear();
+            m_ConnectedClientsList.Clear();
+            m_ConnectedClientIds.Clear();
+            LocalClient = null;
             NetworkObject.OrphanChildren.Clear();
 
             // Create spawn manager instance
@@ -1415,12 +1457,21 @@ private void OnClientDisconnectFromServer(ulong clientId)
                 {
                     if (ConnectedClientsList[i].ClientId == clientId)
                     {
-                        ConnectedClientsList.RemoveAt(i);
+                        m_ConnectedClientsList.RemoveAt(i);
+                        break;
+                    }
+                }
+
+                for (int i = 0; i < ConnectedClientsIds.Count; i++)
+                {
+                    if (ConnectedClientsIds[i] == clientId)
+                    {
+                        m_ConnectedClientIds.RemoveAt(i);
                         break;
                     }
                 }
 
-                ConnectedClients.Remove(clientId);
+                m_ConnectedClients.Remove(clientId);
             }
             m_MessagingSystem.ClientDisconnected(clientId);
         }
@@ -1462,8 +1513,9 @@ internal void HandleApproval(ulong ownerClientId, bool createPlayerObject, uint?
                 PendingClients.Remove(ownerClientId);
 
                 var client = new NetworkClient { ClientId = ownerClientId, };
-                ConnectedClients.Add(ownerClientId, client);
-                ConnectedClientsList.Add(client);
+                m_ConnectedClients.Add(ownerClientId, client);
+                m_ConnectedClientsList.Add(client);
+                m_ConnectedClientIds.Add(client.ClientId);
 
                 if (createPlayerObject)
                 {

com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs

@@ -694,7 +694,7 @@ private void OnTransformParentChanged()
 
             unsafe
             {
-                var maxCount = NetworkManager.ConnectedClientsIds.Length;
+                var maxCount = NetworkManager.ConnectedClientsIds.Count;
                 ulong* clientIds = stackalloc ulong[maxCount];
                 int idx = 0;
                 foreach (var clientId in NetworkManager.ConnectedClientsIds)

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs

@@ -66,7 +66,7 @@ public void Handle(FastBufferReader reader, ulong clientId, NetworkManager netwo
             var time = new NetworkTime(networkManager.NetworkTickSystem.TickRate, NetworkTick);
             networkManager.NetworkTimeSystem.Reset(time.Time, 0.15f); // Start with a constant RTT of 150 until we receive values from the transport.
 
-            networkManager.ConnectedClients.Add(networkManager.LocalClientId, new NetworkClient { ClientId = networkManager.LocalClientId });
+            networkManager.LocalClient = new NetworkClient() { ClientId = networkManager.LocalClientId };
 
             // Only if scene management is disabled do we handle NetworkObject synchronization at this point
             if (!networkManager.NetworkConfig.EnableSceneManagement)

com.unity.netcode.gameobjects/Runtime/Messaging/RpcParams.cs

@@ -25,7 +25,7 @@ public struct ClientRpcSendParams
         /// Note: Even if you provide a value type such as NativeArray, enumerating it will cause boxing.
         /// If you want to avoid boxing, use TargetClientIdsNativeArray
         /// </summary>
-        public IEnumerable<ulong> TargetClientIds;
+        public IReadOnlyList<ulong> TargetClientIds;
 
         /// <summary>
         /// NativeArray version of target id list - use either this OR TargetClientIds

com.unity.netcode.gameobjects/Runtime/Metrics/INetworkMetrics.cs

@@ -81,7 +81,7 @@ void TrackRpcReceived(
 
         void TrackServerLogReceived(ulong senderClientId, uint logType, long bytesCount);
 
-        void TrackSceneEventSent(ulong[] receiverClientIds, uint sceneEventType, string sceneName, long bytesCount);
+        void TrackSceneEventSent(IReadOnlyList<ulong> receiverClientIds, uint sceneEventType, string sceneName, long bytesCount);
 
         void TrackSceneEventSent(ulong receiverClientId, uint sceneEventType, string sceneName, long bytesCount);
 

com.unity.netcode.gameobjects/Runtime/Metrics/NetworkMetrics.cs

@@ -262,7 +262,7 @@ public void TrackServerLogReceived(ulong senderClientId, uint logType, long byte
             m_ServerLogReceivedEvent.Mark(new ServerLogEvent(new ConnectionInfo(senderClientId), (Unity.Multiplayer.Tools.MetricTypes.LogLevel)logType, bytesCount));
         }
 
-        public void TrackSceneEventSent(ulong[] receiverClientIds, uint sceneEventType, string sceneName, long bytesCount)
+        public void TrackSceneEventSent(IReadOnlyList<ulong> receiverClientIds, uint sceneEventType, string sceneName, long bytesCount)
         {
             foreach (var receiverClientId in receiverClientIds)
             {

com.unity.netcode.gameobjects/Runtime/Metrics/NullNetworkMetrics.cs

@@ -131,7 +131,7 @@ public void TrackServerLogReceived(ulong senderClientId, uint logType, long byte
         {
         }
 
-        public void TrackSceneEventSent(ulong[] receiverClientIds, uint sceneEventType, string sceneName, long bytesCount)
+        public void TrackSceneEventSent(IReadOnlyList<ulong> receiverClientIds, uint sceneEventType, string sceneName, long bytesCount)
         {
         }
 

com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs

@@ -20,7 +20,6 @@ public class NetworkSpawnManager
         /// </summary>
         public readonly HashSet<NetworkObject> SpawnedObjectsList = new HashSet<NetworkObject>();
 
-
         /// <summary>
         /// Gets the NetworkManager associated with this SpawnManager.
         /// </summary>
@@ -69,7 +68,7 @@ public NetworkObject GetPlayerNetworkObject(ulong clientId)
                 throw new NotServerException("Only the server can find player objects from other clients.");
             }
 
-            if (NetworkManager.ConnectedClients.TryGetValue(clientId, out NetworkClient networkClient))
+            if (TryGetNetworkClient(clientId, out NetworkClient networkClient))
             {
                 return networkClient.PlayerObject;
             }
@@ -111,12 +110,37 @@ internal void RemoveOwnership(NetworkObject networkObject)
             foreach (var client in NetworkManager.ConnectedClients)
             {
                 var bytesReported = NetworkManager.LocalClientId == client.Key
-                    ? 0
-                    : size;
+                      ? 0
+                      : size;
                 NetworkManager.NetworkMetrics.TrackOwnershipChangeSent(client.Key, networkObject.NetworkObjectId, networkObject.name, bytesReported);
             }
         }
 
+        /// <summary>
+        /// Helper function to get a network client for a clientId from the NetworkManager.
+        /// On the server this will check the <see cref="NetworkManager.ConnectedClients"/> list.
+        /// On a non-server this will check the <see cref="NetworkManager.LocalClient"/> only.
+        /// </summary>
+        /// <param name="clientId">The clientId for which to try getting the NetworkClient for.</param>
+        /// <param name="networkClient">The found NetworkClient. Null if no client was found.</param>
+        /// <returns>True if a NetworkClient with a matching id was found else false.</returns>
+        private bool TryGetNetworkClient(ulong clientId, out NetworkClient networkClient)
+        {
+            if (NetworkManager.IsServer)
+            {
+                return NetworkManager.ConnectedClients.TryGetValue(clientId, out networkClient);
+            }
+
+            if (clientId == NetworkManager.LocalClient.ClientId)
+            {
+                networkClient = NetworkManager.LocalClient;
+                return true;
+            }
+
+            networkClient = null;
+            return false;
+        }
+
         internal void ChangeOwnership(NetworkObject networkObject, ulong clientId)
         {
             if (!NetworkManager.IsServer)
@@ -129,7 +153,7 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId)
                 throw new SpawnStateException("Object is not spawned");
             }
 
-            if (NetworkManager.ConnectedClients.TryGetValue(networkObject.OwnerClientId, out NetworkClient networkClient))
+            if (TryGetNetworkClient(networkObject.OwnerClientId, out NetworkClient networkClient))
             {
                 for (int i = networkClient.OwnedObjects.Count - 1; i >= 0; i--)
                 {
@@ -360,7 +384,7 @@ private void SpawnNetworkObjectLocallyCommon(NetworkObject networkObject, ulong
                 }
                 else if (playerObject && ownerClientId.Value == NetworkManager.LocalClientId)
                 {
-                    NetworkManager.ConnectedClients[ownerClientId.Value].PlayerObject = networkObject;
+                    NetworkManager.LocalClient.PlayerObject = networkObject;
                 }
             }
 
@@ -524,7 +548,6 @@ internal void DestroySceneObjects()
             }
         }
 
-
         internal void ServerSpawnSceneObjectsOnStartSweep()
         {
             var networkObjects = UnityEngine.Object.FindObjectsOfType<NetworkObject>();
@@ -577,7 +600,7 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
                 }
             }
 
-            if (!networkObject.IsOwnedByServer && !networkObject.IsPlayerObject && NetworkManager.Singleton.ConnectedClients.TryGetValue(networkObject.OwnerClientId, out NetworkClient networkClient))
+            if (!networkObject.IsOwnedByServer && !networkObject.IsPlayerObject && TryGetNetworkClient(networkObject.OwnerClientId, out NetworkClient networkClient))
             {
                 //Someone owns it.
                 for (int i = networkClient.OwnedObjects.Count - 1; i > -1; i--)
@@ -641,6 +664,7 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
                     }
                 }
             }
+
             networkObject.IsSpawned = false;
 
             if (SpawnedObjects.Remove(networkObject.NetworkObjectId))

com.unity.netcode.gameobjects/Samples/Bootstrap/Scripts/BootstrapManager.cs

@@ -39,11 +39,10 @@ private void OnGUI()
                 {
                     if (GUILayout.Button("Random Teleport"))
                     {
-                        // Get the local `NetworkClient` by `LocalClientId`
-                        if (networkManager.ConnectedClients.TryGetValue(networkManager.LocalClientId, out var networkedClient))
+                        if (networkManager.LocalClient != null)
                         {
                             // Get `BootstrapPlayer` component from the player's `PlayerObject`
-                            if (networkedClient.PlayerObject.TryGetComponent(out BootstrapPlayer bootstrapPlayer))
+                            if (networkManager.LocalClient.PlayerObject.TryGetComponent(out BootstrapPlayer bootstrapPlayer))
                             {
                                 // Invoke a `ServerRpc` from client-side to teleport player to a random position on the server-side
                                 bootstrapPlayer.RandomTeleportServerRpc();

com.unity.netcode.gameobjects/Tests/Runtime/Helpers/NetworkManagerHelper.cs

@@ -217,7 +217,6 @@ public static void ShutdownNetworkManager()
 
             if (NetworkManagerGameObject != null)
             {
-                NetworkManagerObject.ConnectedClientsList.Clear();
                 Debug.Log($"{nameof(NetworkManager)} shutdown.");
 
                 StopNetworkManagerMode();