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.

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;
                 }
             }
 

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;
-
-        /// <summary>Number of bytes writen to the buffer</summary>
-        public int Length => length;
-
         /// <summary>Next position to write to the buffer</summary>
-        public int Position
-        {
-            get => position;
-            set
-            {
-                position = value;
-                EnsureLength(value);
-            }
-        }
+        public int Position;
 
         /// <summary>Reset both the position and length of the stream</summary>
         // Leaves the capacity the same so that we can reuse this writer without
         // extra allocations
         public void Reset()
         {
-            position = 0;
-            length = 0;
-        }
-
-        /// <summary>Sets length, moves position if it is greater than new length</summary>
-        /// 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)
             }
         }
 
-        /// <summary>Copies buffer to new array of size 'Length'. Ignores 'Position'.</summary>
+        /// <summary>Copies buffer until 'Position' to a new array.</summary>
         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;
         }
 
-        /// <summary>Returns allocatin-free ArraySegment which points to buffer up to 'Length' (ignores 'Position').</summary>
+        /// <summary>Returns allocation-free ArraySegment until 'Position'.</summary>
         public ArraySegment<byte> ToArraySegment()
         {
-            return new ArraySegment<byte>(buffer, 0, length);
+            return new ArraySegment<byte>(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;
         }
 
         /// <summary>Writes any type that mirror supports. Uses weaver populated Writer(T).write.</summary>

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]

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}");
         }

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);

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}");
         }

Assets/Mirror/Tests/Editor/SyncListTest.cs

@@ -17,7 +17,7 @@ public static void SerializeAllTo<T>(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>(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}");
         }

Assets/Mirror/Tests/Editor/SyncVarTestBase.cs

@@ -60,7 +60,7 @@ public static bool ServerWrite<T>(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;