From 3c4c6b173b1e8a0d0dfc56a1585e4ca73611d50e Mon Sep 17 00:00:00 2001 From: Emma Date: Fri, 7 Aug 2026 11:09:09 -0400 Subject: [PATCH 1/5] fix: GC allocations --- .../Runtime/Messaging/Messages/RpcMessages.cs | 8 +- .../Messaging/NetworkMessageManager.cs | 13 ++- .../Runtime/Metrics/MetricHooks.cs | 23 +++- .../Collections/NetworkList.cs | 33 +++--- .../Runtime/Serialization/BitReader.cs | 2 +- .../Runtime/Serialization/FastBufferReader.cs | 11 +- .../Editor/Messaging/MessageReceivingTests.cs | 105 +++++++++--------- .../Editor/Serialization/BytePackerTests.cs | 2 +- .../Serialization/FastBufferReaderTests.cs | 5 +- .../NetworkVariable/NetworkListTests.cs | 12 +- .../NetworkVariable/NetworkVariableTests.cs | 2 +- .../Tests/Runtime/Rpc/RpcInvocationTests.cs | 8 ++ .../Tests/Runtime/Rpc/RpcManyClientsTests.cs | 7 ++ 13 files changed, 145 insertions(+), 86 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/RpcMessages.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/RpcMessages.cs index b7f1320788..0dbb3b6bf6 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/RpcMessages.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/RpcMessages.cs @@ -83,12 +83,8 @@ public static void Handle(ref NetworkContext context, ref RpcMetadata metadata, { networkManager.Log.Exception(ex, new Context(LogLevel.Error, "Unhandled RPC exception!").AddNetworkBehaviour(networkBehaviour)); - var methodId = metadata.NetworkRpcMethodId; - networkManager.Log.Info(new Context(LogLevel.Developer, "RPC Table Contents").AddCollection(rpcsForBehaviour, entry => - { - var invokePermission = NetworkBehaviour.__rpc_permission_table[networkBehaviour.GetType()][methodId]; - return $"{entry.Key} | {entry.Value.Method.Name} | {invokePermission}"; - })); + var invokePermission = permission; + networkManager.Log.Info(new Context(LogLevel.Developer, "RPC Table Contents").AddCollection(rpcsForBehaviour, entry => $"{entry.Key} | {entry.Value.Method.Name} | {invokePermission}")); } } } diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs index 343cec9961..7034f6ae9f 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs @@ -541,7 +541,15 @@ internal int GetMessageVersion(Type type, ulong clientId, bool forReceive = fals public static void ReceiveMessage(FastBufferReader reader, ref NetworkContext context, NetworkMessageManager manager) where T : INetworkMessage, new() { var messageType = typeof(T); - var message = new T(); + + // new(T) is boxed by Mono and so will make an allocation even when T is a struct. default(T) avoids the allocation. + var message = default(T); + // If T is a class, default(T) will be null. Users can register classes as custom messages, so we need to ensure we create a valid instance. + if (message == null) + { + message = new T(); + } + var messageVersion = 0; // Special cases because these are the messages that carry the version info - thus the version info isn't @@ -633,8 +641,9 @@ internal int SendMessage(ref TMessageType messa return largestSerializedSize; } - internal unsafe int SendPreSerializedMessage(in FastBufferWriter tmpSerializer, int maxSize, ref TMessageType message, NetworkDelivery delivery, in IReadOnlyList clientIds, int messageVersionFilter) + internal unsafe int SendPreSerializedMessage(in FastBufferWriter tmpSerializer, int maxSize, ref TMessageType message, NetworkDelivery delivery, in TClientIdListType clientIds, int messageVersionFilter) where TMessageType : INetworkMessage + where TClientIdListType : IReadOnlyList { using var headerSerializer = new FastBufferWriter(FastBufferWriter.GetWriteSize(), Allocator.Temp); diff --git a/com.unity.netcode.gameobjects/Runtime/Metrics/MetricHooks.cs b/com.unity.netcode.gameobjects/Runtime/Metrics/MetricHooks.cs index fd12bc5adc..c7badc20a0 100644 --- a/com.unity.netcode.gameobjects/Runtime/Metrics/MetricHooks.cs +++ b/com.unity.netcode.gameobjects/Runtime/Metrics/MetricHooks.cs @@ -1,10 +1,12 @@ using System; +using System.Collections.Generic; namespace Unity.Netcode { internal class MetricHooks : INetworkHooks { private readonly NetworkManager m_NetworkManager; + private readonly Dictionary m_CachedTypeNames = new(); public MetricHooks(NetworkManager networkManager) { @@ -17,12 +19,12 @@ public void OnBeforeSendMessage(ulong clientId, ref T message, NetworkDeliver public void OnAfterSendMessage(ulong clientId, ref T message, NetworkDelivery delivery, int messageSizeBytes) where T : INetworkMessage { - m_NetworkManager.NetworkMetrics.TrackNetworkMessageSent(clientId, typeof(T).Name, messageSizeBytes); + m_NetworkManager.NetworkMetrics.TrackNetworkMessageSent(clientId, GetNameForType(typeof(T)), messageSizeBytes); } public void OnBeforeReceiveMessage(ulong senderId, Type messageType, int messageSizeBytes) { - m_NetworkManager.NetworkMetrics.TrackNetworkMessageReceived(senderId, messageType.Name, messageSizeBytes); + m_NetworkManager.NetworkMetrics.TrackNetworkMessageReceived(senderId, GetNameForType(messageType), messageSizeBytes); } public void OnAfterReceiveMessage(ulong senderId, Type messageType, int messageSizeBytes) @@ -66,5 +68,22 @@ public void OnAfterHandleMessage(ref T message, ref NetworkContext context) w { // TODO: Per-message metrics recording moved here } + + /// + /// Gets the Name from a given type. + /// + private string GetNameForType(Type type) + { + if (m_CachedTypeNames.TryGetValue(type, out var cachedName)) + { + return cachedName; + } + + // type.Name does a reflection lookup that does a GC allocation + // Grab the name once and save to a cache. + var name = type.Name; + m_CachedTypeNames.Add(type, name); + return name; + } } } diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs index 9c0cb4bd83..acf139b0d1 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs @@ -137,6 +137,15 @@ public override void WriteDelta(FastBufferWriter writer) } } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private T ReadValue(FastBufferReader reader) + { + // T is constrained to unmanaged, use default rather than new() to avoid an allocation. + var value = default(T); + NetworkVariableSerialization.Serializer.Read(reader, ref value); + return value; + } + /// public override void WriteField(FastBufferWriter writer) { @@ -154,9 +163,7 @@ public override void ReadField(FastBufferReader reader) reader.ReadValueSafe(out ushort count); for (int i = 0; i < count; i++) { - var value = new T(); - NetworkVariableSerialization.Serializer.Read(reader, ref value); - m_List.Add(value); + m_List.Add(ReadValue(reader)); } } @@ -178,8 +185,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) { case NetworkListEvent.EventType.Add: { - var value = new T(); - NetworkVariableSerialization.Serializer.Read(reader, ref value); + var value = ReadValue(reader); m_List.Add(value); if (OnListChanged != null) @@ -188,7 +194,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) { Type = eventType, Index = m_List.Length - 1, - Value = m_List[m_List.Length - 1] + Value = value }); } @@ -198,7 +204,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) { Type = eventType, Index = m_List.Length - 1, - Value = m_List[m_List.Length - 1] + Value = value }); // Preserve the legacy way of handling this if (keepDirtyDelta) @@ -211,8 +217,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) case NetworkListEvent.EventType.Insert: { ByteUnpacker.ReadValueBitPacked(reader, out int index); - var value = new T(); - NetworkVariableSerialization.Serializer.Read(reader, ref value); + var value = ReadValue(reader); if (index < m_List.Length) { @@ -230,7 +235,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) { Type = eventType, Index = index, - Value = m_List[index] + Value = value }); } @@ -240,7 +245,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) { Type = eventType, Index = index, - Value = m_List[index] + Value = value }); // Preserve the legacy way of handling this if (keepDirtyDelta) @@ -252,8 +257,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) break; case NetworkListEvent.EventType.Remove: { - var value = new T(); - NetworkVariableSerialization.Serializer.Read(reader, ref value); + var value = ReadValue(reader); int index = m_List.IndexOf(value); if (index == -1) { @@ -323,8 +327,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) case NetworkListEvent.EventType.Value: { ByteUnpacker.ReadValueBitPacked(reader, out int index); - var value = new T(); - NetworkVariableSerialization.Serializer.Read(reader, ref value); + var value = ReadValue(reader); if (index >= m_List.Length) { throw new Exception("Shouldn't be here, index is higher than list length"); diff --git a/com.unity.netcode.gameobjects/Runtime/Serialization/BitReader.cs b/com.unity.netcode.gameobjects/Runtime/Serialization/BitReader.cs index ab8c9bc5ce..92d1951a7a 100644 --- a/com.unity.netcode.gameobjects/Runtime/Serialization/BitReader.cs +++ b/com.unity.netcode.gameobjects/Runtime/Serialization/BitReader.cs @@ -170,7 +170,7 @@ public unsafe void ReadBit(out bool bit) [MethodImpl(MethodImplOptions.AggressiveInlining)] private unsafe void ReadPartialValue(out T value, int bytesToRead, int offsetBytes = 0) where T : unmanaged { - var val = new T(); + var val = default(T); byte* ptr = ((byte*)&val) + offsetBytes; byte* bufferPointer = m_BufferPointer + BytePosition; UnsafeUtility.MemCpy(ptr, bufferPointer, bytesToRead); diff --git a/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs b/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs index d132f775fd..de51bd8a87 100644 --- a/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs +++ b/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs @@ -493,7 +493,14 @@ public unsafe byte[] ToArray() /// Thrown if the type T does not properly implement NetworkSerialize public void ReadNetworkSerializable(out T value) where T : INetworkSerializable, new() { - value = new T(); + // new T() will always cause an allocation. + // Assign default first to see if T is a value type with a valid default + value = default; + if (value == null) + { + // The allocation is unavoidable, ensure T is created. + value = new T(); + } var bufferSerializer = new BufferSerializer(new BufferSerializerReader(this)); value.NetworkSerialize(bufferSerializer); } @@ -707,7 +714,7 @@ public unsafe void ReadPartialValue(out T value, int bytesToRead, int offsetB } #endif - var val = new T(); + var val = default(T); byte* ptr = ((byte*)&val) + offsetBytes; byte* bufferPointer = Handle->BufferPointer + Handle->Position; UnsafeUtility.MemCpy(ptr, bufferPointer, bytesToRead); diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs index d77bf4ea72..6e34ba50d2 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs @@ -1,9 +1,10 @@ using System; using System.Collections.Generic; using NUnit.Framework; -using NUnit.Framework.Internal; using Unity.Collections; using Unity.Collections.LowLevel.Unsafe; +using UnityEngine.TestTools.Constraints; +using Is = NUnit.Framework.Is; namespace Unity.Netcode.EditorTests { @@ -16,7 +17,7 @@ private struct TestMessage : INetworkMessage, INetworkSerializeByMemcpy public int C; public static bool Deserialized; public static bool Handled; - public static List DeserializedValues = new List(); + public static List DeserializedValues = new List(100000); public void Serialize(FastBufferWriter writer, int targetVersion) { @@ -95,22 +96,23 @@ public void WhenHandlingAMessage_ReceiveMethodIsCalled() }; var message = GetMessage(); - var writer = new FastBufferWriter(1300, Allocator.Temp); - using (writer) - { - writer.TryBeginWrite(FastBufferWriter.GetWriteSize(message)); - writer.WriteValue(message); + using var writer = new FastBufferWriter(1300, Allocator.Temp); + writer.TryBeginWrite(FastBufferWriter.GetWriteSize(message)); + writer.WriteValue(message); - var reader = new FastBufferReader(writer, Allocator.Temp); - using (reader) - { - m_MessageManager.HandleMessage(messageHeader, reader, 0, 0, 0); - Assert.IsTrue(TestMessage.Deserialized); - Assert.IsTrue(TestMessage.Handled); - Assert.AreEqual(1, TestMessage.DeserializedValues.Count); - Assert.AreEqual(message, TestMessage.DeserializedValues[0]); - } - } + using var reader = new FastBufferReader(writer, Allocator.Temp); + m_MessageManager.HandleMessage(messageHeader, reader, 0, 0, 0); + Assert.IsTrue(TestMessage.Deserialized); + Assert.IsTrue(TestMessage.Handled); + Assert.AreEqual(1, TestMessage.DeserializedValues.Count); + Assert.AreEqual(message, TestMessage.DeserializedValues[0]); + + // Check for GC Allocations + Assert.That(() => + { + reader.Seek(0); + m_MessageManager.HandleMessage(messageHeader, reader, 0, 0, 0); + }, Is.Not.AllocatingGCMemory()); } [Test] @@ -220,44 +222,45 @@ public unsafe void WhenReceivingMultipleMessagesAndProcessingMessageQueue_Receiv var message = GetMessage(); var message2 = GetMessage(); - var writer = new FastBufferWriter(1300, Allocator.Temp); - using (writer) + using var writer = new FastBufferWriter(1300, Allocator.Temp); + writer.WriteValueSafe(batchHeader); + BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType); + BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize); + writer.WriteValueSafe(message); + BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType); + BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize); + writer.WriteValueSafe(message2); + + // Fill out the rest of the batch header + writer.Seek(0); + batchHeader = new NetworkBatchHeader { - writer.WriteValueSafe(batchHeader); - BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType); - BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize); - writer.WriteValueSafe(message); - BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType); - BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize); - writer.WriteValueSafe(message2); + Magic = NetworkBatchHeader.MagicValue, + BatchSize = writer.Length, + BatchHash = XXHash.Hash64(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), + BatchCount = 2 + }; + writer.WriteValue(batchHeader); - // Fill out the rest of the batch header - writer.Seek(0); - batchHeader = new NetworkBatchHeader - { - Magic = NetworkBatchHeader.MagicValue, - BatchSize = writer.Length, - BatchHash = XXHash.Hash64(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), - BatchCount = 2 - }; - writer.WriteValue(batchHeader); + var data = new ArraySegment(writer.ToArray()); + m_MessageManager.HandleIncomingData(0, data, 0); + Assert.IsFalse(TestMessage.Deserialized); + Assert.IsFalse(TestMessage.Handled); + Assert.IsEmpty(TestMessage.DeserializedValues); - var reader = new FastBufferReader(writer, Allocator.Temp); - using (reader) - { - m_MessageManager.HandleIncomingData(0, new ArraySegment(writer.ToArray()), 0); - Assert.IsFalse(TestMessage.Deserialized); - Assert.IsFalse(TestMessage.Handled); - Assert.IsEmpty(TestMessage.DeserializedValues); + m_MessageManager.ProcessIncomingMessageQueue(); + Assert.IsTrue(TestMessage.Deserialized); + Assert.IsTrue(TestMessage.Handled); + Assert.AreEqual(2, TestMessage.DeserializedValues.Count); + Assert.AreEqual(message, TestMessage.DeserializedValues[0]); + Assert.AreEqual(message2, TestMessage.DeserializedValues[1]); - m_MessageManager.ProcessIncomingMessageQueue(); - Assert.IsTrue(TestMessage.Deserialized); - Assert.IsTrue(TestMessage.Handled); - Assert.AreEqual(2, TestMessage.DeserializedValues.Count); - Assert.AreEqual(message, TestMessage.DeserializedValues[0]); - Assert.AreEqual(message2, TestMessage.DeserializedValues[1]); - } - } + // Check for GC Allocations + Assert.That(() => + { + m_MessageManager.HandleIncomingData(0, data, 0); + m_MessageManager.ProcessIncomingMessageQueue(); + }, Is.Not.AllocatingGCMemory()); } } } diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Serialization/BytePackerTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Serialization/BytePackerTests.cs index 1bd197997f..8c1be03140 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Serialization/BytePackerTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Serialization/BytePackerTests.cs @@ -92,7 +92,7 @@ private unsafe void RunTypeTest(T value) where T : unmanaged using (reader) { - var outVal = new T(); + var outVal = default(T); MethodInfo method; if (value is Enum) { diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs index d38c10e0a7..d63b7bb067 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs @@ -259,7 +259,7 @@ private void RunReadMethod(string methodName, FastBufferReader reader, out T } } } - value = new T(); + value = default; Assert.NotNull(method); @@ -766,8 +766,7 @@ public unsafe void RunFixedStringTest(T fixedStringValue, int numBytesWritten { VerifyPositionAndLength(reader, writer.Length); - var result = new T(); - reader.ReadValueSafe(out result); + reader.ReadValueSafe(out T result); Assert.AreEqual(fixedStringValue, result); VerifyCheckBytes(reader, serializedValueSize); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs index 5f3be93f57..c7bc4a71c7 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs @@ -9,6 +9,8 @@ using UnityEngine; using UnityEngine.TestTools; using Random = UnityEngine.Random; +using UnityEngine.TestTools.Constraints; +using Is = NUnit.Framework.Is; namespace Unity.Netcode.RuntimeTests { @@ -104,7 +106,10 @@ public IEnumerator ValidateNetworkListSynchronization() var val = Random.Range(0, 1234); m_ExpectedValues.Add(val); - authorityInstance.TheList.Add(val); + Assert.That(() => + { + authorityInstance.TheList.Add(val); + }, Is.Not.AllocatingGCMemory()); } yield return WaitForConditionOrTimeOut(OnVerifyData); @@ -124,7 +129,10 @@ public IEnumerator ValidateNetworkListSynchronization() var valToInsert = Random.Range(1, 99); m_ExpectedValues.Insert(indexToInsert, valToInsert); - authorityInstance.TheList.Insert(indexToInsert, valToInsert); + Assert.That(() => + { + authorityInstance.TheList.Insert(indexToInsert, valToInsert); + }, Is.Not.AllocatingGCMemory()); } yield return WaitForConditionOrTimeOut(OnVerifyData); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableTests.cs index 85a72d9f19..4eb9611a75 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableTests.cs @@ -1780,7 +1780,7 @@ public void WhenSerializingAndDeserializingValueTypeNativeArrayNetworkVariables_ public unsafe T RandGenBytes(System.Random rand) where T : unmanaged { - var t = new T(); + var t = default(T); T* tPtr = &t; var s = new Span(tPtr, sizeof(T)); rand.NextBytes(s); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Rpc/RpcInvocationTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Rpc/RpcInvocationTests.cs index 613b81f5e5..9f36890f2a 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Rpc/RpcInvocationTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Rpc/RpcInvocationTests.cs @@ -8,6 +8,8 @@ using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; using UnityEngine.TestTools; +using UnityEngine.TestTools.Constraints; +using Is = NUnit.Framework.Is; namespace Unity.Netcode.RuntimeTests { @@ -335,6 +337,12 @@ public IEnumerator RpcInvocationOrderTests() Assert.IsTrue(ValidateInvocationOrder(errorLog), $"[Has nested][nonAuthority][{testType}] Rpcs were invoked in an incorrect order\n {errorLog}"); errorLog.Clear(); } + + // Safety check for GC allocations + Assert.That(() => + { + nonAuthorityInstance.EveryoneInvokePermissionRpc(); + }, Is.Not.AllocatingGCMemory()); } private void ResetAllExpectedInvocations() diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Rpc/RpcManyClientsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Rpc/RpcManyClientsTests.cs index 62fb94bbe5..99f88ecf85 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Rpc/RpcManyClientsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Rpc/RpcManyClientsTests.cs @@ -4,6 +4,7 @@ using NUnit.Framework; using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; +using UnityEngine.TestTools.Constraints; namespace Unity.Netcode.RuntimeTests { @@ -150,6 +151,12 @@ public void RpcManyClientsTest() var possibility1 = new List { m_ClientNetworkManagers[1].LocalClientId, m_ClientNetworkManagers[2].LocalClientId }; var possibility2 = new List { m_ClientNetworkManagers[2].LocalClientId, m_ClientNetworkManagers[1].LocalClientId }; Debug.Assert(Enumerable.SequenceEqual(rpcManyClientsObject.ReceivedFrom, possibility1) || Enumerable.SequenceEqual(rpcManyClientsObject.ReceivedFrom, possibility2)); + + // Safety check for GC allocations + Assert.That(() => + { + rpcManyClientsObject.WithParamsClientRpc(param); + }, NUnit.Framework.Is.Not.AllocatingGCMemory()); } } } From c4067dac6ed7a97e422a6e8ee8a4954b1e830a86 Mon Sep 17 00:00:00 2001 From: Emma Date: Fri, 7 Aug 2026 12:32:12 -0400 Subject: [PATCH 2/5] Add message receive allocations test --- .../Messages/NetworkVariableDeltaMessage.cs | 8 +- .../Tests/Runtime/Helpers/MessageCatcher.cs | 9 +- .../MessageReceiveAllocationTests.cs | 106 ++++++++++++++++++ .../MessageReceiveAllocationTests.cs.meta | 2 + .../NetworkVariable/NetworkListTests.cs | 2 +- 5 files changed, 121 insertions(+), 6 deletions(-) create mode 100644 com.unity.netcode.gameobjects/Tests/Runtime/Messaging/MessageReceiveAllocationTests.cs create mode 100644 com.unity.netcode.gameobjects/Tests/Runtime/Messaging/MessageReceiveAllocationTests.cs.meta diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs index 11fe2e2a7e..3e4c6a0541 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs @@ -47,7 +47,7 @@ internal struct NetworkVariableDeltaMessage : INetworkMessage private Dictionary> m_ForwardUpdates; - private List m_UpdatedNetworkVariables; + private NativeList m_UpdatedNetworkVariables; [MethodImpl(MethodImplOptions.AggressiveInlining)] private void WriteNetworkVariable(ref FastBufferWriter writer, ref NetworkVariableBase networkVariable, bool ensureNetworkVariableLengthSafety, int nonfragmentedSize, int fragmentedSize) @@ -217,7 +217,7 @@ public void Handle(ref NetworkContext context) var networkBehaviour = networkObject.GetNetworkBehaviourAtOrderIndex(NetworkBehaviourIndex); var isServerAndDeltaForwarding = m_ReceivedMessageVersion >= k_ServerDeltaForwardingAndNetworkDelivery && networkManager.IsServer; var markNetworkVariableDirty = m_ReceivedMessageVersion >= k_ServerDeltaForwardingAndNetworkDelivery ? false : networkManager.IsServer; - m_UpdatedNetworkVariables = new List(); + m_UpdatedNetworkVariables = new NativeList(Allocator.Temp); if (networkBehaviour == null) { @@ -396,9 +396,9 @@ public void Handle(ref NetworkContext context) // When a server forwards delta updates to connected clients, it needs to preserve the previous value // until it is done serializing all valid NetworkVariable field deltas (relative to each client). This // is invoked after it is done forwarding the deltas. - foreach (var fieldIndex in m_UpdatedNetworkVariables) + for (int i = 0; i < m_UpdatedNetworkVariables.Length; i++) { - networkBehaviour.NetworkVariableFields[fieldIndex].PostDeltaRead(); + networkBehaviour.NetworkVariableFields[m_UpdatedNetworkVariables[i]].PostDeltaRead(); } } } diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Helpers/MessageCatcher.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Helpers/MessageCatcher.cs index f408532871..a5935ca268 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Helpers/MessageCatcher.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Helpers/MessageCatcher.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using Unity.Collections; +using UnityEngine; namespace Unity.Netcode.RuntimeTests { @@ -25,7 +26,6 @@ private struct TriggerData public void ReleaseMessages() { - foreach (var caughtSpawn in m_CaughtMessages) { // Reader will be disposed within HandleMessage @@ -33,6 +33,13 @@ public void ReleaseMessages() } } + public void HandleCaughtMessage(int index) + { + var caughtMessage = m_CaughtMessages[index]; + // Reader will be disposed within HandleMessage + m_OwnerNetworkManager.ConnectionManager.MessageManager.HandleMessage(caughtMessage.Header, caughtMessage.Reader, caughtMessage.SenderId, caughtMessage.Timestamp, caughtMessage.SerializedHeaderSize); + } + public int CaughtMessageCount => m_CaughtMessages.Count; public void OnBeforeSendMessage(ulong clientId, ref T message, NetworkDelivery delivery) where T : INetworkMessage diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/MessageReceiveAllocationTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/MessageReceiveAllocationTests.cs new file mode 100644 index 0000000000..571278b598 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/MessageReceiveAllocationTests.cs @@ -0,0 +1,106 @@ +using System.Collections; +using NUnit.Framework; +using Unity.Netcode.TestHelpers.Runtime; +using UnityEngine; +using UnityEngine.TestTools; +using UnityEngine.TestTools.Constraints; +using Is = NUnit.Framework.Is; + +namespace Unity.Netcode.RuntimeTests +{ + internal class AllocationTestBehaviour : NetworkBehaviour + { + internal int RpcReceivedCount; + + public NetworkVariable TestVariable = new(); + + [Rpc(SendTo.NotMe)] + public void NotMeRpc() + { + RpcReceivedCount++; + } + } + + [TestFixture(HostOrServer.Host)] + [TestFixture(HostOrServer.DAHost)] + internal class MessageReceiveAllocationTests : NetcodeIntegrationTest + { + protected override int NumberOfClients => 1; + + public MessageReceiveAllocationTests(HostOrServer hostOrServer) : base(hostOrServer) {} + + private GameObject m_Prefab; + + protected override void OnServerAndClientsCreated() + { + m_Prefab = CreateNetworkObjectPrefab(nameof(AllocationTestBehaviour)); + m_Prefab.AddComponent(); + base.OnServerAndClientsCreated(); + } + + [UnityTest] + public IEnumerator NoAllocationsOnMessageReceive() + { + var authority = GetAuthorityNetworkManager(); + var nonAuthority = GetNonAuthorityNetworkManager(); + + var authorityInstance = SpawnObject(m_Prefab, authority); + var authorityComponent = authorityInstance.GetComponent(); + yield return WaitForSpawnedOnAllOrTimeOut(authorityInstance); + AssertOnTimeout("Timed out waiting for objects to spawn"); + + var nonAuthorityComponent = nonAuthority.SpawnManager.SpawnedObjects[authorityComponent.NetworkObjectId].GetComponent(); + + /* + * RpcMessage + */ + var rpcCatcher = new MessageCatcher(nonAuthority); + nonAuthority.ConnectionManager.MessageManager.Hook(rpcCatcher); + + // Send the same message twice: the first is replayed as a warm-up. + // The second will follow the identical code path and is checked for allocations. + authorityComponent.NotMeRpc(); + authorityComponent.NotMeRpc(); + yield return WaitForConditionOrTimeOut(() => rpcCatcher.CaughtMessageCount == 2); + AssertOnTimeout($"Timed out waiting to catch all expected {nameof(RpcMessage)} messages. Expected: 2, Actual: {rpcCatcher.CaughtMessageCount}"); + + // Unhook first so the replayed messages are handled instead of being caught again + nonAuthority.ConnectionManager.MessageManager.Unhook(rpcCatcher); + + rpcCatcher.HandleCaughtMessage(0); + Assert.AreEqual(1, nonAuthorityComponent.RpcReceivedCount); + + Assert.That(() => + { + rpcCatcher.HandleCaughtMessage(1); + }, Is.Not.AllocatingGCMemory()); + Assert.AreEqual(2, nonAuthorityComponent.RpcReceivedCount); + + /* + * NetworkVariableDeltaMessage + */ + var deltaCatcher = new MessageCatcher(nonAuthority); + nonAuthority.ConnectionManager.MessageManager.Hook(deltaCatcher); + + authorityComponent.TestVariable.Value = 1; + // Wait for the first change to be received client-side before sending the second change + yield return WaitForConditionOrTimeOut(() => deltaCatcher.CaughtMessageCount == 1); + authorityComponent.TestVariable.Value = 2; + yield return WaitForConditionOrTimeOut(() => deltaCatcher.CaughtMessageCount == 2); + AssertOnTimeout($"Timed out waiting to catch all expected {nameof(NetworkVariableDeltaMessage)} messages. Expected: 2, Actual: {deltaCatcher.CaughtMessageCount}"); + + // Unhook first so the replayed messages are handled instead of being caught again + nonAuthority.ConnectionManager.MessageManager.Unhook(deltaCatcher); + + deltaCatcher.HandleCaughtMessage(0); + Assert.AreEqual(1, nonAuthorityComponent.TestVariable.Value); + + Assert.That(() => + { + deltaCatcher.HandleCaughtMessage(1); + }, Is.Not.AllocatingGCMemory()); + Assert.AreEqual(2, nonAuthorityComponent.TestVariable.Value); + } + + } +} diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/MessageReceiveAllocationTests.cs.meta b/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/MessageReceiveAllocationTests.cs.meta new file mode 100644 index 0000000000..88be098d8d --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/MessageReceiveAllocationTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 52d657f70a8454deaa08d1de60e0fee1 \ No newline at end of file diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs index c7bc4a71c7..c53abfa0b9 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs @@ -8,9 +8,9 @@ using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; using UnityEngine.TestTools; -using Random = UnityEngine.Random; using UnityEngine.TestTools.Constraints; using Is = NUnit.Framework.Is; +using Random = UnityEngine.Random; namespace Unity.Netcode.RuntimeTests { From d98a645359c6a5d63fb4a03d6aa7ddbf171b2e7a Mon Sep 17 00:00:00 2001 From: Emma Date: Fri, 7 Aug 2026 12:33:01 -0400 Subject: [PATCH 3/5] Revert the fix to check ILPP build fails the test --- .../Runtime/Messaging/NetworkMessageManager.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs index 7034f6ae9f..9431197cce 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs @@ -543,12 +543,12 @@ internal int GetMessageVersion(Type type, ulong clientId, bool forReceive = fals var messageType = typeof(T); // new(T) is boxed by Mono and so will make an allocation even when T is a struct. default(T) avoids the allocation. - var message = default(T); + var message = new T(); // If T is a class, default(T) will be null. Users can register classes as custom messages, so we need to ensure we create a valid instance. - if (message == null) - { - message = new T(); - } + // if (message == null) + // { + // message = new T(); + // } var messageVersion = 0; From 009c121dde22d3bc1bbd67014ad6edb34729ead3 Mon Sep 17 00:00:00 2001 From: Emma Date: Fri, 7 Aug 2026 14:42:13 -0400 Subject: [PATCH 4/5] Revert forced failure --- .../Runtime/Messaging/NetworkMessageManager.cs | 16 ++++------------ .../Runtime/Serialization/FastBufferReader.cs | 9 +-------- .../Editor/Messaging/MessageReceivingTests.cs | 2 +- .../Tests/Runtime/Helpers/MessageCatcher.cs | 1 - .../Messaging/MessageReceiveAllocationTests.cs | 2 +- 5 files changed, 7 insertions(+), 23 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs index 9431197cce..21df4c7e4b 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs @@ -503,9 +503,9 @@ internal void CleanupDisconnectedClients() m_DisconnectedClients.Clear(); } - public static int CreateMessageAndGetVersion() where T : INetworkMessage, new() + public static int CreateMessageAndGetVersion() where T : struct, INetworkMessage { - return new T().Version; + return default(T).Version; } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -538,18 +538,10 @@ internal int GetMessageVersion(Type type, ulong clientId, bool forReceive = fals - public static void ReceiveMessage(FastBufferReader reader, ref NetworkContext context, NetworkMessageManager manager) where T : INetworkMessage, new() + public static void ReceiveMessage(FastBufferReader reader, ref NetworkContext context, NetworkMessageManager manager) where T : struct, INetworkMessage { var messageType = typeof(T); - - // new(T) is boxed by Mono and so will make an allocation even when T is a struct. default(T) avoids the allocation. - var message = new T(); - // If T is a class, default(T) will be null. Users can register classes as custom messages, so we need to ensure we create a valid instance. - // if (message == null) - // { - // message = new T(); - // } - + var message = default(T); var messageVersion = 0; // Special cases because these are the messages that carry the version info - thus the version info isn't diff --git a/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs b/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs index de51bd8a87..522e9c4bad 100644 --- a/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs +++ b/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs @@ -493,14 +493,7 @@ public unsafe byte[] ToArray() /// Thrown if the type T does not properly implement NetworkSerialize public void ReadNetworkSerializable(out T value) where T : INetworkSerializable, new() { - // new T() will always cause an allocation. - // Assign default first to see if T is a value type with a valid default - value = default; - if (value == null) - { - // The allocation is unavoidable, ensure T is created. - value = new T(); - } + value = new T(); var bufferSerializer = new BufferSerializer(new BufferSerializerReader(this)); value.NetworkSerialize(bufferSerializer); } diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs index 6e34ba50d2..4d3df172cf 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs @@ -17,7 +17,7 @@ private struct TestMessage : INetworkMessage, INetworkSerializeByMemcpy public int C; public static bool Deserialized; public static bool Handled; - public static List DeserializedValues = new List(100000); + public static List DeserializedValues = new List(); public void Serialize(FastBufferWriter writer, int targetVersion) { diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Helpers/MessageCatcher.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Helpers/MessageCatcher.cs index a5935ca268..63955c1acc 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Helpers/MessageCatcher.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Helpers/MessageCatcher.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using Unity.Collections; -using UnityEngine; namespace Unity.Netcode.RuntimeTests { diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/MessageReceiveAllocationTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/MessageReceiveAllocationTests.cs index 571278b598..bcd768ac9c 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/MessageReceiveAllocationTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/MessageReceiveAllocationTests.cs @@ -27,7 +27,7 @@ internal class MessageReceiveAllocationTests : NetcodeIntegrationTest { protected override int NumberOfClients => 1; - public MessageReceiveAllocationTests(HostOrServer hostOrServer) : base(hostOrServer) {} + public MessageReceiveAllocationTests(HostOrServer hostOrServer) : base(hostOrServer) { } private GameObject m_Prefab; From efb3a4368eaba021d5d9beb1931cbf5ab5f6de25 Mon Sep 17 00:00:00 2001 From: Emma Date: Mon, 10 Aug 2026 10:25:00 -0400 Subject: [PATCH 5/5] Remove gc checks on NetworkListTests --- .../Runtime/NetworkVariable/NetworkListTests.cs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs index c53abfa0b9..5f3be93f57 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs @@ -8,8 +8,6 @@ using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; using UnityEngine.TestTools; -using UnityEngine.TestTools.Constraints; -using Is = NUnit.Framework.Is; using Random = UnityEngine.Random; namespace Unity.Netcode.RuntimeTests @@ -106,10 +104,7 @@ public IEnumerator ValidateNetworkListSynchronization() var val = Random.Range(0, 1234); m_ExpectedValues.Add(val); - Assert.That(() => - { - authorityInstance.TheList.Add(val); - }, Is.Not.AllocatingGCMemory()); + authorityInstance.TheList.Add(val); } yield return WaitForConditionOrTimeOut(OnVerifyData); @@ -129,10 +124,7 @@ public IEnumerator ValidateNetworkListSynchronization() var valToInsert = Random.Range(1, 99); m_ExpectedValues.Insert(indexToInsert, valToInsert); - Assert.That(() => - { - authorityInstance.TheList.Insert(indexToInsert, valToInsert); - }, Is.Not.AllocatingGCMemory()); + authorityInstance.TheList.Insert(indexToInsert, valToInsert); } yield return WaitForConditionOrTimeOut(OnVerifyData);