Skip to content

Commit

Permalink
perf: SyncVar hook invocations no longer instantiate a new action del…
Browse files Browse the repository at this point in the history 
…egate on every call (#3615)

Co-authored-by: Clayton Hunsinger <clayton@talofagames.com>
  • Loading branch information
@cshunsinger @ClaytonHunsinger
cshunsinger and ClaytonHunsinger authored Dec 6, 2023
1 parent ab99215 commit 608429c
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 20 deletions.
28 changes: 21 additions & 7 deletions Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ class NetworkBehaviourProcessor
List<FieldDefinition> syncObjects = new List<FieldDefinition>();
// <SyncVarField,NetIdField>
Dictionary<FieldDefinition, FieldDefinition> syncVarNetIds = new Dictionary<FieldDefinition, FieldDefinition>();
// <SyncVarHookDelegateField, (FieldDefinition, MethodDefinition)> - Every syncvar with a hook has a new field created to store the Action<T,T> delegate so we don't allocate on every hook invocation
// This dictionary maps each syncvar field to the field that will store the hook method delegate instance, and the method from which the delegate instance is constructed from
Dictionary<FieldDefinition, (FieldDefinition hookDelegateField, MethodDefinition hookMethod)> syncVarHookDelegates = new Dictionary<FieldDefinition, (FieldDefinition hookDelegateField, MethodDefinition hookMethod)>();
readonly List<CmdResult> commands = new List<CmdResult>();
readonly List<ClientRpcResult> clientRpcs = new List<ClientRpcResult>();
readonly List<MethodDefinition> targetRpcs = new List<MethodDefinition>();
Expand Down Expand Up @@ -71,7 +74,7 @@ public bool Process(ref bool WeavingFailed)
MarkAsProcessed(netBehaviourSubclass);

// deconstruct tuple and set fields
(syncVars, syncVarNetIds) = syncVarAttributeProcessor.ProcessSyncVars(netBehaviourSubclass, ref WeavingFailed);
(syncVars, syncVarNetIds, syncVarHookDelegates) = syncVarAttributeProcessor.ProcessSyncVars(netBehaviourSubclass, ref WeavingFailed);

syncObjects = SyncObjectProcessor.FindSyncObjectsFields(writers, readers, Log, netBehaviourSubclass, ref WeavingFailed);

Expand Down Expand Up @@ -321,7 +324,7 @@ void InjectIntoStaticConstructor(ref bool WeavingFailed)
// we need to inject several initializations into NetworkBehaviour ctor
void InjectIntoInstanceConstructor(ref bool WeavingFailed)
{
if (syncObjects.Count == 0)
if ((syncObjects.Count == 0) && (syncVarHookDelegates.Count == 0))
return;

// find instance constructor
Expand Down Expand Up @@ -349,6 +352,14 @@ void InjectIntoInstanceConstructor(ref bool WeavingFailed)
SyncObjectInitializer.GenerateSyncObjectInitializer(ctorWorker, weaverTypes, fd);
}

// initialize all delegate fields in ctor
foreach(KeyValuePair<FieldDefinition, (FieldDefinition, MethodDefinition)> entry in syncVarHookDelegates)
{
FieldDefinition syncVarField = entry.Key;
(FieldDefinition hookDelegate, MethodDefinition hookMethod) = entry.Value;
syncVarAttributeProcessor.GenerateSyncVarHookDelegateInitializer(ctorWorker, syncVarField, hookDelegate, hookMethod);
}

// add final 'Ret' instruction to ctor
ctorWorker.Append(ctorWorker.Create(OpCodes.Ret));
}
Expand Down Expand Up @@ -582,15 +593,18 @@ void DeserializeField(FieldDefinition syncVar, ILProcessor worker, ref bool Weav
worker.Emit(OpCodes.Ldflda, syncVar);
}

// hook? then push 'new Action<T,T>(Hook)' onto stack
MethodDefinition hookMethod = syncVarAttributeProcessor.GetHookMethod(netBehaviourSubclass, syncVar, ref WeavingFailed);
if (hookMethod != null)
// If a hook exists, then we need to load the hook delegate on the stack
// The hook delegate is created once in the constructor and stored in an instance field
// We load the delegate from this instance field to avoid instantiating a new delegate instance every time (drastically reduces allocations)
if(syncVarHookDelegates.TryGetValue(syncVar, out (FieldDefinition hookDelegateField, MethodDefinition) value))
{
syncVarAttributeProcessor.GenerateNewActionFromHookMethod(syncVar, worker, hookMethod);
// A hook exists. Push this.hookDelegateField onto the stack
worker.Emit(OpCodes.Ldarg_0);
worker.Emit(OpCodes.Ldfld, value.hookDelegateField);
}
// otherwise push 'null' as hook
else
{
// No hook exists. Push 'null' as hook
worker.Emit(OpCodes.Ldnull);
}

Expand Down
58 changes: 45 additions & 13 deletions Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,26 @@ public MethodDefinition GetHookMethod(TypeDefinition td, FieldDefinition syncVar
return FindHookMethod(td, syncVar, hookFunctionName, ref WeavingFailed);
}

// Create a field definition for a field that will store the Action<T, T> delegate instance for the syncvar hook method (only instantiate delegate once)
public FieldDefinition CreateNewActionFieldDefinitionFromHookMethod(FieldDefinition syncVarField)
{
TypeReference actionRef = assembly.MainModule.ImportReference(typeof(Action<,>));
GenericInstanceType syncVarHookActionDelegateType = actionRef.MakeGenericInstanceType(syncVarField.FieldType, syncVarField.FieldType);
string syncVarHookDelegateFieldName = $"_Mirror_SyncVarHookDelegate_{syncVarField.Name}";
return new FieldDefinition(syncVarHookDelegateFieldName, FieldAttributes.Public, syncVarHookActionDelegateType);
}

// push hook from GetHookMethod() onto the stack as a new Action<T,T>.
// allows for reuse without handling static/virtual cases every time.
// perf warning: it is recommended to use this method only when generating IL to create a new Action<T, T>() in order to store it into a field
// avoid using this to emit IL to instantiate a new action instance every single time one is needed for the same method
public void GenerateNewActionFromHookMethod(FieldDefinition syncVar, ILProcessor worker, MethodDefinition hookMethod)
{
// IL_000a: ldarg.0
// IL_000b: ldftn instance void Mirror.Examples.Tanks.Tank::ExampleHook(int32, int32)
// IL_0011: newobj instance void class [netstandard]System.Action`2<int32, int32>::.ctor(object, native int)

// we support static hook sand instance hooks.
// we support static hooks and instance hooks.
if (hookMethod.IsStatic)
{
// for static hooks, we need to push 'null' first.
Expand Down Expand Up @@ -95,15 +106,23 @@ public void GenerateNewActionFromHookMethod(FieldDefinition syncVar, ILProcessor

// call 'new Action<T,T>()' constructor to convert the function to an action
// we need to make an instance of the generic Action<T,T>.
//
// TODO this allocates a new 'Action' for every SyncVar hook call.
// we should allocate it once and store it somewhere in the future.
// hooks are only called on the client though, so it's not too bad for now.
TypeReference actionRef = assembly.MainModule.ImportReference(typeof(Action<,>));
GenericInstanceType genericInstance = actionRef.MakeGenericInstanceType(syncVar.FieldType, syncVar.FieldType);
worker.Emit(OpCodes.Newobj, weaverTypes.ActionT_T.MakeHostInstanceGeneric(assembly.MainModule, genericInstance));
}

// generates CIL to set an Action<T,T> instance field to a new Action<T,T>(hookMethod)
// this.hookDelegate = new Action<T, T>(HookMethod);
public void GenerateSyncVarHookDelegateInitializer(ILProcessor worker, FieldDefinition syncVar, FieldDefinition hookDelegate, MethodDefinition hookMethod)
{
// push this
worker.Emit(OpCodes.Ldarg_0);
// push new Action<T, T>(hookMethod)
GenerateNewActionFromHookMethod(syncVar, worker, hookMethod);
// set field
worker.Emit(OpCodes.Stfld, hookDelegate);
}

MethodDefinition FindHookMethod(TypeDefinition td, FieldDefinition syncVar, string hookFunctionName, ref bool WeavingFailed)
{
List<MethodDefinition> methods = td.GetMethods(hookFunctionName);
Expand Down Expand Up @@ -242,7 +261,7 @@ public MethodDefinition GenerateSyncVarGetter(FieldDefinition fd, string origina
// }
//
// the setter used to be manually IL generated, but we moved it to C# :)
public MethodDefinition GenerateSyncVarSetter(TypeDefinition td, FieldDefinition fd, string originalName, long dirtyBit, FieldDefinition netFieldId, ref bool WeavingFailed)
public MethodDefinition GenerateSyncVarSetter(TypeDefinition td, FieldDefinition fd, string originalName, long dirtyBit, FieldDefinition netFieldId, Dictionary<FieldDefinition, (FieldDefinition hookDelegateField, MethodDefinition hookMethod)> syncVarHookDelegates, ref bool WeavingFailed)
{
//Create the set method
MethodDefinition set = new MethodDefinition($"set_Network{originalName}", MethodAttributes.Public |
Expand Down Expand Up @@ -304,11 +323,17 @@ public MethodDefinition GenerateSyncVarSetter(TypeDefinition td, FieldDefinition
// push the dirty bit for this SyncVar
worker.Emit(OpCodes.Ldc_I8, dirtyBit);

// hook? then push 'new Action<T,T>(Hook)' onto stack
// hook? then push 'this.HookDelegate' onto stack
MethodDefinition hookMethod = GetHookMethod(td, fd, ref WeavingFailed);
if (hookMethod != null)
{
GenerateNewActionFromHookMethod(fd, worker, hookMethod);
// Create the field that will store a single instance of the hook as a delegate (field will be set in constructor)
FieldDefinition hookActionDelegateField = CreateNewActionFieldDefinitionFromHookMethod(fd);
syncVarHookDelegates[fd] = (hookActionDelegateField, hookMethod);

// push this.hookActionDelegateField
worker.Emit(OpCodes.Ldarg_0);
worker.Emit(OpCodes.Ldfld, hookActionDelegateField);
}
// otherwise push 'null' as hook
else
Expand Down Expand Up @@ -362,7 +387,7 @@ public MethodDefinition GenerateSyncVarSetter(TypeDefinition td, FieldDefinition
return set;
}

public void ProcessSyncVar(TypeDefinition td, FieldDefinition fd, Dictionary<FieldDefinition, FieldDefinition> syncVarNetIds, long dirtyBit, ref bool WeavingFailed)
public void ProcessSyncVar(TypeDefinition td, FieldDefinition fd, Dictionary<FieldDefinition, FieldDefinition> syncVarNetIds, Dictionary<FieldDefinition, (FieldDefinition hookDelegateField, MethodDefinition hookMethod)> syncVarHookDelegates, long dirtyBit, ref bool WeavingFailed)
{
string originalName = fd.Name;

Expand Down Expand Up @@ -391,7 +416,7 @@ public void ProcessSyncVar(TypeDefinition td, FieldDefinition fd, Dictionary<Fie
}

MethodDefinition get = GenerateSyncVarGetter(fd, originalName, netIdField);
MethodDefinition set = GenerateSyncVarSetter(td, fd, originalName, dirtyBit, netIdField, ref WeavingFailed);
MethodDefinition set = GenerateSyncVarSetter(td, fd, originalName, dirtyBit, netIdField, syncVarHookDelegates, ref WeavingFailed);

//NOTE: is property even needed? Could just use a setter function?
//create the property
Expand All @@ -417,10 +442,11 @@ public void ProcessSyncVar(TypeDefinition td, FieldDefinition fd, Dictionary<Fie
}
}

public (List<FieldDefinition> syncVars, Dictionary<FieldDefinition, FieldDefinition> syncVarNetIds) ProcessSyncVars(TypeDefinition td, ref bool WeavingFailed)
public (List<FieldDefinition> syncVars, Dictionary<FieldDefinition, FieldDefinition> syncVarNetIds, Dictionary<FieldDefinition, (FieldDefinition hookDelegateField, MethodDefinition hookMethod)> syncVarHookDelegates) ProcessSyncVars(TypeDefinition td, ref bool WeavingFailed)
{
List<FieldDefinition> syncVars = new List<FieldDefinition>();
Dictionary<FieldDefinition, FieldDefinition> syncVarNetIds = new Dictionary<FieldDefinition, FieldDefinition>();
Dictionary<FieldDefinition, (FieldDefinition hookDelegateField, MethodDefinition hookMethod)> syncVarHookDelegates = new Dictionary<FieldDefinition, (FieldDefinition hookDelegateField, MethodDefinition hookMethod)>();

// the mapping of dirtybits to sync-vars is implicit in the order of the fields here. this order is recorded in m_replacementProperties.
// start assigning syncvars at the place the base class stopped, if any
Expand Down Expand Up @@ -460,7 +486,7 @@ public void ProcessSyncVar(TypeDefinition td, FieldDefinition fd, Dictionary<Fie
{
syncVars.Add(fd);

ProcessSyncVar(td, fd, syncVarNetIds, 1L << dirtyBitCounter, ref WeavingFailed);
ProcessSyncVar(td, fd, syncVarNetIds, syncVarHookDelegates, 1L << dirtyBitCounter, ref WeavingFailed);
dirtyBitCounter += 1;

if (dirtyBitCounter > SyncVarLimit)
Expand All @@ -479,12 +505,18 @@ public void ProcessSyncVar(TypeDefinition td, FieldDefinition fd, Dictionary<Fie
td.Fields.Add(fd);
}

// add all of the new SyncVar Action<T,T> fields
foreach((FieldDefinition hookDelegateInstanceField, MethodDefinition) entry in syncVarHookDelegates.Values)
{
td.Fields.Add(entry.hookDelegateInstanceField);
}

// include parent class syncvars
// fixes: https://github.com/MirrorNetworking/Mirror/issues/3457
int parentSyncVarCount = syncVarAccessLists.GetSyncVarStart(td.BaseType.FullName);
syncVarAccessLists.SetNumSyncVars(td.FullName, parentSyncVarCount + syncVars.Count);

return (syncVars, syncVarNetIds);
return (syncVars, syncVarNetIds, syncVarHookDelegates);
}
}
}

0 comments on commit 608429c

Please sign in to comment.