From 8a4416e3b59f6c1bd4fbc4de0b37cfae4fcdae2c Mon Sep 17 00:00:00 2001 From: vis2k Date: Tue, 18 May 2021 12:07:10 +0800 Subject: [PATCH] breaking: perf: remove NetworkWriter.Length/SetLength/EnsureLength. Position is enough. (#2731) * NetworkWriter: obsolete .Length and .SetLength() * breaking: perf: remove NetworkWriter.Length/SetLength/EnsureLength. Position is enough. --- .../Runtime/NetworkConnectionToClient.cs | 4 +- Assets/Mirror/Runtime/NetworkWriter.cs | 70 ++++--------------- .../Mirror/Tests/Editor/EnumReadWriteTests.cs | 4 +- .../Tests/Editor/MessageInheritanceTest.cs | 6 +- .../Mirror/Tests/Editor/NetworkWriterTest.cs | 31 -------- .../Tests/Editor/StructMessagesTests.cs | 2 +- Assets/Mirror/Tests/Editor/SyncListTest.cs | 4 +- Assets/Mirror/Tests/Editor/SyncVarTestBase.cs | 2 +- 8 files changed, 23 insertions(+), 100 deletions(-) diff --git a/Assets/Mirror/Runtime/NetworkConnectionToClient.cs b/Assets/Mirror/Runtime/NetworkConnectionToClient.cs index c866df192..beae0261b 100644 --- a/Assets/Mirror/Runtime/NetworkConnectionToClient.cs +++ b/Assets/Mirror/Runtime/NetworkConnectionToClient.cs @@ -92,7 +92,7 @@ internal void SendBatch(int channelId, Batch batch) { // flush & reset writer Transport.activeTransport.ServerSend(connectionId, writer.ToArraySegment(), channelId); - writer.SetLength(0); + writer.Position = 0; } // now add to writer in any case @@ -114,7 +114,7 @@ internal void SendBatch(int channelId, Batch batch) if (writer.Position > 0) { Transport.activeTransport.ServerSend(connectionId, writer.ToArraySegment(), channelId); - writer.SetLength(0); + writer.Position = 0; } } diff --git a/Assets/Mirror/Runtime/NetworkWriter.cs b/Assets/Mirror/Runtime/NetworkWriter.cs index 2c435c7bd..25a6cc847 100644 --- a/Assets/Mirror/Runtime/NetworkWriter.cs +++ b/Assets/Mirror/Runtime/NetworkWriter.cs @@ -24,61 +24,15 @@ public class NetworkWriter // => 1500 bytes by default because on average, most packets will be <= MTU byte[] buffer = new byte[1500]; - // 'int' is the best type for .Position. 'short' is too small if we send >32kb which would result in negative .Position - // -> converting long to int is fine until 2GB of data (MAX_INT), so we don't have to worry about overflows here - int position; - int length; - - /// Number of bytes writen to the buffer - public int Length => length; - /// Next position to write to the buffer - public int Position - { - get => position; - set - { - position = value; - EnsureLength(value); - } - } + public int Position; /// Reset both the position and length of the stream // Leaves the capacity the same so that we can reuse this writer without // extra allocations public void Reset() { - position = 0; - length = 0; - } - - /// Sets length, moves position if it is greater than new length - /// Zeros out any extra length created by setlength - public void SetLength(int newLength) - { - int oldLength = length; - - // ensure length & capacity - EnsureLength(newLength); - - // zero out new length - if (oldLength < newLength) - { - Array.Clear(buffer, oldLength, newLength - oldLength); - } - - length = newLength; - position = Mathf.Min(position, length); - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - void EnsureLength(int value) - { - if (length < value) - { - length = value; - EnsureCapacity(value); - } + Position = 0; } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -91,33 +45,33 @@ void EnsureCapacity(int value) } } - /// Copies buffer to new array of size 'Length'. Ignores 'Position'. + /// Copies buffer until 'Position' to a new array. public byte[] ToArray() { - byte[] data = new byte[length]; - Array.ConstrainedCopy(buffer, 0, data, 0, length); + byte[] data = new byte[Position]; + Array.ConstrainedCopy(buffer, 0, data, 0, Position); return data; } - /// Returns allocatin-free ArraySegment which points to buffer up to 'Length' (ignores 'Position'). + /// Returns allocation-free ArraySegment until 'Position'. public ArraySegment ToArraySegment() { - return new ArraySegment(buffer, 0, length); + return new ArraySegment(buffer, 0, Position); } public void WriteByte(byte value) { - EnsureLength(position + 1); - buffer[position++] = value; + EnsureCapacity(Position + 1); + buffer[Position++] = value; } // for byte arrays with consistent size, where the reader knows how many to read // (like a packet opcode that's always the same) public void WriteBytes(byte[] buffer, int offset, int count) { - EnsureLength(position + count); - Array.ConstrainedCopy(buffer, offset, this.buffer, position, count); - position += count; + EnsureCapacity(Position + count); + Array.ConstrainedCopy(buffer, offset, this.buffer, Position, count); + Position += count; } /// Writes any type that mirror supports. Uses weaver populated Writer(T).write. diff --git a/Assets/Mirror/Tests/Editor/EnumReadWriteTests.cs b/Assets/Mirror/Tests/Editor/EnumReadWriteTests.cs index 55b410795..36c4699ed 100644 --- a/Assets/Mirror/Tests/Editor/EnumReadWriteTests.cs +++ b/Assets/Mirror/Tests/Editor/EnumReadWriteTests.cs @@ -52,7 +52,7 @@ public void ByteIsSentForByteEnum() writer.Write(msg); // should be 1 byte for data - Assert.That(writer.Length, Is.EqualTo(1)); + Assert.That(writer.Position, Is.EqualTo(1)); } [Test] @@ -64,7 +64,7 @@ public void ShortIsSentForShortEnum() writer.Write(msg); // should be 2 bytes for data - Assert.That(writer.Length, Is.EqualTo(2)); + Assert.That(writer.Position, Is.EqualTo(2)); } [Test] diff --git a/Assets/Mirror/Tests/Editor/MessageInheritanceTest.cs b/Assets/Mirror/Tests/Editor/MessageInheritanceTest.cs index 4ee0ae885..8ece53466 100644 --- a/Assets/Mirror/Tests/Editor/MessageInheritanceTest.cs +++ b/Assets/Mirror/Tests/Editor/MessageInheritanceTest.cs @@ -58,7 +58,7 @@ public void SendsVauesInParentAndChildClass() Assert.AreEqual(3, received.parentValue); Assert.AreEqual(4, received.childValue); - int writeLength = writer.Length; + int writeLength = writer.Position; int readLength = reader.Position; Assert.That(writeLength == readLength, $"OnSerializeAll and OnDeserializeAll calls write the same amount of data\n writeLength={writeLength}\n readLength={readLength}"); } @@ -87,7 +87,7 @@ public void SendsVauesWhenUsingAbstractClass() Assert.AreEqual(message, received.message); Assert.AreEqual(responseId, received.responseId); - int writeLength = writer.Length; + int writeLength = writer.Position; int readLength = reader.Position; Assert.That(writeLength == readLength, $"OnSerializeAll and OnDeserializeAll calls write the same amount of data\n writeLength={writeLength}\n readLength={readLength}"); } @@ -116,7 +116,7 @@ public void SendsVauesWhenUsingAbstractClassReverseDefineOrder() Assert.AreEqual(message, received.message); Assert.AreEqual(responseId, received.responseId); - int writeLength = writer.Length; + int writeLength = writer.Position; int readLength = reader.Position; Assert.That(writeLength == readLength, $"OnSerializeAll and OnDeserializeAll calls write the same amount of data\n writeLength={writeLength}\n readLength={readLength}"); } diff --git a/Assets/Mirror/Tests/Editor/NetworkWriterTest.cs b/Assets/Mirror/Tests/Editor/NetworkWriterTest.cs index 97c07b0d8..9a4386d0c 100644 --- a/Assets/Mirror/Tests/Editor/NetworkWriterTest.cs +++ b/Assets/Mirror/Tests/Editor/NetworkWriterTest.cs @@ -104,36 +104,6 @@ public void TestWritingSegmentAndReadingSegment() Assert.That(deserialized.Array[deserialized.Offset + i], Is.EqualTo(segment.Array[segment.Offset + i])); } - [Test] - public void TestSetLengthZeroes() - { - NetworkWriter writer = new NetworkWriter(); - writer.WriteString("I saw"); - writer.WriteInt64(0xA_FADED_DEAD_EEL); - writer.WriteString("and ate it"); - int position = writer.Position; - - writer.SetLength(10); - Assert.That(writer.Position, Is.EqualTo(10), "Decreasing length should move position"); - - // lets grow it back and check there's zeroes now. - writer.SetLength(position); - byte[] data = writer.ToArray(); - for (int i = 10; i < data.Length; i++) - { - Assert.That(data[i], Is.EqualTo(0), $"index {i} should have value 0"); - } - } - - [Test] - public void TestSetLengthInitialization() - { - NetworkWriter writer = new NetworkWriter(); - - writer.SetLength(10); - Assert.That(writer.Position, Is.EqualTo(0), "Increasing length should not move position"); - } - [Test] public void TestResetSetsPotionAndLength() { @@ -144,7 +114,6 @@ public void TestResetSetsPotionAndLength() writer.Reset(); Assert.That(writer.Position, Is.EqualTo(0)); - Assert.That(writer.Length, Is.EqualTo(0)); byte[] data = writer.ToArray(); Assert.That(data, Is.Empty); diff --git a/Assets/Mirror/Tests/Editor/StructMessagesTests.cs b/Assets/Mirror/Tests/Editor/StructMessagesTests.cs index 0e49ac6fb..79f03fca1 100644 --- a/Assets/Mirror/Tests/Editor/StructMessagesTests.cs +++ b/Assets/Mirror/Tests/Editor/StructMessagesTests.cs @@ -28,7 +28,7 @@ public void SerializeAreAddedWhenEmptyInStruct() Assert.AreEqual(someValue, received.someValue); - int writeLength = writer.Length; + int writeLength = writer.Position; int readLength = reader.Position; Assert.That(writeLength == readLength, $"OnSerializeAll and OnDeserializeAll calls write the same amount of data\n writeLength={writeLength}\n readLength={readLength}"); } diff --git a/Assets/Mirror/Tests/Editor/SyncListTest.cs b/Assets/Mirror/Tests/Editor/SyncListTest.cs index d34dd09e2..f1b995d94 100644 --- a/Assets/Mirror/Tests/Editor/SyncListTest.cs +++ b/Assets/Mirror/Tests/Editor/SyncListTest.cs @@ -17,7 +17,7 @@ public static void SerializeAllTo(T fromList, T toList) where T : SyncObject NetworkReader reader = new NetworkReader(writer.ToArray()); toList.OnDeserializeAll(reader); - int writeLength = writer.Length; + int writeLength = writer.Position; int readLength = reader.Position; Assert.That(writeLength == readLength, $"OnSerializeAll and OnDeserializeAll calls write the same amount of data\n writeLength={writeLength}\n readLength={readLength}"); @@ -31,7 +31,7 @@ public static void SerializeDeltaTo(T fromList, T toList) where T : SyncObjec toList.OnDeserializeDelta(reader); fromList.Flush(); - int writeLength = writer.Length; + int writeLength = writer.Position; int readLength = reader.Position; Assert.That(writeLength == readLength, $"OnSerializeDelta and OnDeserializeDelta calls write the same amount of data\n writeLength={writeLength}\n readLength={readLength}"); } diff --git a/Assets/Mirror/Tests/Editor/SyncVarTestBase.cs b/Assets/Mirror/Tests/Editor/SyncVarTestBase.cs index 5c56b8a8e..1a0a8424a 100644 --- a/Assets/Mirror/Tests/Editor/SyncVarTestBase.cs +++ b/Assets/Mirror/Tests/Editor/SyncVarTestBase.cs @@ -60,7 +60,7 @@ public static bool ServerWrite(T serverObject, bool initialState, out ArraySe { NetworkWriter writer = new NetworkWriter(); bool written = serverObject.OnSerialize(writer, initialState); - writeLength = writer.Length; + writeLength = writer.Position; data = writer.ToArraySegment(); return written;