From 880ba798915bc28492b292a60b6f694a0c73e11d Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 1 Aug 2023 19:44:12 -0500 Subject: [PATCH 01/21] temp fix Temporarily commenting out the batch header hash check to have a user validate that this fixes the issue with Android Armv7 builds. --- .../Runtime/Messaging/CustomMessageManager.cs | 40 +++++++++++-------- .../Messaging/NetworkMessageManager.cs | 15 +++---- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs index 4e15ec6496..b39c8657cc 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs @@ -199,14 +199,18 @@ internal void InvokeNamedMessage(ulong hash, ulong sender, FastBufferReader read /// The callback to run when a named message is received. public void RegisterNamedMessageHandler(string name, HandleNamedMessageDelegate callback) { - var hash32 = XXHash.Hash32(name); - var hash64 = XXHash.Hash64(name); - - m_NamedMessageHandlers32[hash32] = callback; - m_NamedMessageHandlers64[hash64] = callback; - - m_MessageHandlerNameLookup32[hash32] = name; - m_MessageHandlerNameLookup64[hash64] = name; + if (m_NetworkManager.NetworkConfig.RpcHashSize == HashSize.VarIntFourBytes) + { + var hash32 = XXHash.Hash32(name); + m_NamedMessageHandlers32[hash32] = callback; + m_MessageHandlerNameLookup32[hash32] = name; + } + else + { + var hash64 = XXHash.Hash64(name); + m_MessageHandlerNameLookup64[hash64] = name; + m_NamedMessageHandlers64[hash64] = callback; + } } /// @@ -215,14 +219,18 @@ public void RegisterNamedMessageHandler(string name, HandleNamedMessageDelegate /// The name of the message. public void UnregisterNamedMessageHandler(string name) { - var hash32 = XXHash.Hash32(name); - var hash64 = XXHash.Hash64(name); - - m_NamedMessageHandlers32.Remove(hash32); - m_NamedMessageHandlers64.Remove(hash64); - - m_MessageHandlerNameLookup32.Remove(hash32); - m_MessageHandlerNameLookup64.Remove(hash64); + if (m_NetworkManager.NetworkConfig.RpcHashSize == HashSize.VarIntFourBytes) + { + var hash32 = XXHash.Hash32(name); + m_NamedMessageHandlers32.Remove(hash32); + m_MessageHandlerNameLookup32.Remove(hash32); + } + else + { + var hash64 = XXHash.Hash64(name); + m_NamedMessageHandlers64.Remove(hash64); + m_MessageHandlerNameLookup64.Remove(hash64); + } } /// diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs index dcef1dc378..286b179172 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs @@ -261,13 +261,13 @@ internal void HandleIncomingData(ulong clientId, ArraySegment data, float return; } - var hash = XXHash.Hash64(batchReader.GetUnsafePtrAtCurrentPosition(), batchReader.Length - batchReader.Position); + //var hash = XXHash.Hash64(batchReader.GetUnsafePtrAtCurrentPosition(), batchReader.Length - batchReader.Position); - if (hash != batchHeader.BatchHash) - { - NetworkLog.LogError($"Received a packet with an invalid Hash Value. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Received Hash: {batchHeader.BatchHash}, Calculated Hash: {hash}, Offset: {data.Offset}, Size: {data.Count}, Full receive array: {ByteArrayToString(data.Array, 0, data.Array.Length)}"); - return; - } + //if (hash != batchHeader.BatchHash) + //{ + // NetworkLog.LogError($"Received a packet with an invalid Hash Value. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Received Hash: {batchHeader.BatchHash}, Calculated Hash: {hash}, Offset: {data.Offset}, Size: {data.Count}, Full receive array: {ByteArrayToString(data.Array, 0, data.Array.Length)}"); + // return; + //} for (var hookIdx = 0; hookIdx < m_Hooks.Count; ++hookIdx) { @@ -829,7 +829,8 @@ internal unsafe void ProcessSendQueues() // Skipping the Verify and sneaking the write mark in because we know it's fine. queueItem.Writer.Handle->AllowedWriteMark = sizeof(NetworkBatchHeader); #endif - queueItem.BatchHeader.BatchHash = XXHash.Hash64(queueItem.Writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), queueItem.Writer.Length - sizeof(NetworkBatchHeader)); + + //queueItem.BatchHeader.BatchHash = XXHash.Hash64(queueItem.Writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), queueItem.Writer.Length - sizeof(NetworkBatchHeader)); queueItem.BatchHeader.BatchSize = queueItem.Writer.Length; From 931d9bda22e50bdb8092a8d6550d7278e1cea614 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Fri, 4 Aug 2023 18:48:24 -0500 Subject: [PATCH 02/21] fix This resolves the issue with XXHash causing ARMv7 builds to crash when validating the batched message header and payload. Keep messages sent word aligned. Use the 32bit version of XXHash. --- .../Runtime/Messaging/NetworkMessageManager.cs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs index 286b179172..f91dbb02c5 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs @@ -33,6 +33,8 @@ public InvalidMessageStructureException(string issue) : base(issue) internal class NetworkMessageManager : IDisposable { + + private const double k_WordAlignBatchCalc = 1.0 / 8.0; public bool StopProcessing = false; private struct ReceiveQueueItem @@ -261,13 +263,13 @@ internal void HandleIncomingData(ulong clientId, ArraySegment data, float return; } - //var hash = XXHash.Hash64(batchReader.GetUnsafePtrAtCurrentPosition(), batchReader.Length - batchReader.Position); + var hash = XXHash.Hash32(batchReader.GetUnsafePtrAtCurrentPosition(), batchReader.Length - batchReader.Position); - //if (hash != batchHeader.BatchHash) - //{ - // NetworkLog.LogError($"Received a packet with an invalid Hash Value. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Received Hash: {batchHeader.BatchHash}, Calculated Hash: {hash}, Offset: {data.Offset}, Size: {data.Count}, Full receive array: {ByteArrayToString(data.Array, 0, data.Array.Length)}"); - // return; - //} + if (hash != batchHeader.BatchHash) + { + NetworkLog.LogError($"Received a packet with an invalid Hash Value. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Received Hash: {batchHeader.BatchHash}, Calculated Hash: {hash}, Offset: {data.Offset}, Size: {data.Count}, Full receive array: {ByteArrayToString(data.Array, 0, data.Array.Length)}"); + return; + } for (var hookIdx = 0; hookIdx < m_Hooks.Count; ++hookIdx) { @@ -704,6 +706,8 @@ internal unsafe int SendPreSerializedMessage(in FastBufferWriter t writeQueueItem.Writer.WriteBytes(headerSerializer.GetUnsafePtr(), headerSerializer.Length); writeQueueItem.Writer.WriteBytes(tmpSerializer.GetUnsafePtr(), tmpSerializer.Length); + // Keep word aligned for 32 bit systems (i.e. avoids issues on ARMv7) + writeQueueItem.Writer.Seek((int)Math.Ceiling(writeQueueItem.Writer.Position * k_WordAlignBatchCalc) * 8); writeQueueItem.BatchHeader.BatchCount++; for (var hookIdx = 0; hookIdx < m_Hooks.Count; ++hookIdx) { @@ -830,7 +834,7 @@ internal unsafe void ProcessSendQueues() queueItem.Writer.Handle->AllowedWriteMark = sizeof(NetworkBatchHeader); #endif - //queueItem.BatchHeader.BatchHash = XXHash.Hash64(queueItem.Writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), queueItem.Writer.Length - sizeof(NetworkBatchHeader)); + queueItem.BatchHeader.BatchHash = XXHash.Hash32(queueItem.Writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), queueItem.Writer.Length - sizeof(NetworkBatchHeader)); queueItem.BatchHeader.BatchSize = queueItem.Writer.Length; From 0f5a775e5302a8fcc8191f5c75aee9fe9fc28ce7 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Fri, 4 Aug 2023 18:49:40 -0500 Subject: [PATCH 03/21] fix Prevent padding on the NetworkBatchHeader --- .../Runtime/Messaging/NetworkBatchHeader.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkBatchHeader.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkBatchHeader.cs index 1039ce1b0b..14aed354cb 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkBatchHeader.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkBatchHeader.cs @@ -12,6 +12,11 @@ internal struct NetworkBatchHeader : INetworkSerializeByMemcpy /// public ushort Magic; + /// + /// Total number of messages in the batch. + /// + public ushort BatchCount; + /// /// Total number of bytes in the batch. /// @@ -22,9 +27,5 @@ internal struct NetworkBatchHeader : INetworkSerializeByMemcpy /// public ulong BatchHash; - /// - /// Total number of messages in the batch. - /// - public ushort BatchCount; } } From 36377a44e225a83e7bd8c11dc271be4a60a93124 Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Mon, 7 Aug 2023 13:50:31 -0500 Subject: [PATCH 04/21] Added logic on the receive end to account for the extra padding, added some extra safety logic on both read and write sides. --- .../Runtime/Messaging/NetworkMessageManager.cs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs index f91dbb02c5..0031e3d4b6 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs @@ -310,6 +310,13 @@ internal void HandleIncomingData(ulong clientId, ArraySegment data, float MessageHeaderSerializedSize = receivedHeaderSize, }); batchReader.Seek(batchReader.Position + (int)messageHeader.MessageSize); + var nextWordAlignedPosition = (int)Math.Ceiling(batchReader.Position * k_WordAlignBatchCalc) * 8; + if (!batchReader.TryBeginRead(nextWordAlignedPosition - batchReader.Position)) + { + NetworkLog.LogError("Received a message with an invalid total size!"); + return; + } + batchReader.Seek(nextWordAlignedPosition); } for (var hookIdx = 0; hookIdx < m_Hooks.Count; ++hookIdx) @@ -694,7 +701,8 @@ internal unsafe int SendPreSerializedMessage(in FastBufferWriter t else { ref var lastQueueItem = ref sendQueueItem.ElementAt(sendQueueItem.Length - 1); - if (lastQueueItem.NetworkDelivery != delivery || lastQueueItem.Writer.MaxCapacity - lastQueueItem.Writer.Position < tmpSerializer.Length + headerSerializer.Length) + var alignedTotalSize = (int)Math.Ceiling((tmpSerializer.Length + headerSerializer.Length) * k_WordAlignBatchCalc) * 8; + if (lastQueueItem.NetworkDelivery != delivery || lastQueueItem.Writer.MaxCapacity - lastQueueItem.Writer.Position < alignedTotalSize) { sendQueueItem.Add(new SendQueueItem(delivery, NonFragmentedMessageMaxSize, Allocator.TempJob, maxSize)); sendQueueItem.ElementAt(sendQueueItem.Length - 1).Writer.Seek(sizeof(NetworkBatchHeader)); @@ -706,8 +714,12 @@ internal unsafe int SendPreSerializedMessage(in FastBufferWriter t writeQueueItem.Writer.WriteBytes(headerSerializer.GetUnsafePtr(), headerSerializer.Length); writeQueueItem.Writer.WriteBytes(tmpSerializer.GetUnsafePtr(), tmpSerializer.Length); - // Keep word aligned for 32 bit systems (i.e. avoids issues on ARMv7) - writeQueueItem.Writer.Seek((int)Math.Ceiling(writeQueueItem.Writer.Position * k_WordAlignBatchCalc) * 8); + + var nextWordAlignedPosition = (int)Math.Ceiling(writeQueueItem.Writer.Position * k_WordAlignBatchCalc) * 8; + // TryBeginWrite just in case the writer needs to resize + writeQueueItem.Writer.TryBeginWrite(nextWordAlignedPosition - writeQueueItem.Writer.Position); + writeQueueItem.Writer.Seek(nextWordAlignedPosition); + writeQueueItem.BatchHeader.BatchCount++; for (var hookIdx = 0; hookIdx < m_Hooks.Count; ++hookIdx) { From 38a643b2efe611aefa6b117ab3c154e271866c27 Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Mon, 7 Aug 2023 15:23:24 -0500 Subject: [PATCH 05/21] Fixed tests --- .../Editor/Messaging/MessageReceivingTests.cs | 19 ++++++++++++++++--- .../Editor/Messaging/MessageSendingTests.cs | 13 ++++++++++--- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs index 1a9b3f644b..c5beef56b6 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs @@ -143,7 +143,7 @@ public unsafe void WhenHandlingIncomingData_ReceiveIsNotCalledBeforeProcessingIn { Magic = NetworkBatchHeader.MagicValue, BatchSize = writer.Length, - BatchHash = XXHash.Hash64(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), + BatchHash = XXHash.Hash32(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), BatchCount = 1 }; writer.WriteValue(batchHeader); @@ -180,6 +180,10 @@ public unsafe void WhenReceivingAMessageAndProcessingMessageQueue_ReceiveMethodI BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType); BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize); writer.WriteValueSafe(message); + var nextWordAlignedPosition = (int)Math.Ceiling(writer.Position * 1.0f/8.0f) * 8; + // TryBeginWrite just in case the writer needs to resize + writer.TryBeginWrite(nextWordAlignedPosition - writer.Position); + writer.Seek(nextWordAlignedPosition); // Fill out the rest of the batch header writer.Seek(0); @@ -187,7 +191,7 @@ public unsafe void WhenReceivingAMessageAndProcessingMessageQueue_ReceiveMethodI { Magic = NetworkBatchHeader.MagicValue, BatchSize = writer.Length, - BatchHash = XXHash.Hash64(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), + BatchHash = XXHash.Hash32(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), BatchCount = 1 }; writer.WriteValue(batchHeader); @@ -227,9 +231,18 @@ public unsafe void WhenReceivingMultipleMessagesAndProcessingMessageQueue_Receiv BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType); BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize); writer.WriteValueSafe(message); + var nextWordAlignedPosition = (int)Math.Ceiling(writer.Position * 1.0f/8.0f) * 8; + // TryBeginWrite just in case the writer needs to resize + writer.TryBeginWrite(nextWordAlignedPosition - writer.Position); + writer.Seek(nextWordAlignedPosition); + BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType); BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize); writer.WriteValueSafe(message2); + nextWordAlignedPosition = (int)Math.Ceiling(writer.Position * 1.0f/8.0f) * 8; + // TryBeginWrite just in case the writer needs to resize + writer.TryBeginWrite(nextWordAlignedPosition - writer.Position); + writer.Seek(nextWordAlignedPosition); // Fill out the rest of the batch header writer.Seek(0); @@ -237,7 +250,7 @@ public unsafe void WhenReceivingMultipleMessagesAndProcessingMessageQueue_Receiv { Magic = NetworkBatchHeader.MagicValue, BatchSize = writer.Length, - BatchHash = XXHash.Hash64(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), + BatchHash = XXHash.Hash32(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), BatchCount = 2 }; writer.WriteValue(batchHeader); diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageSendingTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageSendingTests.cs index a7492bc974..297e91e44b 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageSendingTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageSendingTests.cs @@ -157,7 +157,8 @@ public void WhenNotExceedingBatchSize_NewBatchesAreNotCreated() { var message = GetMessage(); var size = UnsafeUtility.SizeOf() + 2; // MessageHeader packed with this message will be 2 bytes - for (var i = 0; i < (m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / size; ++i) + var wordAlignedSize = (int)Math.Ceiling(size * 1.0f/8.0f) * 8; + for (var i = 0; i < (m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / wordAlignedSize; ++i) { m_MessageManager.SendMessage(ref message, NetworkDelivery.Reliable, m_Clients); } @@ -172,7 +173,8 @@ public void WhenExceedingBatchSize_NewBatchesAreCreated([Values(500, 1000, 1300, var message = GetMessage(); m_MessageManager.NonFragmentedMessageMaxSize = maxMessageSize; var size = UnsafeUtility.SizeOf() + 2; // MessageHeader packed with this message will be 2 bytes - for (var i = 0; i < ((m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / size) + 1; ++i) + var wordAlignedSize = (int)Math.Ceiling(size * 1.0f/8.0f) * 8; + for (var i = 0; i < ((m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / wordAlignedSize) + 1; ++i) { m_MessageManager.SendMessage(ref message, NetworkDelivery.Reliable, m_Clients); } @@ -187,7 +189,8 @@ public void WhenExceedingMTUSizeWithFragmentedDelivery_NewBatchesAreNotCreated([ var message = GetMessage(); m_MessageManager.NonFragmentedMessageMaxSize = maxMessageSize; var size = UnsafeUtility.SizeOf() + 2; // MessageHeader packed with this message will be 2 bytes - for (var i = 0; i < ((m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / size) + 1; ++i) + var wordAlignedSize = (int)Math.Ceiling(size * 1.0f/8.0f) * 8; + for (var i = 0; i < ((m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / wordAlignedSize) + 1; ++i) { m_MessageManager.SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, m_Clients); } @@ -245,6 +248,10 @@ public void WhenSendingMessaged_SentDataIsCorrect() reader.ReadValueSafe(out TestMessage receivedMessage); Assert.AreEqual(message, receivedMessage); + + var nextWordAlignedPosition = (int)Math.Ceiling(reader.Position * 1.0f/8.0f) * 8; + reader.Seek(nextWordAlignedPosition); + ByteUnpacker.ReadValueBitPacked(reader, out messageHeader.MessageType); ByteUnpacker.ReadValueBitPacked(reader, out messageHeader.MessageSize); From 12ee54ddb4222b8afb441d54426cd30b55dbac0d Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 7 Aug 2023 16:14:46 -0500 Subject: [PATCH 06/21] style Minor style adjustment --- .../Tests/Editor/Messaging/MessageSendingTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageSendingTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageSendingTests.cs index 297e91e44b..bc1f502c0d 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageSendingTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageSendingTests.cs @@ -157,7 +157,7 @@ public void WhenNotExceedingBatchSize_NewBatchesAreNotCreated() { var message = GetMessage(); var size = UnsafeUtility.SizeOf() + 2; // MessageHeader packed with this message will be 2 bytes - var wordAlignedSize = (int)Math.Ceiling(size * 1.0f/8.0f) * 8; + var wordAlignedSize = (int)Math.Ceiling(size * 1.0f / 8.0f) * 8; for (var i = 0; i < (m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / wordAlignedSize; ++i) { m_MessageManager.SendMessage(ref message, NetworkDelivery.Reliable, m_Clients); @@ -173,7 +173,7 @@ public void WhenExceedingBatchSize_NewBatchesAreCreated([Values(500, 1000, 1300, var message = GetMessage(); m_MessageManager.NonFragmentedMessageMaxSize = maxMessageSize; var size = UnsafeUtility.SizeOf() + 2; // MessageHeader packed with this message will be 2 bytes - var wordAlignedSize = (int)Math.Ceiling(size * 1.0f/8.0f) * 8; + var wordAlignedSize = (int)Math.Ceiling(size * 1.0f / 8.0f) * 8; for (var i = 0; i < ((m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / wordAlignedSize) + 1; ++i) { m_MessageManager.SendMessage(ref message, NetworkDelivery.Reliable, m_Clients); @@ -189,7 +189,7 @@ public void WhenExceedingMTUSizeWithFragmentedDelivery_NewBatchesAreNotCreated([ var message = GetMessage(); m_MessageManager.NonFragmentedMessageMaxSize = maxMessageSize; var size = UnsafeUtility.SizeOf() + 2; // MessageHeader packed with this message will be 2 bytes - var wordAlignedSize = (int)Math.Ceiling(size * 1.0f/8.0f) * 8; + var wordAlignedSize = (int)Math.Ceiling(size * 1.0f / 8.0f) * 8; for (var i = 0; i < ((m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / wordAlignedSize) + 1; ++i) { m_MessageManager.SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, m_Clients); @@ -249,7 +249,7 @@ public void WhenSendingMessaged_SentDataIsCorrect() Assert.AreEqual(message, receivedMessage); - var nextWordAlignedPosition = (int)Math.Ceiling(reader.Position * 1.0f/8.0f) * 8; + var nextWordAlignedPosition = (int)Math.Ceiling(reader.Position * 1.0f / 8.0f) * 8; reader.Seek(nextWordAlignedPosition); ByteUnpacker.ReadValueBitPacked(reader, out messageHeader.MessageType); From 6b36235936bc1c42e519da70ddddc0188554b17d Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 7 Aug 2023 16:40:31 -0500 Subject: [PATCH 07/21] style fixing whitespace issue with the other test changes. --- .../Tests/Editor/Messaging/MessageReceivingTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs index c5beef56b6..9b404ed4f2 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs @@ -180,7 +180,7 @@ public unsafe void WhenReceivingAMessageAndProcessingMessageQueue_ReceiveMethodI BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType); BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize); writer.WriteValueSafe(message); - var nextWordAlignedPosition = (int)Math.Ceiling(writer.Position * 1.0f/8.0f) * 8; + var nextWordAlignedPosition = (int)Math.Ceiling(writer.Position * 1.0f / 8.0f) * 8; // TryBeginWrite just in case the writer needs to resize writer.TryBeginWrite(nextWordAlignedPosition - writer.Position); writer.Seek(nextWordAlignedPosition); @@ -231,7 +231,7 @@ public unsafe void WhenReceivingMultipleMessagesAndProcessingMessageQueue_Receiv BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType); BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize); writer.WriteValueSafe(message); - var nextWordAlignedPosition = (int)Math.Ceiling(writer.Position * 1.0f/8.0f) * 8; + var nextWordAlignedPosition = (int)Math.Ceiling(writer.Position * 1.0f / 8.0f) * 8; // TryBeginWrite just in case the writer needs to resize writer.TryBeginWrite(nextWordAlignedPosition - writer.Position); writer.Seek(nextWordAlignedPosition); @@ -239,7 +239,7 @@ public unsafe void WhenReceivingMultipleMessagesAndProcessingMessageQueue_Receiv BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType); BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize); writer.WriteValueSafe(message2); - nextWordAlignedPosition = (int)Math.Ceiling(writer.Position * 1.0f/8.0f) * 8; + nextWordAlignedPosition = (int)Math.Ceiling(writer.Position * 1.0f / 8.0f) * 8; // TryBeginWrite just in case the writer needs to resize writer.TryBeginWrite(nextWordAlignedPosition - writer.Position); writer.Seek(nextWordAlignedPosition); From 31807b6f3720044ff0251f923a33e40778b21957 Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Tue, 8 Aug 2023 11:14:47 -0500 Subject: [PATCH 08/21] Move alignment to only happen at the batch level --- .../Messaging/NetworkMessageManager.cs | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs index 0031e3d4b6..711abdf550 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs @@ -310,13 +310,6 @@ internal void HandleIncomingData(ulong clientId, ArraySegment data, float MessageHeaderSerializedSize = receivedHeaderSize, }); batchReader.Seek(batchReader.Position + (int)messageHeader.MessageSize); - var nextWordAlignedPosition = (int)Math.Ceiling(batchReader.Position * k_WordAlignBatchCalc) * 8; - if (!batchReader.TryBeginRead(nextWordAlignedPosition - batchReader.Position)) - { - NetworkLog.LogError("Received a message with an invalid total size!"); - return; - } - batchReader.Seek(nextWordAlignedPosition); } for (var hookIdx = 0; hookIdx < m_Hooks.Count; ++hookIdx) @@ -701,8 +694,7 @@ internal unsafe int SendPreSerializedMessage(in FastBufferWriter t else { ref var lastQueueItem = ref sendQueueItem.ElementAt(sendQueueItem.Length - 1); - var alignedTotalSize = (int)Math.Ceiling((tmpSerializer.Length + headerSerializer.Length) * k_WordAlignBatchCalc) * 8; - if (lastQueueItem.NetworkDelivery != delivery || lastQueueItem.Writer.MaxCapacity - lastQueueItem.Writer.Position < alignedTotalSize) + if (lastQueueItem.NetworkDelivery != delivery || lastQueueItem.Writer.MaxCapacity - lastQueueItem.Writer.Position < tmpSerializer.Length + headerSerializer.Length) { sendQueueItem.Add(new SendQueueItem(delivery, NonFragmentedMessageMaxSize, Allocator.TempJob, maxSize)); sendQueueItem.ElementAt(sendQueueItem.Length - 1).Writer.Seek(sizeof(NetworkBatchHeader)); @@ -715,11 +707,6 @@ internal unsafe int SendPreSerializedMessage(in FastBufferWriter t writeQueueItem.Writer.WriteBytes(headerSerializer.GetUnsafePtr(), headerSerializer.Length); writeQueueItem.Writer.WriteBytes(tmpSerializer.GetUnsafePtr(), tmpSerializer.Length); - var nextWordAlignedPosition = (int)Math.Ceiling(writeQueueItem.Writer.Position * k_WordAlignBatchCalc) * 8; - // TryBeginWrite just in case the writer needs to resize - writeQueueItem.Writer.TryBeginWrite(nextWordAlignedPosition - writeQueueItem.Writer.Position); - writeQueueItem.Writer.Seek(nextWordAlignedPosition); - writeQueueItem.BatchHeader.BatchCount++; for (var hookIdx = 0; hookIdx < m_Hooks.Count; ++hookIdx) { @@ -846,11 +833,16 @@ internal unsafe void ProcessSendQueues() queueItem.Writer.Handle->AllowedWriteMark = sizeof(NetworkBatchHeader); #endif - queueItem.BatchHeader.BatchHash = XXHash.Hash32(queueItem.Writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), queueItem.Writer.Length - sizeof(NetworkBatchHeader)); - queueItem.BatchHeader.BatchSize = queueItem.Writer.Length; + var alignedLength = (int)(Math.Ceiling(queueItem.Writer.Length * k_WordAlignBatchCalc) * 8); + queueItem.Writer.TryBeginWrite(alignedLength); + + queueItem.BatchHeader.BatchHash = XXHash.Hash32(queueItem.Writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), alignedLength - sizeof(NetworkBatchHeader)); + + queueItem.BatchHeader.BatchSize = alignedLength; queueItem.Writer.WriteValue(queueItem.BatchHeader); + queueItem.Writer.Seek(alignedLength); try From b0dbd09e188d0a972cf5672299b1b4d1f4b0c842 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 8 Aug 2023 11:16:09 -0500 Subject: [PATCH 09/21] fix This resolves the tools bytes measured tests. --- .../Metrics/TransportBytesMetricsTests.cs | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs index 643a4244e5..0f3abdfffb 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs @@ -41,7 +41,10 @@ public IEnumerator TrackTotalNumberOfBytesSent() } Assert.True(observer.Found); - Assert.AreEqual(FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead, observer.Value); + var measuredSize = (long)(Math.Ceiling(observer.Value * 1.0f / 8.0f) * 8.0f); + var expectedSize = (long)(Math.Ceiling((FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead) * (1.0f / 8.0f)) * 8.0f); + + Assert.AreEqual(expectedSize, measuredSize); } [UnityTest] @@ -71,7 +74,11 @@ public IEnumerator TrackTotalNumberOfBytesReceived() } Assert.True(observer.Found); - Assert.AreEqual(FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead, observer.Value); + + var measuredSize = (long)(Math.Ceiling(observer.Value * 1.0f / 8.0f) * 8.0f); + var expectedSize = (long)(Math.Ceiling((FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead) * (1.0f / 8.0f)) * 8.0f); + + Assert.AreEqual(expectedSize, measuredSize); } private class TotalBytesObserver : IMetricObserver @@ -89,12 +96,23 @@ public TotalBytesObserver(IMetricDispatcher dispatcher, DirectionalMetricInfo me public long Value { get; private set; } + private int m_BytesFoundCounter; + private long m_TotalBytes; + public void Observe(MetricCollection collection) { if (collection.TryGetCounter(m_MetricInfo.Id, out var counter) && counter.Value > 0) { - Found = true; - Value = counter.Value; + // Don't assign another observed value once one is already observed + if (!Found) + { + Found = true; + Value = counter.Value; + var alignedSize = (long)(Math.Ceiling(counter.Value * 1.0f / 8.0f) * 8.0f); + m_TotalBytes += alignedSize; + m_BytesFoundCounter++; + UnityEngine.Debug.Log($"[{m_BytesFoundCounter}] Bytes Observed {counter.Value} | Total Bytes Observed: {m_TotalBytes}"); + } } } } From 90d6e38489091590258f4afbd54afa761bde8cee Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 8 Aug 2023 11:16:09 -0500 Subject: [PATCH 10/21] fix This resolves the tools bytes measured tests. --- .../Metrics/TransportBytesMetricsTests.cs | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs index 643a4244e5..0f3abdfffb 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs @@ -41,7 +41,10 @@ public IEnumerator TrackTotalNumberOfBytesSent() } Assert.True(observer.Found); - Assert.AreEqual(FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead, observer.Value); + var measuredSize = (long)(Math.Ceiling(observer.Value * 1.0f / 8.0f) * 8.0f); + var expectedSize = (long)(Math.Ceiling((FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead) * (1.0f / 8.0f)) * 8.0f); + + Assert.AreEqual(expectedSize, measuredSize); } [UnityTest] @@ -71,7 +74,11 @@ public IEnumerator TrackTotalNumberOfBytesReceived() } Assert.True(observer.Found); - Assert.AreEqual(FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead, observer.Value); + + var measuredSize = (long)(Math.Ceiling(observer.Value * 1.0f / 8.0f) * 8.0f); + var expectedSize = (long)(Math.Ceiling((FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead) * (1.0f / 8.0f)) * 8.0f); + + Assert.AreEqual(expectedSize, measuredSize); } private class TotalBytesObserver : IMetricObserver @@ -89,12 +96,23 @@ public TotalBytesObserver(IMetricDispatcher dispatcher, DirectionalMetricInfo me public long Value { get; private set; } + private int m_BytesFoundCounter; + private long m_TotalBytes; + public void Observe(MetricCollection collection) { if (collection.TryGetCounter(m_MetricInfo.Id, out var counter) && counter.Value > 0) { - Found = true; - Value = counter.Value; + // Don't assign another observed value once one is already observed + if (!Found) + { + Found = true; + Value = counter.Value; + var alignedSize = (long)(Math.Ceiling(counter.Value * 1.0f / 8.0f) * 8.0f); + m_TotalBytes += alignedSize; + m_BytesFoundCounter++; + UnityEngine.Debug.Log($"[{m_BytesFoundCounter}] Bytes Observed {counter.Value} | Total Bytes Observed: {m_TotalBytes}"); + } } } } From 67531605011bcfc31e852f885183ededb6f4dfda Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Tue, 8 Aug 2023 11:55:35 -0500 Subject: [PATCH 11/21] Test fixes --- .../Tests/Editor/Messaging/MessageReceivingTests.cs | 12 ------------ .../Tests/Editor/Messaging/MessageSendingTests.cs | 13 +++---------- 2 files changed, 3 insertions(+), 22 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs index 9b404ed4f2..84d0d316e2 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs @@ -180,10 +180,6 @@ public unsafe void WhenReceivingAMessageAndProcessingMessageQueue_ReceiveMethodI BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType); BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize); writer.WriteValueSafe(message); - var nextWordAlignedPosition = (int)Math.Ceiling(writer.Position * 1.0f / 8.0f) * 8; - // TryBeginWrite just in case the writer needs to resize - writer.TryBeginWrite(nextWordAlignedPosition - writer.Position); - writer.Seek(nextWordAlignedPosition); // Fill out the rest of the batch header writer.Seek(0); @@ -231,18 +227,10 @@ public unsafe void WhenReceivingMultipleMessagesAndProcessingMessageQueue_Receiv BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType); BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize); writer.WriteValueSafe(message); - var nextWordAlignedPosition = (int)Math.Ceiling(writer.Position * 1.0f / 8.0f) * 8; - // TryBeginWrite just in case the writer needs to resize - writer.TryBeginWrite(nextWordAlignedPosition - writer.Position); - writer.Seek(nextWordAlignedPosition); BytePacker.WriteValueBitPacked(writer, messageHeader.MessageType); BytePacker.WriteValueBitPacked(writer, messageHeader.MessageSize); writer.WriteValueSafe(message2); - nextWordAlignedPosition = (int)Math.Ceiling(writer.Position * 1.0f / 8.0f) * 8; - // TryBeginWrite just in case the writer needs to resize - writer.TryBeginWrite(nextWordAlignedPosition - writer.Position); - writer.Seek(nextWordAlignedPosition); // Fill out the rest of the batch header writer.Seek(0); diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageSendingTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageSendingTests.cs index bc1f502c0d..a7492bc974 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageSendingTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageSendingTests.cs @@ -157,8 +157,7 @@ public void WhenNotExceedingBatchSize_NewBatchesAreNotCreated() { var message = GetMessage(); var size = UnsafeUtility.SizeOf() + 2; // MessageHeader packed with this message will be 2 bytes - var wordAlignedSize = (int)Math.Ceiling(size * 1.0f / 8.0f) * 8; - for (var i = 0; i < (m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / wordAlignedSize; ++i) + for (var i = 0; i < (m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / size; ++i) { m_MessageManager.SendMessage(ref message, NetworkDelivery.Reliable, m_Clients); } @@ -173,8 +172,7 @@ public void WhenExceedingBatchSize_NewBatchesAreCreated([Values(500, 1000, 1300, var message = GetMessage(); m_MessageManager.NonFragmentedMessageMaxSize = maxMessageSize; var size = UnsafeUtility.SizeOf() + 2; // MessageHeader packed with this message will be 2 bytes - var wordAlignedSize = (int)Math.Ceiling(size * 1.0f / 8.0f) * 8; - for (var i = 0; i < ((m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / wordAlignedSize) + 1; ++i) + for (var i = 0; i < ((m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / size) + 1; ++i) { m_MessageManager.SendMessage(ref message, NetworkDelivery.Reliable, m_Clients); } @@ -189,8 +187,7 @@ public void WhenExceedingMTUSizeWithFragmentedDelivery_NewBatchesAreNotCreated([ var message = GetMessage(); m_MessageManager.NonFragmentedMessageMaxSize = maxMessageSize; var size = UnsafeUtility.SizeOf() + 2; // MessageHeader packed with this message will be 2 bytes - var wordAlignedSize = (int)Math.Ceiling(size * 1.0f / 8.0f) * 8; - for (var i = 0; i < ((m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / wordAlignedSize) + 1; ++i) + for (var i = 0; i < ((m_MessageManager.NonFragmentedMessageMaxSize - UnsafeUtility.SizeOf()) / size) + 1; ++i) { m_MessageManager.SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, m_Clients); } @@ -248,10 +245,6 @@ public void WhenSendingMessaged_SentDataIsCorrect() reader.ReadValueSafe(out TestMessage receivedMessage); Assert.AreEqual(message, receivedMessage); - - var nextWordAlignedPosition = (int)Math.Ceiling(reader.Position * 1.0f / 8.0f) * 8; - reader.Seek(nextWordAlignedPosition); - ByteUnpacker.ReadValueBitPacked(reader, out messageHeader.MessageType); ByteUnpacker.ReadValueBitPacked(reader, out messageHeader.MessageSize); From 9ea78df1fb1f3f7a8cb67222b8a8b1b068646500 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 8 Aug 2023 12:31:15 -0500 Subject: [PATCH 12/21] update switching back to 64 bit hash values for batch header validation. --- .../Runtime/Messaging/NetworkMessageManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs index 711abdf550..97cc189eeb 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs @@ -263,7 +263,7 @@ internal void HandleIncomingData(ulong clientId, ArraySegment data, float return; } - var hash = XXHash.Hash32(batchReader.GetUnsafePtrAtCurrentPosition(), batchReader.Length - batchReader.Position); + var hash = XXHash.Hash64(batchReader.GetUnsafePtrAtCurrentPosition(), batchReader.Length - batchReader.Position); if (hash != batchHeader.BatchHash) { @@ -837,7 +837,7 @@ internal unsafe void ProcessSendQueues() var alignedLength = (int)(Math.Ceiling(queueItem.Writer.Length * k_WordAlignBatchCalc) * 8); queueItem.Writer.TryBeginWrite(alignedLength); - queueItem.BatchHeader.BatchHash = XXHash.Hash32(queueItem.Writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), alignedLength - sizeof(NetworkBatchHeader)); + queueItem.BatchHeader.BatchHash = XXHash.Hash64(queueItem.Writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), alignedLength - sizeof(NetworkBatchHeader)); queueItem.BatchHeader.BatchSize = alignedLength; From 77334040ec54b40a69253ca20ffecb730bfe2500 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 8 Aug 2023 13:17:42 -0500 Subject: [PATCH 13/21] test revert reverting the switch to 32 bit hash values. --- .../Tests/Editor/Messaging/MessageReceivingTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs index 84d0d316e2..06b2d1a110 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs @@ -143,7 +143,7 @@ public unsafe void WhenHandlingIncomingData_ReceiveIsNotCalledBeforeProcessingIn { Magic = NetworkBatchHeader.MagicValue, BatchSize = writer.Length, - BatchHash = XXHash.Hash32(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), + BatchHash = XXHash.Hash64(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), BatchCount = 1 }; writer.WriteValue(batchHeader); @@ -187,7 +187,7 @@ public unsafe void WhenReceivingAMessageAndProcessingMessageQueue_ReceiveMethodI { Magic = NetworkBatchHeader.MagicValue, BatchSize = writer.Length, - BatchHash = XXHash.Hash32(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), + BatchHash = XXHash.Hash64(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), BatchCount = 1 }; writer.WriteValue(batchHeader); @@ -238,7 +238,7 @@ public unsafe void WhenReceivingMultipleMessagesAndProcessingMessageQueue_Receiv { Magic = NetworkBatchHeader.MagicValue, BatchSize = writer.Length, - BatchHash = XXHash.Hash32(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), + BatchHash = XXHash.Hash64(writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), writer.Length - sizeof(NetworkBatchHeader)), BatchCount = 2 }; writer.WriteValue(batchHeader); From a83ee6da1663b17c3a636019c4cd54a3d6c1ba01 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 8 Aug 2023 13:18:58 -0500 Subject: [PATCH 14/21] update revert reverting the conditional registration of either 32bit or 64 bit hash values in either table to just register in both tables. --- .../Runtime/Messaging/CustomMessageManager.cs | 38 +++++++------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs index b39c8657cc..890953e291 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs @@ -199,18 +199,13 @@ internal void InvokeNamedMessage(ulong hash, ulong sender, FastBufferReader read /// The callback to run when a named message is received. public void RegisterNamedMessageHandler(string name, HandleNamedMessageDelegate callback) { - if (m_NetworkManager.NetworkConfig.RpcHashSize == HashSize.VarIntFourBytes) - { - var hash32 = XXHash.Hash32(name); - m_NamedMessageHandlers32[hash32] = callback; - m_MessageHandlerNameLookup32[hash32] = name; - } - else - { - var hash64 = XXHash.Hash64(name); - m_MessageHandlerNameLookup64[hash64] = name; - m_NamedMessageHandlers64[hash64] = callback; - } + var hash32 = XXHash.Hash32(name); + m_NamedMessageHandlers32[hash32] = callback; + m_MessageHandlerNameLookup32[hash32] = name; + + var hash64 = XXHash.Hash64(name); + m_MessageHandlerNameLookup64[hash64] = name; + m_NamedMessageHandlers64[hash64] = callback; } /// @@ -219,18 +214,13 @@ public void RegisterNamedMessageHandler(string name, HandleNamedMessageDelegate /// The name of the message. public void UnregisterNamedMessageHandler(string name) { - if (m_NetworkManager.NetworkConfig.RpcHashSize == HashSize.VarIntFourBytes) - { - var hash32 = XXHash.Hash32(name); - m_NamedMessageHandlers32.Remove(hash32); - m_MessageHandlerNameLookup32.Remove(hash32); - } - else - { - var hash64 = XXHash.Hash64(name); - m_NamedMessageHandlers64.Remove(hash64); - m_MessageHandlerNameLookup64.Remove(hash64); - } + var hash32 = XXHash.Hash32(name); + m_NamedMessageHandlers32.Remove(hash32); + m_MessageHandlerNameLookup32.Remove(hash32); + + var hash64 = XXHash.Hash64(name); + m_NamedMessageHandlers64.Remove(hash64); + m_MessageHandlerNameLookup64.Remove(hash64); } /// From fa155fc8b748f7f38b2b59fc58417a44f77e3f57 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 8 Aug 2023 13:22:26 -0500 Subject: [PATCH 15/21] style reverting the additional CR/LF --- .../Tests/Editor/Messaging/MessageReceivingTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs index 06b2d1a110..1a9b3f644b 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageReceivingTests.cs @@ -227,7 +227,6 @@ public unsafe void WhenReceivingMultipleMessagesAndProcessingMessageQueue_Receiv 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); From f31cc4b888a4e31088ad0e008ed95c77c45b79d3 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 8 Aug 2023 13:23:55 -0500 Subject: [PATCH 16/21] style reverting the code to be exactly as it was (no need to make organizational changes) --- .../Runtime/Messaging/CustomMessageManager.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs index 890953e291..4e15ec6496 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs @@ -200,12 +200,13 @@ internal void InvokeNamedMessage(ulong hash, ulong sender, FastBufferReader read public void RegisterNamedMessageHandler(string name, HandleNamedMessageDelegate callback) { var hash32 = XXHash.Hash32(name); + var hash64 = XXHash.Hash64(name); + m_NamedMessageHandlers32[hash32] = callback; - m_MessageHandlerNameLookup32[hash32] = name; + m_NamedMessageHandlers64[hash64] = callback; - var hash64 = XXHash.Hash64(name); + m_MessageHandlerNameLookup32[hash32] = name; m_MessageHandlerNameLookup64[hash64] = name; - m_NamedMessageHandlers64[hash64] = callback; } /// @@ -215,11 +216,12 @@ public void RegisterNamedMessageHandler(string name, HandleNamedMessageDelegate public void UnregisterNamedMessageHandler(string name) { var hash32 = XXHash.Hash32(name); - m_NamedMessageHandlers32.Remove(hash32); - m_MessageHandlerNameLookup32.Remove(hash32); - var hash64 = XXHash.Hash64(name); + + m_NamedMessageHandlers32.Remove(hash32); m_NamedMessageHandlers64.Remove(hash64); + + m_MessageHandlerNameLookup32.Remove(hash32); m_MessageHandlerNameLookup64.Remove(hash64); } From 7f61c9bca470076988bb7f3070d818f98b27d31b Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 8 Aug 2023 13:26:54 -0500 Subject: [PATCH 17/21] test removing the alignment calculation from the observed byte count. Removing additional CR/LFs. --- .../Runtime/Metrics/TransportBytesMetricsTests.cs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs index 0f3abdfffb..dcdec0ee40 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs @@ -41,10 +41,8 @@ public IEnumerator TrackTotalNumberOfBytesSent() } Assert.True(observer.Found); - var measuredSize = (long)(Math.Ceiling(observer.Value * 1.0f / 8.0f) * 8.0f); var expectedSize = (long)(Math.Ceiling((FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead) * (1.0f / 8.0f)) * 8.0f); - - Assert.AreEqual(expectedSize, measuredSize); + Assert.AreEqual(expectedSize, observer.Value); } [UnityTest] @@ -64,8 +62,6 @@ public IEnumerator TrackTotalNumberOfBytesReceived() writer.Dispose(); } - - var nbFrames = 0; while (!observer.Found || nbFrames < 10) { @@ -74,11 +70,8 @@ public IEnumerator TrackTotalNumberOfBytesReceived() } Assert.True(observer.Found); - - var measuredSize = (long)(Math.Ceiling(observer.Value * 1.0f / 8.0f) * 8.0f); var expectedSize = (long)(Math.Ceiling((FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead) * (1.0f / 8.0f)) * 8.0f); - - Assert.AreEqual(expectedSize, measuredSize); + Assert.AreEqual(expectedSize, observer.Value); } private class TotalBytesObserver : IMetricObserver From c6ddd23cbe574c3f396d15c66efba1fd3d5916ba Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 8 Aug 2023 13:28:23 -0500 Subject: [PATCH 18/21] style removing extra LF/CRs. --- .../Runtime/Messaging/NetworkMessageManager.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs index 97cc189eeb..abff074712 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs @@ -33,7 +33,6 @@ public InvalidMessageStructureException(string issue) : base(issue) internal class NetworkMessageManager : IDisposable { - private const double k_WordAlignBatchCalc = 1.0 / 8.0; public bool StopProcessing = false; @@ -706,7 +705,6 @@ internal unsafe int SendPreSerializedMessage(in FastBufferWriter t writeQueueItem.Writer.WriteBytes(headerSerializer.GetUnsafePtr(), headerSerializer.Length); writeQueueItem.Writer.WriteBytes(tmpSerializer.GetUnsafePtr(), tmpSerializer.Length); - writeQueueItem.BatchHeader.BatchCount++; for (var hookIdx = 0; hookIdx < m_Hooks.Count; ++hookIdx) { From 070c29e3e87bdd0adc4c8d5ac573207c196338f6 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 8 Aug 2023 15:00:11 -0500 Subject: [PATCH 19/21] update Applying Simon's suggestion. --- .../Runtime/Messaging/NetworkMessageManager.cs | 3 +-- .../Tests/Runtime/Metrics/TransportBytesMetricsTests.cs | 9 +++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs index abff074712..a08175d123 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs @@ -33,7 +33,6 @@ public InvalidMessageStructureException(string issue) : base(issue) internal class NetworkMessageManager : IDisposable { - private const double k_WordAlignBatchCalc = 1.0 / 8.0; public bool StopProcessing = false; private struct ReceiveQueueItem @@ -832,7 +831,7 @@ internal unsafe void ProcessSendQueues() #endif - var alignedLength = (int)(Math.Ceiling(queueItem.Writer.Length * k_WordAlignBatchCalc) * 8); + var alignedLength = (queueItem.Writer.Length + 7) & ~7; queueItem.Writer.TryBeginWrite(alignedLength); queueItem.BatchHeader.BatchHash = XXHash.Hash64(queueItem.Writer.GetUnsafePtr() + sizeof(NetworkBatchHeader), alignedLength - sizeof(NetworkBatchHeader)); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs index dcdec0ee40..94e67e5630 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs @@ -41,8 +41,7 @@ public IEnumerator TrackTotalNumberOfBytesSent() } Assert.True(observer.Found); - var expectedSize = (long)(Math.Ceiling((FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead) * (1.0f / 8.0f)) * 8.0f); - Assert.AreEqual(expectedSize, observer.Value); + Assert.AreEqual(((FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead) + 7) & ~7, observer.Value); } [UnityTest] @@ -70,8 +69,7 @@ public IEnumerator TrackTotalNumberOfBytesReceived() } Assert.True(observer.Found); - var expectedSize = (long)(Math.Ceiling((FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead) * (1.0f / 8.0f)) * 8.0f); - Assert.AreEqual(expectedSize, observer.Value); + Assert.AreEqual(((FastBufferWriter.GetWriteSize(messageName) + k_MessageOverhead) + 7) & ~7, observer.Value); } private class TotalBytesObserver : IMetricObserver @@ -101,8 +99,7 @@ public void Observe(MetricCollection collection) { Found = true; Value = counter.Value; - var alignedSize = (long)(Math.Ceiling(counter.Value * 1.0f / 8.0f) * 8.0f); - m_TotalBytes += alignedSize; + m_TotalBytes += ((counter.Value + 7) & ~7); m_BytesFoundCounter++; UnityEngine.Debug.Log($"[{m_BytesFoundCounter}] Bytes Observed {counter.Value} | Total Bytes Observed: {m_TotalBytes}"); } From 58d81fee0efc3b055106bf684c9913eaf30ee1ba Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 8 Aug 2023 15:18:53 -0500 Subject: [PATCH 20/21] update This update assures we won't go above the MTU size due to word alignment. --- com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs | 2 +- .../Runtime/Messaging/NetworkMessageManager.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs index fb9e040aa9..03076e657e 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs @@ -586,7 +586,7 @@ private void OnEnable() /// public int MaximumTransmissionUnitSize { - set => MessageManager.NonFragmentedMessageMaxSize = value; + set => MessageManager.NonFragmentedMessageMaxSize = value & ~7; // Round down to nearest word aligned size get => MessageManager.NonFragmentedMessageMaxSize; } diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs index a08175d123..f45e877222 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs @@ -6,6 +6,7 @@ using Unity.Collections; using Unity.Collections.LowLevel.Unsafe; using UnityEngine; +using UnityEngine.UIElements; namespace Unity.Netcode { @@ -95,7 +96,7 @@ internal uint GetMessageType(Type t) return m_MessageTypes[t]; } - public const int DefaultNonFragmentedMessageMaxSize = 1300; + public const int DefaultNonFragmentedMessageMaxSize = 1300 & ~7; // Round down to nearest word aligned size (1296) public int NonFragmentedMessageMaxSize = DefaultNonFragmentedMessageMaxSize; public int FragmentedMessageMaxSize = int.MaxValue; From 49f7cb20ab31f27213d5b1004a0e7d1c49790bad Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 8 Aug 2023 15:19:37 -0500 Subject: [PATCH 21/21] style removing VS 2022 auto-assigned namespace... --- .../Runtime/Messaging/NetworkMessageManager.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs index f45e877222..6caf7310bd 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs @@ -6,7 +6,6 @@ using Unity.Collections; using Unity.Collections.LowLevel.Unsafe; using UnityEngine; -using UnityEngine.UIElements; namespace Unity.Netcode {