Reduce garbage allocation in SyncVar Deserialize when syncvar has no hook

[liamcary]

While profiling my project I found that deserializing sync vars causes small allocations on the client each time the value changes. In my case I had a double and an int that was changing frequently, and each time they changed there was a 40 bytes and 32 byte allocation in GeneratedSyncVarDeserialize. There's no explicit memory allocation in GeneratedSyncVarDeserialize, but removing the Action<T, T> OnChanged parameter fixes the allocation, even though the NetworkBehaviour's DeserializeSyncVar method passes in null. I assume its a boxing operation on the generic delegate when checking if its null.

To fix the allocation, I implemented a NoHook alternative for each of the 4 GeneratedSyncVarDeserialize methods, then updated the NetworkBehaviourProcessor to call the NoHook method for SyncVar's that don't have a hook. Its not the cleanest implementation, and ideally the delegate could be passed in without any allocation, to reduce memory allocations when deserializing syncvars that DO have a hook.

This is a before and after screenshot of the profiler with Deep Profile enabled:

[liamcary]

Out of curiousity, i tried an alternative approach of adding a boolean 'hasOnChange' parameter to see if the garbage allocation only happens when checking if OnChanged is null. Unfortunately it still allocates, so it seems like the presence of the Action<T, T> parameter causes allocations, whether its used or not, and whether its null or not.

[miwarnec]

interesting find, gonna check this soon

[liamcary]

I've investigated this a bit further, and i've now found that this garbage allocation is a new issue introduced with the Defer SyncVar Hooks feature introduced in mirror 96.9.9 (not released to asset store yet). It turns out the presence of the closure created in the "deferredSyncVarHooks.Add(delegate...)" line is responsible for the allocation, regardless of whether that line of code is reached.

This is apparently a compiler decision to create all anonymous method closure instances when you first enter the method, rather than the first time you execute them. There's a relevant stackoverflow discussion about the decision here: https://stackoverflow.com/questions/53732274/why-does-this-lambda-closure-generate-garbage-although-it-is-not-executed-at-run

Fortunately, it's a super easy workaround and much simpler than my current changes in this PR. Since the allocation happens upon entering the method, you can simply move the "deferredSyncVarHooks.Add" logic into a separate method. I've validated this with some test code and the unity profiler, and will update my PR in a moment.