perf: Remove reflection from NetworkBehaviour code to improve performance. [MTT-1968] #2522
Conversation
ShadauxCat
changed the title
| @@ -18,6 +18,7 @@ Additional documentation and release notes are available at [Multiplayer Documen | |||
| - Fixed some errors that could occur if a connection is lost and the loss is detected when attempting to write to the socket. (#2495) | |||
|
|
|||
| ## Changed | |||
| - Improved performance of NetworkBehaviour initialization, which should help reduce hitching during additive scene loads. (#2522) | |||
There was a problem hiding this comment.
Improved performance of NetworkBehaviour initialization...
perhaps adding "by bla bla, which should..." might be more informative, but just nit-picking on this.
| m_NetworkVariableBase_TypeRef = moduleDefinition.ImportReference(networkVariableBaseTypeDef); | ||
| foreach (var methodDef in networkVariableBaseTypeDef.Methods) | ||
| { | ||
| Console.WriteLine($"=>Method: {methodDef.Name}"); |
There was a problem hiding this comment.
probably a debugging leftover :P
There was a problem hiding this comment.
Yep, removed, thanks for the catch.
| } | ||
| if (field.FieldType.IsSubclassOf(m_NetworkVariableBase_TypeRef)) | ||
| { | ||
| processor.Emit(OpCodes.Ldarg_0); |
There was a problem hiding this comment.
I usually find inline pseudo-C# code helpful understanding what was tried to be achieved with IL emits.
There was a problem hiding this comment.
Nice... now I can understand what the OpCodes actually mean! 😸
(TILT)
| initializeVariablesBaseReference = initializeVariablesBaseReference.MakeGeneric(baseTypeInstance.GenericArguments.ToArray()); | ||
| } | ||
|
|
||
| processor.Emit(OpCodes.Nop); |
| var sortedFields = GetFieldInfoForType(GetType()); | ||
| for (int i = 0; i < sortedFields.Length; i++) | ||
| { | ||
| var fieldType = sortedFields[i].FieldType; | ||
| if (fieldType.IsSubclassOf(typeof(NetworkVariableBase))) | ||
| { | ||
| var instance = (NetworkVariableBase)sortedFields[i].GetValue(this) ?? throw new Exception($"{GetType().FullName}.{sortedFields[i].Name} cannot be null. All {nameof(NetworkVariableBase)} instances must be initialized."); | ||
| instance.Initialize(this); | ||
|
|
||
| var instanceNameProperty = fieldType.GetProperty(nameof(NetworkVariableBase.Name)); | ||
| var sanitizedVariableName = sortedFields[i].Name.Replace("<", string.Empty).Replace(">k__BackingField", string.Empty); | ||
| instanceNameProperty?.SetValue(instance, sanitizedVariableName); | ||
|
|
||
| NetworkVariableFields.Add(instance); | ||
| } | ||
| } |
There was a problem hiding this comment.
what this code was doing anyway? why did we try to access variable name?
There was a problem hiding this comment.
Based on the tests that failed when I forgot to bring over the "k__BackingField" part of this, it seems that the network variable's name is used for tools/metrics. This is just setting a runtime property to match the variable's name in the source, so basically it's doing Foo.Name = "Foo".
- Added an extra error handler to catch an edge case I missed before - Added a bit more info to the changelog - Fixed testproject-tools-integration tests
com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs
Outdated
Show resolved
Hide resolved
com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Fatih Mar <mfatihmar@gmail.com>
Changelog
Testing and Documentation