From 166b8c946736447a76c1886c4d1fb036f6e56e20 Mon Sep 17 00:00:00 2001 From: Paul Pacheco Date: Sat, 28 Mar 2020 12:17:53 -0500 Subject: [PATCH] perf: use byte[] directly instead of MemoryStream (#1618) * perf: use byte[] directly instead of MemoryStream * Optimize writing int32 and int64 * Update Assets/Mirror/Runtime/NetworkWriter.cs Co-Authored-By: vis2k * Update Assets/Mirror/Runtime/NetworkWriter.cs Co-Authored-By: vis2k * Update Assets/Mirror/Runtime/NetworkWriter.cs * Start with bigger buffer * Woops, should have double checked suggestion * Removed invalid Test We should not require NetworkWriter to behave in certain way when Position is set to out of bounds. That is an invalid use of NetworkWriter, so NW should be free to do any behavior * smells * Update NetworkWriter.cs Co-authored-by: vis2k --- Assets/Mirror/Runtime/NetworkWriter.cs | 91 +++++++++++++------ .../Mirror/Tests/Editor/NetworkWriterTest.cs | 40 -------- 2 files changed, 64 insertions(+), 67 deletions(-) diff --git a/Assets/Mirror/Runtime/NetworkWriter.cs b/Assets/Mirror/Runtime/NetworkWriter.cs index 0534ca255ee..0f32684cb00 100644 --- a/Assets/Mirror/Runtime/NetworkWriter.cs +++ b/Assets/Mirror/Runtime/NetworkWriter.cs @@ -1,5 +1,4 @@ using System; -using System.IO; using System.Text; using UnityEngine; @@ -12,11 +11,35 @@ public class NetworkWriter // create writer immediately with it's own buffer so no one can mess with it and so that we can resize it. // note: BinaryWriter allocates too much, so we only use a MemoryStream - readonly MemoryStream stream = new MemoryStream(); + // => 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 - public int Position { get { return (int)stream.Position; } set { stream.Position = value; } } + public int Position; + + int length; + + public int Length + { + get => length; + private set + { + EnsureCapacity(value); + this.length = value; + if (Position > length) + Position = length; + } + } + + void EnsureCapacity(int value) + { + if (buffer.Length < value) + { + int capacity = Math.Max(value, buffer.Length * 2); + Array.Resize(ref buffer, capacity); + } + } // MemoryStream has 3 values: Position, Length and Capacity. // Position is used to indicate where we are writing @@ -25,8 +48,9 @@ public class NetworkWriter // ToArray returns all the data we have written, regardless of the current position public byte[] ToArray() { - stream.Flush(); - return stream.ToArray(); + byte[] data = new byte[Length]; + Array.ConstrainedCopy(buffer, 0, data, 0, Length); + return data; } // Gets the serialized data in an ArraySegment @@ -36,51 +60,64 @@ public byte[] ToArray() // while you are using the ArraySegment public ArraySegment ToArraySegment() { - stream.Flush(); - if (stream.TryGetBuffer(out ArraySegment data)) - { - return data; - } - throw new Exception("Cannot expose contents of memory stream. Make sure that MemoryStream buffer is publicly visible (see MemoryStream source code)."); + return new ArraySegment(buffer, 0, length); } // reset both the position and length of the stream, but leaves the capacity the same // so that we can reuse this writer without extra allocations - public void SetLength(long value) + public void SetLength(int value) { - stream.SetLength(value); + this.Length = value; } - public void WriteByte(byte value) => stream.WriteByte(value); + public void WriteByte(byte value) + { + if (Position >= Length) + { + Length += 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) { // no null check because we would need to write size info for that too (hence WriteBytesAndSize) - stream.Write(buffer, offset, count); + if (Position + count > Length) + { + Length = Position + count; + } + Array.ConstrainedCopy(buffer, offset, this.buffer, Position, count); + Position += count; } public void WriteUInt32(uint value) { - WriteByte((byte)value); - WriteByte((byte)(value >> 8)); - WriteByte((byte)(value >> 16)); - WriteByte((byte)(value >> 24)); + EnsureCapacity(Position + 4); + buffer[Position++] = (byte)value; + buffer[Position++] = (byte)(value >> 8); + buffer[Position++] = (byte)(value >> 16); + buffer[Position++] = (byte)(value >> 24); + Length = Math.Max(Length, Position); } public void WriteInt32(int value) => WriteUInt32((uint)value); public void WriteUInt64(ulong value) { - WriteByte((byte)value); - WriteByte((byte)(value >> 8)); - WriteByte((byte)(value >> 16)); - WriteByte((byte)(value >> 24)); - WriteByte((byte)(value >> 32)); - WriteByte((byte)(value >> 40)); - WriteByte((byte)(value >> 48)); - WriteByte((byte)(value >> 56)); + EnsureCapacity(Position + 8); + buffer[Position++] = (byte)value; + buffer[Position++] = (byte)(value >> 8); + buffer[Position++] = (byte)(value >> 16); + buffer[Position++] = (byte)(value >> 24); + buffer[Position++] = (byte)(value >> 32); + buffer[Position++] = (byte)(value >> 40); + buffer[Position++] = (byte)(value >> 48); + buffer[Position++] = (byte)(value >> 56); + Length = Math.Max(Length, Position); } public void WriteInt64(long value) => WriteUInt64((ulong)value); diff --git a/Assets/Mirror/Tests/Editor/NetworkWriterTest.cs b/Assets/Mirror/Tests/Editor/NetworkWriterTest.cs index b0d10eba8fc..96fc876988e 100644 --- a/Assets/Mirror/Tests/Editor/NetworkWriterTest.cs +++ b/Assets/Mirror/Tests/Editor/NetworkWriterTest.cs @@ -101,46 +101,6 @@ public void TestWritingSegmentAndReadingSegment() Assert.That(deserialized.Array[deserialized.Offset + i], Is.EqualTo(segment.Array[segment.Offset + i])); } - [Test] - public void TestOverwritingData() - { - NetworkWriter writer = new NetworkWriter(); - writer.WriteMatrix4x4(Matrix4x4.identity); - writer.WriteDecimal(1.23456789m); - writer.Position += 10; - writer.WriteVector3(Vector3.negativeInfinity); - writer.Position = 46; - // write right at the boundary before SetLength - writer.WriteInt64(0xfeed_babe_c0ffee); - // test that SetLength clears data beyond length - writer.SetLength(50); - // check that jumping leaves 0s between - writer.Position = 100; - writer.WriteString("no worries, m8"); - writer.Position = 64; - writer.WriteBoolean(true); - // check that clipping off the end affect ToArray()'s length - writer.SetLength(128); - byte[] output = writer.ToArray(); - //Debug.Log(BitConverter.ToString(output)); - byte[] expected = { - 0x00, 0x00, 0x80, 0x3F, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x80, 0x3F, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x80, 0x3F, 0x00, 0x00, 0xEE, 0xFF, 0xC0, 0xBE, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x0F, 0x00, 0x6E, 0x6F, 0x20, 0x77, 0x6F, 0x72, 0x72, 0x69, - 0x65, 0x73, 0x2C, 0x20, 0x6D, 0x38, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 - }; - Assert.That(output, Is.EqualTo(expected)); - } - [Test] public void TestSetLengthZeroes() {