From de7ac40af9e5d6bd49921bf010ecf76d779bb3e2 Mon Sep 17 00:00:00 2001 From: mischa <16416509+miwarnec@users.noreply.github.com> Date: Sat, 10 Aug 2024 11:54:21 +0200 Subject: [PATCH] perf: Weaver automatically uses VarInt compression for all integers in [SyncVar]/[Command]/[Rpc]/SyncList/NetworkMessage/etc. (#3870) * perf: NetworkReader/Writer: read/write collection size headers as VarInt for significant bandwidth reduction! * Weaver: when defining multiple Readers/Writers of same type, choose the one with priority suffix * perf: NetworkReader/Writer: define weaver preferred VarInt compression for integers * [WeaverPriority] attribute and more tests * remove unused --------- Co-authored-by: mischa --- Assets/Mirror/Core/Attributes.cs | 6 ++ Assets/Mirror/Core/NetworkReaderExtensions.cs | 8 +++ Assets/Mirror/Core/NetworkWriterExtensions.cs | 8 +++ Assets/Mirror/Editor/Weaver/Readers.cs | 22 ++++-- Assets/Mirror/Editor/Weaver/Writers.cs | 22 ++++-- .../NetworkWriterCollectionTest.cs | 69 +++++++++++++++++-- .../NetworkReaderWriter/NetworkWriterTest.cs | 2 +- 7 files changed, 122 insertions(+), 15 deletions(-) diff --git a/Assets/Mirror/Core/Attributes.cs b/Assets/Mirror/Core/Attributes.cs index 0aebfbabe..a76adc6ac 100644 --- a/Assets/Mirror/Core/Attributes.cs +++ b/Assets/Mirror/Core/Attributes.cs @@ -92,4 +92,10 @@ public class ShowInInspectorAttribute : Attribute {} /// [AttributeUsage(AttributeTargets.Field)] public class ReadOnlyAttribute : PropertyAttribute {} + + /// + /// When defining multiple Readers/Writers for the same type, indicate which one Weaver must use. + /// + [AttributeUsage(AttributeTargets.Method)] + public class WeaverPriorityAttribute : Attribute {} } diff --git a/Assets/Mirror/Core/NetworkReaderExtensions.cs b/Assets/Mirror/Core/NetworkReaderExtensions.cs index ad496a796..6b9a4128f 100644 --- a/Assets/Mirror/Core/NetworkReaderExtensions.cs +++ b/Assets/Mirror/Core/NetworkReaderExtensions.cs @@ -45,6 +45,14 @@ public static class NetworkReaderExtensions public static ulong ReadULong(this NetworkReader reader) => reader.ReadBlittable(); public static ulong? ReadULongNullable(this NetworkReader reader) => reader.ReadBlittableNullable(); + // ReadInt/UInt/Long/ULong writes full bytes by default. + // define additional "VarInt" versions that Weaver will automatically prefer. + // 99% of the time [SyncVar] ints are small values, which makes this very much worth it. + [WeaverPriority] public static int ReadVarInt(this NetworkReader reader) => (int)Compression.DecompressVarInt(reader); + [WeaverPriority] public static uint ReadVarUInt(this NetworkReader reader) => (uint)Compression.DecompressVarUInt(reader); + [WeaverPriority] public static long ReadVarLong(this NetworkReader reader) => Compression.DecompressVarInt(reader); + [WeaverPriority] public static ulong ReadVarULong(this NetworkReader reader) => Compression.DecompressVarUInt(reader); + public static float ReadFloat(this NetworkReader reader) => reader.ReadBlittable(); public static float? ReadFloatNullable(this NetworkReader reader) => reader.ReadBlittableNullable(); diff --git a/Assets/Mirror/Core/NetworkWriterExtensions.cs b/Assets/Mirror/Core/NetworkWriterExtensions.cs index 280fa890b..34a815104 100644 --- a/Assets/Mirror/Core/NetworkWriterExtensions.cs +++ b/Assets/Mirror/Core/NetworkWriterExtensions.cs @@ -40,6 +40,14 @@ public static class NetworkWriterExtensions public static void WriteULong(this NetworkWriter writer, ulong value) => writer.WriteBlittable(value); public static void WriteULongNullable(this NetworkWriter writer, ulong? value) => writer.WriteBlittableNullable(value); + // WriteInt/UInt/Long/ULong writes full bytes by default. + // define additional "VarInt" versions that Weaver will automatically prefer. + // 99% of the time [SyncVar] ints are small values, which makes this very much worth it. + [WeaverPriority] public static void WriteVarInt(this NetworkWriter writer, int value) => Compression.CompressVarInt(writer, value); + [WeaverPriority] public static void WriteVarUInt(this NetworkWriter writer, uint value) => Compression.CompressVarUInt(writer, value); + [WeaverPriority] public static void WriteVarLong(this NetworkWriter writer, long value) => Compression.CompressVarInt(writer, value); + [WeaverPriority] public static void WriteVarULong(this NetworkWriter writer, ulong value) => Compression.CompressVarUInt(writer, value); + public static void WriteFloat(this NetworkWriter writer, float value) => writer.WriteBlittable(value); public static void WriteFloatNullable(this NetworkWriter writer, float? value) => writer.WriteBlittableNullable(value); diff --git a/Assets/Mirror/Editor/Weaver/Readers.cs b/Assets/Mirror/Editor/Weaver/Readers.cs index 33941d9f9..9963f6422 100644 --- a/Assets/Mirror/Editor/Weaver/Readers.cs +++ b/Assets/Mirror/Editor/Weaver/Readers.cs @@ -33,12 +33,24 @@ public Readers(AssemblyDefinition assembly, WeaverTypes weaverTypes, TypeDefinit internal void Register(TypeReference dataType, MethodReference methodReference) { - if (readFuncs.ContainsKey(dataType)) + // sometimes we define multiple read methods for the same type. + // for example: + // ReadInt() // alwasy writes 4 bytes: should be available to the user for binary protocols etc. + // ReadVarInt() // varint compression: we may want Weaver to always use this for minimal bandwidth + // give the user a way to define the weaver prefered one if two exists: + // "[WeaverPriority]" attribute is automatically detected and prefered. + MethodDefinition methodDefinition = methodReference.Resolve(); + bool priority = methodDefinition.HasCustomAttribute(); + // if (priority) Log.Warning($"Weaver: Registering priority Read<{dataType.FullName}> with {methodReference.FullName}.", methodReference); + + // Weaver sometimes calls Register for multiple times because we resolve assemblies multiple times. + // if the function name is the same: always use the latest one. + // if the function name differes: use the priority one. + if (readFuncs.TryGetValue(dataType, out MethodReference existingMethod) && // if it was already defined + existingMethod.FullName != methodReference.FullName && // and this one is a different name + !priority) // and it's not the priority one { - // TODO enable this again later. - // Reader has some obsolete functions that were renamed. - // Don't want weaver warnings for all of them. - //Log.Warning($"Registering a Read method for {dataType.FullName} when one already exists", methodReference); + return; // then skip } // we need to import type when we Initialize Readers so import here in case it is used anywhere else diff --git a/Assets/Mirror/Editor/Weaver/Writers.cs b/Assets/Mirror/Editor/Weaver/Writers.cs index 8311a2615..a1cb00eb2 100644 --- a/Assets/Mirror/Editor/Weaver/Writers.cs +++ b/Assets/Mirror/Editor/Weaver/Writers.cs @@ -33,12 +33,24 @@ public Writers(AssemblyDefinition assembly, WeaverTypes weaverTypes, TypeDefinit public void Register(TypeReference dataType, MethodReference methodReference) { - if (writeFuncs.ContainsKey(dataType)) + // sometimes we define multiple write methods for the same type. + // for example: + // WriteInt() // alwasy writes 4 bytes: should be available to the user for binary protocols etc. + // WriteVarInt() // varint compression: we may want Weaver to always use this for minimal bandwidth + // give the user a way to define the weaver prefered one if two exists: + // "[WeaverPriority]" attribute is automatically detected and prefered. + MethodDefinition methodDefinition = methodReference.Resolve(); + bool priority = methodDefinition.HasCustomAttribute(); + // if (priority) Log.Warning($"Weaver: Registering priority Write<{dataType.FullName}> with {methodReference.FullName}.", methodReference); + + // Weaver sometimes calls Register for multiple times because we resolve assemblies multiple times. + // if the function name is the same: always use the latest one. + // if the function name differes: use the priority one. + if (writeFuncs.TryGetValue(dataType, out MethodReference existingMethod) && // if it was already defined + existingMethod.FullName != methodReference.FullName && // and this one is a different name + !priority) // and it's not the priority one { - // TODO enable this again later. - // Writer has some obsolete functions that were renamed. - // Don't want weaver warnings for all of them. - //Log.Warning($"Registering a Write method for {dataType.FullName} when one already exists", methodReference); + return; // then skip } // we need to import type when we Initialize Writers so import here in case it is used anywhere else diff --git a/Assets/Mirror/Tests/Editor/NetworkReaderWriter/NetworkWriterCollectionTest.cs b/Assets/Mirror/Tests/Editor/NetworkReaderWriter/NetworkWriterCollectionTest.cs index 6a708270f..29a9511e9 100644 --- a/Assets/Mirror/Tests/Editor/NetworkReaderWriter/NetworkWriterCollectionTest.cs +++ b/Assets/Mirror/Tests/Editor/NetworkReaderWriter/NetworkWriterCollectionTest.cs @@ -6,24 +6,85 @@ namespace Mirror.Tests.NetworkReaderWriter { public class NetworkWriterCollectionTest { + // we defined WriteInt and WriteVarInt. make sure it's using the priority one. [Test] - public void HasWriteFunctionForInt() + public void UsesPriorityWriteFunctionForInt() { Assert.That(Writer.write, Is.Not.Null, "int write function was not found"); - Action action = NetworkWriterExtensions.WriteInt; + Action action = NetworkWriterExtensions.WriteVarInt; Assert.That(Writer.write, Is.EqualTo(action), "int write function was incorrect value"); } + // we defined ReadInt and ReadVarInt. make sure it's using the priority one. [Test] - public void HasReadFunctionForInt() + public void UsesPriorityReadFunctionForInt() { Assert.That(Reader.read, Is.Not.Null, "int read function was not found"); - Func action = NetworkReaderExtensions.ReadInt; + Func action = NetworkReaderExtensions.ReadVarInt; Assert.That(Reader.read, Is.EqualTo(action), "int read function was incorrect value"); } + // we defined WriteInt and WriteVarUInt. make sure it's using the priority one. + [Test] + public void UsesPriorityWriteFunctionForUInt() + { + Assert.That(Writer.write, Is.Not.Null, "uint write function was not found"); + + Action action = NetworkWriterExtensions.WriteVarUInt; + Assert.That(Writer.write, Is.EqualTo(action), "uint write function was incorrect value"); + } + + // we defined ReadInt and ReadVarInt. make sure it's using the priority one. + [Test] + public void UsesPriorityReadFunctionForUInt() + { + Assert.That(Reader.read, Is.Not.Null, "uint read function was not found"); + + Func action = NetworkReaderExtensions.ReadVarUInt; + Assert.That(Reader.read, Is.EqualTo(action), "uint read function was incorrect value"); + } + // we defined WriteInt and WriteVarInt. make sure it's using the priority one. + [Test] + public void UsesPriorityWriteFunctionForLong() + { + Assert.That(Writer.write, Is.Not.Null, "long write function was not found"); + + Action action = NetworkWriterExtensions.WriteVarLong; + Assert.That(Writer.write, Is.EqualTo(action), "long write function was incorrect value"); + } + + // we defined ReadLong and ReadVarLong. make sure it's using the priority one. + [Test] + public void UsesPriorityReadFunctionForLong() + { + Assert.That(Reader.read, Is.Not.Null, "long read function was not found"); + + Func action = NetworkReaderExtensions.ReadVarLong; + Assert.That(Reader.read, Is.EqualTo(action), "long read function was incorrect value"); + } + + // we defined WriteLong and WriteVarULong. make sure it's using the priority one. + [Test] + public void UsesPriorityWriteFunctionForULong() + { + Assert.That(Writer.write, Is.Not.Null, "ulong write function was not found"); + + Action action = NetworkWriterExtensions.WriteVarULong; + Assert.That(Writer.write, Is.EqualTo(action), "ulong write function was incorrect value"); + } + + // we defined ReadLong and ReadVarLong. make sure it's using the priority one. + [Test] + public void UsesPriorityReadFunctionForULong() + { + Assert.That(Reader.read, Is.Not.Null, "ulong read function was not found"); + + Func action = NetworkReaderExtensions.ReadVarULong; + Assert.That(Reader.read, Is.EqualTo(action), "ulong read function was incorrect value"); + } + [Test] public void HasWriteNetworkBehaviourFunction() { diff --git a/Assets/Mirror/Tests/Editor/NetworkReaderWriter/NetworkWriterTest.cs b/Assets/Mirror/Tests/Editor/NetworkReaderWriter/NetworkWriterTest.cs index a1f5cbd38..95fd5394b 100644 --- a/Assets/Mirror/Tests/Editor/NetworkReaderWriter/NetworkWriterTest.cs +++ b/Assets/Mirror/Tests/Editor/NetworkReaderWriter/NetworkWriterTest.cs @@ -1405,7 +1405,7 @@ public void TestArrayThrowsIfLengthIsWrong(int badLength) void WriteBadArray() { // Reader/Writer encode null as count=0 and [] as count=1 (+1 offset) - writer.WriteUInt((uint)(badLength+1)); + writer.WriteVarUInt((uint)(badLength+1)); // Reader/Writer encode size headers as VarInt int[] array = new int[testArraySize] { 1, 2, 3, 4 }; for (int i = 0; i < array.Length; i++) writer.Write(array[i]);