Add new patch to automatically add tracing spans to stock events#355
Open
Phantomical wants to merge 1 commit intoKSPModdingLibs:devfrom
Open
Add new patch to automatically add tracing spans to stock events#355Phantomical wants to merge 1 commit intoKSPModdingLibs:devfrom
Phantomical wants to merge 1 commit intoKSPModdingLibs:devfrom
Conversation
When looking at KSP within the unity profiler the event handlers are completely invisible. This is a shame, because a lot of frame hangs or stutter happen because an event was fired and event handlers end up running unnecessarily. This commit introduces a new disabled-by-default patch that automatically adds two sets of profiler spans every time the event is fired: - One wraps the whole event and contains the event name - We wrap each individual function call with another span that contains the full name (i.e. `typename.methodname`) of the method being called. That way you can see right away which events and and which callback specifically are slow. Actually implementing this is a bit messy. EventVoid is easy, it can just use a regular harmony patch, but the remaining EventData types are all generics. Patching generics in general is difficult. You lose class type parameters and you need to separately patch each instantiation for value type parameters. Making this actually work in practice for stock is somewhat involved - We declare `Fire0` through `Fire4`. These are instantiated with the actual type parameters dynamically are basically just a copy-paste of the stock event handling code with the extra spans added. - We declare `EventDataN_Fire_Override` variants, these are the calls that actually get patched into the resulting method. Their generic parameters will match for value types but not for class types. These assign the arguments to an array and use reflection to call the correct FireN overload. - At patch time we dynamically construct a new lambda that calls into the appropriate `EventDataN_Fire_Override` call. Harmony does not support using generic methods for patches so we use runtime generation instead. Using `LambdaExpression.Compile` does not result in a method with the right signature, so we instead directly use `Reflection.Emit` and `LambdaExpression.CompileToMethod` to create the method we need. We then apply the appropriate patch for every known EventData instantiation in the stock game. If there are important events in mods that use known types then we can add those at a later date.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When looking at KSP within the unity profiler the event handlers are completely invisible. This is a shame, because a lot of frame hangs or stutter happen because an event was fired and event handlers end up running unnecessarily.
This commit introduces a new disabled-by-default patch that automatically adds two sets of profiler spans every time the event is fired:
typename.methodname) of the method being called.That way you can see right away which events and and which callback specifically are slow.
Actually implementing this is a bit messy. EventVoid is easy, it can just use a regular harmony patch, but the remaining EventData types are all generics. Patching generics in general is difficult. You lose class type parameters and you need to separately patch each instantiation for value type parameters.
Making this actually work in practice for stock is somewhat involved
Fire0throughFire4. These are instantiated with the actual type parameters dynamically are basically just a copy-paste of the stock event handling code with the extra spans added.EventDataN_Fire_Overridevariants, these are the calls that actually get patched into the resulting method. Their generic parameters will match for value types but not for class types. These assign the arguments to an array and use reflection to call the correct FireN overload.EventDataN_Fire_Overridecall. Harmony does not support using generic methods for patches so we use runtime generation instead. UsingLambdaExpression.Compiledoes not result in a method with the right signature, so we instead directly useReflection.EmitandLambdaExpression.CompileToMethodto create the method we need.We then apply the appropriate patch for every known EventData instantiation in the stock game. If there are important events in mods that use known types then we can add those at a later date.
Here's an example of what the profiling spans look like with this patch enabled
