Skip to content

Commit 3c4c6b1

Browse files
EmandMEmandM
committed
fix: GC allocations
1 parent c6475da commit 3c4c6b1

13 files changed

Lines changed: 145 additions & 86 deletions

File tree

‎com.unity.netcode.gameobjects/Runtime/Messaging/Messages/RpcMessages.cs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,8 @@ public static void Handle(ref NetworkContext context, ref RpcMetadata metadata,
8383
{
8484
networkManager.Log.Exception(ex, new Context(LogLevel.Error, "Unhandled RPC exception!").AddNetworkBehaviour(networkBehaviour));
8585

86-
var methodId = metadata.NetworkRpcMethodId;
87-
networkManager.Log.Info(new Context(LogLevel.Developer, "RPC Table Contents").AddCollection(rpcsForBehaviour, entry =>
88-
{
89-
var invokePermission = NetworkBehaviour.__rpc_permission_table[networkBehaviour.GetType()][methodId];
90-
return $"{entry.Key} | {entry.Value.Method.Name} | {invokePermission}";
91-
}));
86+
var invokePermission = permission;
87+
networkManager.Log.Info(new Context(LogLevel.Developer, "RPC Table Contents").AddCollection(rpcsForBehaviour, entry => $"{entry.Key} | {entry.Value.Method.Name} | {invokePermission}"));
9288
}
9389
}
9490
}

‎com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,15 @@ internal int GetMessageVersion(Type type, ulong clientId, bool forReceive = fals
541541
public static void ReceiveMessage<T>(FastBufferReader reader, ref NetworkContext context, NetworkMessageManager manager) where T : INetworkMessage, new()
542542
{
543543
var messageType = typeof(T);
544-
var message = new T();
544+
545+
// new(T) is boxed by Mono and so will make an allocation even when T is a struct. default(T) avoids the allocation.
546+
var message = default(T);
547+
// 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.
548+
if (message == null)
549+
{
550+
message = new T();
551+
}
552+
545553
var messageVersion = 0;
546554

547555
// 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<TMessageType, TClientIdListType>(ref TMessageType messa
633641
return largestSerializedSize;
634642
}
635643

636-
internal unsafe int SendPreSerializedMessage<TMessageType>(in FastBufferWriter tmpSerializer, int maxSize, ref TMessageType message, NetworkDelivery delivery, in IReadOnlyList<ulong> clientIds, int messageVersionFilter)
644+
internal unsafe int SendPreSerializedMessage<TMessageType, TClientIdListType>(in FastBufferWriter tmpSerializer, int maxSize, ref TMessageType message, NetworkDelivery delivery, in TClientIdListType clientIds, int messageVersionFilter)
637645
where TMessageType : INetworkMessage
646+
where TClientIdListType : IReadOnlyList<ulong>
638647
{
639648
using var headerSerializer = new FastBufferWriter(FastBufferWriter.GetWriteSize<NetworkMessageHeader>(), Allocator.Temp);
640649

‎com.unity.netcode.gameobjects/Runtime/Metrics/MetricHooks.cs‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
using System;
2+
using System.Collections.Generic;
23

34
namespace Unity.Netcode
45
{
56
internal class MetricHooks : INetworkHooks
67
{
78
private readonly NetworkManager m_NetworkManager;
9+
private readonly Dictionary<Type, string> m_CachedTypeNames = new();
810

911
public MetricHooks(NetworkManager networkManager)
1012
{
@@ -17,12 +19,12 @@ public void OnBeforeSendMessage<T>(ulong clientId, ref T message, NetworkDeliver
1719

1820
public void OnAfterSendMessage<T>(ulong clientId, ref T message, NetworkDelivery delivery, int messageSizeBytes) where T : INetworkMessage
1921
{
20-
m_NetworkManager.NetworkMetrics.TrackNetworkMessageSent(clientId, typeof(T).Name, messageSizeBytes);
22+
m_NetworkManager.NetworkMetrics.TrackNetworkMessageSent(clientId, GetNameForType(typeof(T)), messageSizeBytes);
2123
}
2224

2325
public void OnBeforeReceiveMessage(ulong senderId, Type messageType, int messageSizeBytes)
2426
{
25-
m_NetworkManager.NetworkMetrics.TrackNetworkMessageReceived(senderId, messageType.Name, messageSizeBytes);
27+
m_NetworkManager.NetworkMetrics.TrackNetworkMessageReceived(senderId, GetNameForType(messageType), messageSizeBytes);
2628
}
2729

2830
public void OnAfterReceiveMessage(ulong senderId, Type messageType, int messageSizeBytes)
@@ -66,5 +68,22 @@ public void OnAfterHandleMessage<T>(ref T message, ref NetworkContext context) w
6668
{
6769
// TODO: Per-message metrics recording moved here
6870
}
71+
72+
/// <summary>
73+
/// Gets the Name from a given type.
74+
/// </summary>
75+
private string GetNameForType(Type type)
76+
{
77+
if (m_CachedTypeNames.TryGetValue(type, out var cachedName))
78+
{
79+
return cachedName;
80+
}
81+
82+
// type.Name does a reflection lookup that does a GC allocation
83+
// Grab the name once and save to a cache.
84+
var name = type.Name;
85+
m_CachedTypeNames.Add(type, name);
86+
return name;
87+
}
6988
}
7089
}

‎com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,15 @@ public override void WriteDelta(FastBufferWriter writer)
137137
}
138138
}
139139

140+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
141+
private T ReadValue(FastBufferReader reader)
142+
{
143+
// T is constrained to unmanaged, use default rather than new() to avoid an allocation.
144+
var value = default(T);
145+
NetworkVariableSerialization<T>.Serializer.Read(reader, ref value);
146+
return value;
147+
}
148+
140149
/// <inheritdoc cref="NetworkVariable{T}.WriteField"/>
141150
public override void WriteField(FastBufferWriter writer)
142151
{
@@ -154,9 +163,7 @@ public override void ReadField(FastBufferReader reader)
154163
reader.ReadValueSafe(out ushort count);
155164
for (int i = 0; i < count; i++)
156165
{
157-
var value = new T();
158-
NetworkVariableSerialization<T>.Serializer.Read(reader, ref value);
159-
m_List.Add(value);
166+
m_List.Add(ReadValue(reader));
160167
}
161168
}
162169

@@ -178,8 +185,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta)
178185
{
179186
case NetworkListEvent<T>.EventType.Add:
180187
{
181-
var value = new T();
182-
NetworkVariableSerialization<T>.Serializer.Read(reader, ref value);
188+
var value = ReadValue(reader);
183189
m_List.Add(value);
184190

185191
if (OnListChanged != null)
@@ -188,7 +194,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta)
188194
{
189195
Type = eventType,
190196
Index = m_List.Length - 1,
191-
Value = m_List[m_List.Length - 1]
197+
Value = value
192198
});
193199
}
194200

@@ -198,7 +204,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta)
198204
{
199205
Type = eventType,
200206
Index = m_List.Length - 1,
201-
Value = m_List[m_List.Length - 1]
207+
Value = value
202208
});
203209
// Preserve the legacy way of handling this
204210
if (keepDirtyDelta)
@@ -211,8 +217,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta)
211217
case NetworkListEvent<T>.EventType.Insert:
212218
{
213219
ByteUnpacker.ReadValueBitPacked(reader, out int index);
214-
var value = new T();
215-
NetworkVariableSerialization<T>.Serializer.Read(reader, ref value);
220+
var value = ReadValue(reader);
216221

217222
if (index < m_List.Length)
218223
{
@@ -230,7 +235,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta)
230235
{
231236
Type = eventType,
232237
Index = index,
233-
Value = m_List[index]
238+
Value = value
234239
});
235240
}
236241

@@ -240,7 +245,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta)
240245
{
241246
Type = eventType,
242247
Index = index,
243-
Value = m_List[index]
248+
Value = value
244249
});
245250
// Preserve the legacy way of handling this
246251
if (keepDirtyDelta)
@@ -252,8 +257,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta)
252257
break;
253258
case NetworkListEvent<T>.EventType.Remove:
254259
{
255-
var value = new T();
256-
NetworkVariableSerialization<T>.Serializer.Read(reader, ref value);
260+
var value = ReadValue(reader);
257261
int index = m_List.IndexOf(value);
258262
if (index == -1)
259263
{
@@ -323,8 +327,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta)
323327
case NetworkListEvent<T>.EventType.Value:
324328
{
325329
ByteUnpacker.ReadValueBitPacked(reader, out int index);
326-
var value = new T();
327-
NetworkVariableSerialization<T>.Serializer.Read(reader, ref value);
330+
var value = ReadValue(reader);
328331
if (index >= m_List.Length)
329332
{
330333
throw new Exception("Shouldn't be here, index is higher than list length");

‎com.unity.netcode.gameobjects/Runtime/Serialization/BitReader.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ public unsafe void ReadBit(out bool bit)
170170
[MethodImpl(MethodImplOptions.AggressiveInlining)]
171171
private unsafe void ReadPartialValue<T>(out T value, int bytesToRead, int offsetBytes = 0) where T : unmanaged
172172
{
173-
var val = new T();
173+
var val = default(T);
174174
byte* ptr = ((byte*)&val) + offsetBytes;
175175
byte* bufferPointer = m_BufferPointer + BytePosition;
176176
UnsafeUtility.MemCpy(ptr, bufferPointer, bytesToRead);

‎com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,14 @@ public unsafe byte[] ToArray()
493493
/// <exception cref="NotImplementedException">Thrown if the type T does not properly implement NetworkSerialize</exception>
494494
public void ReadNetworkSerializable<T>(out T value) where T : INetworkSerializable, new()
495495
{
496-
value = new T();
496+
// new T() will always cause an allocation.
497+
// Assign default first to see if T is a value type with a valid default
498+
value = default;
499+
if (value == null)
500+
{
501+
// The allocation is unavoidable, ensure T is created.
502+
value = new T();
503+
}
497504
var bufferSerializer = new BufferSerializer<BufferSerializerReader>(new BufferSerializerReader(this));
498505
value.NetworkSerialize(bufferSerializer);
499506
}
@@ -707,7 +714,7 @@ public unsafe void ReadPartialValue<T>(out T value, int bytesToRead, int offsetB
707714
}
708715
#endif
709716

710-
var val = new T();
717+
var val = default(T);
711718
byte* ptr = ((byte*)&val) + offsetBytes;
712719
byte* bufferPointer = Handle->BufferPointer + Handle->Position;
713720
UnsafeUtility.MemCpy(ptr, bufferPointer, bytesToRead);

‎com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs‎

Lines changed: 54 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
using System;
22
using System.Collections.Generic;
33
using NUnit.Framework;
4-
using NUnit.Framework.Internal;
54
using Unity.Collections;
65
using Unity.Collections.LowLevel.Unsafe;
6+
using UnityEngine.TestTools.Constraints;
7+
using Is = NUnit.Framework.Is;
78

89
namespace Unity.Netcode.EditorTests
910
{
@@ -16,7 +17,7 @@ private struct TestMessage : INetworkMessage, INetworkSerializeByMemcpy
1617
public int C;
1718
public static bool Deserialized;
1819
public static bool Handled;
19-
public static List<TestMessage> DeserializedValues = new List<TestMessage>();
20+
public static List<TestMessage> DeserializedValues = new List<TestMessage>(100000);
2021

2122
public void Serialize(FastBufferWriter writer, int targetVersion)
2223
{
@@ -95,22 +96,23 @@ public void WhenHandlingAMessage_ReceiveMethodIsCalled()
9596
};
9697
var message = GetMessage();
9798

98-
var writer = new FastBufferWriter(1300, Allocator.Temp);
99-
using (writer)
100-
{
101-
writer.TryBeginWrite(FastBufferWriter.GetWriteSize(message));
102-
writer.WriteValue(message);
99+
using var writer = new FastBufferWriter(1300, Allocator.Temp);
100+
writer.TryBeginWrite(FastBufferWriter.GetWriteSize(message));
101+
writer.WriteValue(message);
103102

104-
var reader = new FastBufferReader(writer, Allocator.Temp);
105-
using (reader)
106-
{
107-
m_MessageManager.HandleMessage(messageHeader, reader, 0, 0, 0);
108-
Assert.IsTrue(TestMessage.Deserialized);
109-
Assert.IsTrue(TestMessage.Handled);
110-
Assert.AreEqual(1, TestMessage.DeserializedValues.Count);
111-
Assert.AreEqual(message, TestMessage.DeserializedValues[0]);
112-
}
113-
}
103+
using var reader = new FastBufferReader(writer, Allocator.Temp);
104+
m_MessageManager.HandleMessage(messageHeader, reader, 0, 0, 0);
105+
Assert.IsTrue(TestMessage.Deserialized);
106+
Assert.IsTrue(TestMessage.Handled);
107+
Assert.AreEqual(1, TestMessage.DeserializedValues.Count);
108+
Assert.AreEqual(message, TestMessage.DeserializedValues[0]);
109+
110+
// Check for GC Allocations
111+
Assert.That(() =>
112+
{
113+
reader.Seek(0);
114+
m_MessageManager.HandleMessage(messageHeader, reader, 0, 0, 0);
115+
}, Is.Not.AllocatingGCMemory());
114116
}
115117

116118
[Test]
@@ -220,44 +222,45 @@ public unsafe void WhenReceivingMultipleMessagesAndProcessingMessageQueue_Receiv
220222
var message = GetMessage();
221223
var message2 = GetMessage();
222224

223-
var writer = new FastBufferWriter(1300, Allocator.Temp);
224-
using (writer)
225+
using var writer = new FastBufferWriter(1300, Allocator.Temp);
226+
writer.WriteValueSafe(batchHeader);
227+
BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType);
228+
BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize);
229+
writer.WriteValueSafe(message);
230+
BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType);
231+
BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize);
232+
writer.WriteValueSafe(message2);
233+
234+
// Fill out the rest of the batch header
235+
writer.Seek(0);
236+
batchHeader = new NetworkBatchHeader
225237
{
226-
writer.WriteValueSafe(batchHeader);
227-
BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType);
228-
BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize);
229-
writer.WriteValueSafe(message);
230-
BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType);
231-
BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize);
232-
writer.WriteValueSafe(message2);
238+
Magic = NetworkBatchHeader.MagicValue,
239+
BatchSize = writer.Length,
240+
BatchHash = XXHash.Hash64(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)),
241+
BatchCount = 2
242+
};
243+
writer.WriteValue(batchHeader);
233244

234-
// Fill out the rest of the batch header
235-
writer.Seek(0);
236-
batchHeader = new NetworkBatchHeader
237-
{
238-
Magic = NetworkBatchHeader.MagicValue,
239-
BatchSize = writer.Length,
240-
BatchHash = XXHash.Hash64(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)),
241-
BatchCount = 2
242-
};
243-
writer.WriteValue(batchHeader);
245+
var data = new ArraySegment<byte>(writer.ToArray());
246+
m_MessageManager.HandleIncomingData(0, data, 0);
247+
Assert.IsFalse(TestMessage.Deserialized);
248+
Assert.IsFalse(TestMessage.Handled);
249+
Assert.IsEmpty(TestMessage.DeserializedValues);
244250

245-
var reader = new FastBufferReader(writer, Allocator.Temp);
246-
using (reader)
247-
{
248-
m_MessageManager.HandleIncomingData(0, new ArraySegment<byte>(writer.ToArray()), 0);
249-
Assert.IsFalse(TestMessage.Deserialized);
250-
Assert.IsFalse(TestMessage.Handled);
251-
Assert.IsEmpty(TestMessage.DeserializedValues);
251+
m_MessageManager.ProcessIncomingMessageQueue();
252+
Assert.IsTrue(TestMessage.Deserialized);
253+
Assert.IsTrue(TestMessage.Handled);
254+
Assert.AreEqual(2, TestMessage.DeserializedValues.Count);
255+
Assert.AreEqual(message, TestMessage.DeserializedValues[0]);
256+
Assert.AreEqual(message2, TestMessage.DeserializedValues[1]);
252257

253-
m_MessageManager.ProcessIncomingMessageQueue();
254-
Assert.IsTrue(TestMessage.Deserialized);
255-
Assert.IsTrue(TestMessage.Handled);
256-
Assert.AreEqual(2, TestMessage.DeserializedValues.Count);
257-
Assert.AreEqual(message, TestMessage.DeserializedValues[0]);
258-
Assert.AreEqual(message2, TestMessage.DeserializedValues[1]);
259-
}
260-
}
258+
// Check for GC Allocations
259+
Assert.That(() =>
260+
{
261+
m_MessageManager.HandleIncomingData(0, data, 0);
262+
m_MessageManager.ProcessIncomingMessageQueue();
263+
}, Is.Not.AllocatingGCMemory());
261264
}
262265
}
263266
}

‎com.unity.netcode.gameobjects/Tests/Editor/Serialization/BytePackerTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ private unsafe void RunTypeTest<T>(T value) where T : unmanaged
9292
using (reader)
9393
{
9494

95-
var outVal = new T();
95+
var outVal = default(T);
9696
MethodInfo method;
9797
if (value is Enum)
9898
{

‎com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ private void RunReadMethod<T>(string methodName, FastBufferReader reader, out T
259259
}
260260
}
261261
}
262-
value = new T();
262+
value = default;
263263

264264
Assert.NotNull(method);
265265

@@ -766,8 +766,7 @@ public unsafe void RunFixedStringTest<T>(T fixedStringValue, int numBytesWritten
766766
{
767767
VerifyPositionAndLength(reader, writer.Length);
768768

769-
var result = new T();
770-
reader.ReadValueSafe(out result);
769+
reader.ReadValueSafe(out T result);
771770
Assert.AreEqual(fixedStringValue, result);
772771

773772
VerifyCheckBytes(reader, serializedValueSize);

0 commit comments

Comments
 (0)