-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use generated generic Linker.DefineFunction()
and Function.FromCallback()
overloads for efficiently invoking callbacks
#163
Use generated generic Linker.DefineFunction()
and Function.FromCallback()
overloads for efficiently invoking callbacks
#163
Conversation
…Function() methods that can efficiently call the specified callback without reflection.
It seems we can improve performance even more by using unchecked function variants (bytecodealliance/wasmtime#3350). With commit kpreisser@09560a0 (separate branch When trying the above benchmarks again (under .NET 7.0.0-rc.2, Windows 10 Version 21H2 x64), I get the following results: Benchmark 1 ( Without this PR:
With this PR:
With this PR + commit kpreisser@09560a0:
Benchmark 2 ( Without this PR:
With this PR:
With this PR + commit kpreisser@09560a0:
Benchmark 3 ( Without this PR:
With this PR:
With this PR + commit kpreisser@09560a0:
With the last benchmark, this seems to be roughly a 6x improvement comparing to the current state (without this PR). What do you think? Thanks! |
…equires to use the overload taking a Delegate.
I think the PR is now ready for review. The use of unchecked function variants could be done in a separate follow-up PR. A downside with the overloads is that if you have a
Edit: I implemented a solution by falling back to using reflection (instead of throwing an exception) in such a case. Thank you! |
…tion when the parameter/result type combination cannot be represented with the current generic parameters.
@kpreisser this looks like really excellent work! I'll try to get this reviewed early next week. |
…ker-define-function
…s not necessary to use reflection to find the delegate's Invoke() method.
@kpreisser I apologize for the delay in reviewing this PR. I have time to dive into it fully tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks extremely promising and I really appreciate the templating use here as those overloads were getting unwieldy already.
I just have a few comments to discuss and once we've resolved that, I'll do a final review for approval as I think this is a very valuable thing to have.
…n be included in the .tt files.
…nd invoke it automatically when building the project.
I'll review the recent changes and I'll likely approve, but let's hold off on merging this until CI goes green again with the other PR. |
Linker.DefineFunction()
overloads for efficiently invoking callbacksLinker.DefineFunction()
and Function.FromCallback()
overloads for efficiently invoking callbacks
…ixed in version 2.3.1.
@kpreisser thanks for all the hard work on this! |
This is a follow-up to PR bytecodealliance#163. This also improves the NRT annotations for callback delegates, and fixes an regression that prevented to define callbacks taking or returning an interface.
* Use unchecked functions with raw values for better performance. This is a follow-up to PR #163. This also improves the NRT annotations for callback delegates, and fixes an regression that prevented to define callbacks taking or returning an interface. * Follow-Up: Pass the ValueRaw as reference when boxing a value, for better performance. * Follow-Up: Always set the first 64 bit of the ValueRaw, to match the behavior of the Rust implementation (and this doesn't seem to affect performance). * Only create the Caller instance if it is actually needed. This allows callbacks that don't use a Caller, Function or object parameter to be allocation-free. * Empty commit to retrigger CI. * Refactor to always pass a StoreContext to the IValueRawConverter<T>, so that allocating a Caller is now only necessary when unboxing Functions. * Simplify handling of V128.
Hi! Currently, callbacks defined with
Linker.DefineFunction()
andFunction.FromCallback()
are called using reflection, which has some overhead:wasmtime-dotnet/src/Function.cs
Lines 2421 to 2423 in f3a383a
With this PR, T4 text templates are used to automatically generate generic
Linker.DefineFunction()
andFunction.FromCallback()
overloads for different combinations of parameter count, result count, and the state of having aCaller
argument.Based on the idea from @martindevans in #160 (comment), the template will generate code that uses
ValueBox.Converter<T>
for each generic parameter and result type, and then directly invoke the callback, which avois using reflection and a number of heap allocations (e.g. allocating the arguments array, and boxing the arguments and return values), thereby improving performance and contributing to #113.This way, no dynamic code generation will be needed (see previous PR #160).
Resolving the correct overload by the C# compiler seems to work well:
Currently, overloads will be generated for up to 12 parameters and 4 result values, which will generate 2 * 13 * 5 = 130 overloads. Note that more than 200 overloads will cause the C# compiler to fail resolving them correctly (tested in VS 17.4.0 Preview 2.1).
For delegate types not covered by the overloads (e.g. a
Action
orFunc
with more parameter/result types, or for other delegate types), or if the delegate type isn't known at compile-time, reflection will still be used by providing a overload that takes aDelegate
(that one is still defined inLinker.cs
andFunction.cs
).Edit: To generate the code files (
Linker.DefineFunction.cs
,Function.FromCallback.cs
), we adddotnet-t4
as local tool, which is automatically invoked when building the project. The generated files are still part of the repo, so that e.g. SourceLink will still work correctly for these files.The performance improvements are in the same area of the previous PR (#160) that used
DynamicMethod
to dynamically generate code to invoke the callback:The most performance boost occurs when defining a function with a single parameter. When testing with the following code with .NET 7.0.0-rc.1 on Windows 10 Version 21H2 x64, using an
Action<int>
:Before the change, the times are listed as follows (when compiling for
Release
):After the change:
However, when using more than one arguments, the time with reflection suddenly decreases, and the performance gain is much less. For example, using a
Action<int, float, long>
:Before the change:
After the change:
Testing with a
Func<int, float, long, ValueTuple<int, int, long>>
:Before:
After:
Comparison to other approaches:
Func<...>
/Action<...>
with the maximum number of type parameters; otherwise, reflection will still be used to call it. (Note that thisFunc
/Action
type restriction is already the case today.)What do you think?
Thanks!