From 1245f0b0da94c01dbbf78bc8d72f28609b05c0f0 Mon Sep 17 00:00:00 2001 From: ruccho Date: Wed, 5 Feb 2025 15:34:07 +0900 Subject: [PATCH 1/3] reduce type resolution for extension methods in NetworkBehaviourILPP --- .../Editor/CodeGen/NetworkBehaviourILPP.cs | 39 ++++++++++++++++--- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs b/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs index 1734d02311..bfde8b7148 100644 --- a/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs +++ b/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs @@ -968,11 +968,42 @@ private bool ImportReferences(ModuleDefinition moduleDefinition, string[] assemb continue; } + // to reduce unnecessary type resolution, we first check if it can be an extension without type resolution + + var mayHaveExtension = false; + + foreach (var attr in method.CustomAttributes) + { + if (attr.Constructor.DeclaringType.FullName == extensionConstructor.DeclaringType.FullName) + { + mayHaveExtension = true; + break; + } + } + + if (!mayHaveExtension) + { + continue; + } + + var parameters = method.Parameters; + + if (parameters.Count != 2 || parameters[0].ParameterType is not ByReferenceType byRefType) + { + continue; + } + + if (byRefType.ElementType.FullName != m_FastBufferWriter_TypeRef.FullName && byRefType.ElementType.FullName != m_FastBufferReader_TypeRef.FullName) + { + continue; + } + var isExtension = false; foreach (var attr in method.CustomAttributes) { - if (attr.Constructor.Resolve() == extensionConstructor.Resolve()) + if (attr.Constructor.DeclaringType.FullName == extensionConstructor.DeclaringType.FullName && + attr.Constructor.Resolve() == extensionConstructor.Resolve()) { isExtension = true; } @@ -983,13 +1014,11 @@ private bool ImportReferences(ModuleDefinition moduleDefinition, string[] assemb continue; } - var parameters = method.Parameters; - - if (parameters.Count == 2 && parameters[0].ParameterType.Resolve() == m_FastBufferWriter_TypeRef.MakeByReferenceType().Resolve()) + if (parameters[0].ParameterType.Resolve() == m_FastBufferWriter_TypeRef.MakeByReferenceType().Resolve()) { m_FastBufferWriter_ExtensionMethodRefs.Add(m_MainModule.ImportReference(method)); } - else if (parameters.Count == 2 && parameters[0].ParameterType.Resolve() == m_FastBufferReader_TypeRef.MakeByReferenceType().Resolve()) + else if (parameters[0].ParameterType.Resolve() == m_FastBufferReader_TypeRef.MakeByReferenceType().Resolve()) { m_FastBufferReader_ExtensionMethodRefs.Add(m_MainModule.ImportReference(method)); } From 52c5f669cc44a6f7a506a26a7f56425d41bb82b0 Mon Sep 17 00:00:00 2001 From: ruccho Date: Wed, 5 Feb 2025 15:56:32 +0900 Subject: [PATCH 2/3] match by name earlier --- .../Editor/CodeGen/NetworkBehaviourILPP.cs | 55 ++++++++++--------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs b/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs index bfde8b7148..7e95fea9ca 100644 --- a/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs +++ b/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs @@ -963,37 +963,42 @@ private bool ImportReferences(ModuleDefinition moduleDefinition, string[] assemb foreach (var method in type.Methods) { + // to reduce unnecessary type resolution, we first check if it can be a serialization extension without type resolution + if (!method.IsStatic) { continue; } - // to reduce unnecessary type resolution, we first check if it can be an extension without type resolution + if (method.Name is not (k_WriteValueMethodName or k_ReadValueMethodName)) + { + continue; + } - var mayHaveExtension = false; + var mayHaveExtensionAttr = false; foreach (var attr in method.CustomAttributes) { if (attr.Constructor.DeclaringType.FullName == extensionConstructor.DeclaringType.FullName) { - mayHaveExtension = true; + mayHaveExtensionAttr = true; break; } } - if (!mayHaveExtension) + if (!mayHaveExtensionAttr) { continue; } var parameters = method.Parameters; - if (parameters.Count != 2 || parameters[0].ParameterType is not ByReferenceType byRefType) + if (parameters.Count != 2 || parameters[0].ParameterType is not ByReferenceType firstParameterType) { continue; } - if (byRefType.ElementType.FullName != m_FastBufferWriter_TypeRef.FullName && byRefType.ElementType.FullName != m_FastBufferReader_TypeRef.FullName) + if (firstParameterType.ElementType.FullName != m_FastBufferWriter_TypeRef.FullName && firstParameterType.ElementType.FullName != m_FastBufferReader_TypeRef.FullName) { continue; } @@ -1014,11 +1019,11 @@ private bool ImportReferences(ModuleDefinition moduleDefinition, string[] assemb continue; } - if (parameters[0].ParameterType.Resolve() == m_FastBufferWriter_TypeRef.MakeByReferenceType().Resolve()) + if (method.Name == k_WriteValueMethodName && firstParameterType.Resolve() == m_FastBufferWriter_TypeRef.MakeByReferenceType().Resolve()) { m_FastBufferWriter_ExtensionMethodRefs.Add(m_MainModule.ImportReference(method)); } - else if (parameters[0].ParameterType.Resolve() == m_FastBufferReader_TypeRef.MakeByReferenceType().Resolve()) + else if (method.Name == k_ReadValueMethodName && firstParameterType.Resolve() == m_FastBufferReader_TypeRef.MakeByReferenceType().Resolve()) { m_FastBufferReader_ExtensionMethodRefs.Add(m_MainModule.ImportReference(method)); } @@ -1818,27 +1823,24 @@ private bool GetWriteMethodForParameter(TypeReference paramType, out MethodRefer { var parameters = method.Resolve().Parameters; - if (method.Name == k_WriteValueMethodName) + if (parameters[1].IsIn) { - if (parameters[1].IsIn) + if (((ByReferenceType)parameters[1].ParameterType).ElementType.FullName == paramType.FullName && + ((ByReferenceType)parameters[1].ParameterType).ElementType.IsArray == paramType.IsArray) { - if (((ByReferenceType)parameters[1].ParameterType).ElementType.FullName == paramType.FullName && - ((ByReferenceType)parameters[1].ParameterType).ElementType.IsArray == paramType.IsArray) - { - methodRef = method; - m_FastBufferWriter_WriteValue_MethodRefs[assemblyQualifiedName] = methodRef; - return true; - } + methodRef = method; + m_FastBufferWriter_WriteValue_MethodRefs[assemblyQualifiedName] = methodRef; + return true; } - else + } + else + { + if (parameters[1].ParameterType.FullName == paramType.FullName && + parameters[1].ParameterType.IsArray == paramType.IsArray) { - if (parameters[1].ParameterType.FullName == paramType.FullName && - parameters[1].ParameterType.IsArray == paramType.IsArray) - { - methodRef = method; - m_FastBufferWriter_WriteValue_MethodRefs[assemblyQualifiedName] = methodRef; - return true; - } + methodRef = method; + m_FastBufferWriter_WriteValue_MethodRefs[assemblyQualifiedName] = methodRef; + return true; } } } @@ -2054,8 +2056,7 @@ private bool GetReadMethodForParameter(TypeReference paramType, out MethodRefere foreach (var method in m_FastBufferReader_ExtensionMethodRefs) { var parameters = method.Resolve().Parameters; - if (method.Name == k_ReadValueMethodName && - parameters[1].IsOut && + if (parameters[1].IsOut && ((ByReferenceType)parameters[1].ParameterType).ElementType.FullName == paramType.FullName && ((ByReferenceType)parameters[1].ParameterType).ElementType.IsArray == paramType.IsArray) { From d4cf5c5fc172fdbf3a3ca7b4856fedbe88aea389 Mon Sep 17 00:00:00 2001 From: ruccho Date: Thu, 6 Feb 2025 09:12:46 +0900 Subject: [PATCH 3/3] fixed serialization method resolution for non-by-ref first parameter --- .../Editor/CodeGen/NetworkBehaviourILPP.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs b/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs index 7e95fea9ca..3d56d5136c 100644 --- a/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs +++ b/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs @@ -993,12 +993,17 @@ private bool ImportReferences(ModuleDefinition moduleDefinition, string[] assemb var parameters = method.Parameters; - if (parameters.Count != 2 || parameters[0].ParameterType is not ByReferenceType firstParameterType) + if (parameters.Count != 2) { continue; } - if (firstParameterType.ElementType.FullName != m_FastBufferWriter_TypeRef.FullName && firstParameterType.ElementType.FullName != m_FastBufferReader_TypeRef.FullName) + var firstParameterType = parameters[0].ParameterType; + + // ReadValueSafe() and WriteValueSafe() can use both by-ref and non-by-ref type for the first parameter type + var firstParameterElementType = firstParameterType is ByReferenceType byRefType ? byRefType.ElementType : firstParameterType; + + if (firstParameterElementType.FullName != m_FastBufferWriter_TypeRef.FullName && firstParameterElementType.FullName != m_FastBufferReader_TypeRef.FullName) { continue; } @@ -1011,6 +1016,7 @@ private bool ImportReferences(ModuleDefinition moduleDefinition, string[] assemb attr.Constructor.Resolve() == extensionConstructor.Resolve()) { isExtension = true; + break; } }