ValueBox

[martindevans]

See proposal #113.

To reduce allocations for boxing on every call this PR introduces a new ValueBox struct which wraps up arguments without requiring any heap allocations. Currently this is only used for function arguments, but in principle it could be used anywhere else plain old object references are passed around (e.g. function returns values).

Drawbacks

1. ValueBox is mostly invisible to the end user since things convert to it with an implict conversion. However, C# does not support user defined conversions from object so that has to be done with an explicit conversion method - i.e. passing my_ref_object becomes ValueBox.AsBox(my_ref_object). I've added an implict conversion for strings, since that's likely to be a fairly common case. Unfortunately I don't see a better way to handle this situation for any other references types.

2. I had to remove the old params object[] method, since it confliced with the new params ValueBox[] method, which is a breaking API change. If you'd rather not much such a big change to the external API I can instead remove params ValueBox[] and require that users call it explicitly with a ReadOnlySpan<ValueBox> instead.

3. params ValueBox[] still incurs a single allocation, which can be worked around by explicitly passing in a ReadOnlySpan<ArgBox>, but that's not very ergonomic.

4. I haven't done anything with return types at all, they still get boxed. It's not obvious how to fix them, since the caller does not supply the return type.

Potential Improvements

We could potentially workaround a lot of the drawbacks above by introducing a new set of generic GetFunction overloads which return Func<> or Action<> objects. This would have quite a few benefits:

• Compatibility of types could be checked when GetFunction is called, instead of when Invoke is called. Potentially reducing per-call overhead.
• Fixed number of arguments, so ergonomics issues with params array/span disappear.
• Conversion to ValueBox can be done inside the Func/Action which is returned, making it invisible to the end user (ValueBox.AsBox doesn't need to be called externally)
• Return type unboxing can be handled easily in the same way.
• Fits better into a normal C# workflow - wasm functions can be used just like any other Func/Action object.

If you're interested I can put together another PR which does this (don't merge this PR if so).

[martindevans]

I think CI was already failing before my PR, so these failures aren't something I need to fix?

[peterhuene]

Hi @martindevans. Thanks again for putting this up!

I'll try to get this reviewed shortly. The CI failure is indeed an existing problem in main due to upstream changes around backtraces in traps. There's nothing you need to do as I'll get CI fixed as well.

[peterhuene]

@martindevans if you rebase your PR on main, that should resolve the test failures.

[peterhuene]

These changes look good to me, just a few comments to address.

Regarding this approach and its drawbacks, I think it's beneficial enough to justify any breaking changes around passing arguments existing users might encounter.

Obviously some sort of function wrapping where we can return a callable object where the argument type checks are elided for the actual call would be most ideal. If you want to take a stab at that approach, I'd be happy to review it in lieu of or in addition to merging this.

[peterhuene]

👍 looks good to merge.

Would you like this to go in now or would you like to instead wait for taking the alternative approach of returning Func-like wrappers that elide the parameter / result conversions and type checking?

[martindevans]

We may as well leave this PR for a couple of days, just in case I need to make any major changes to it to support the Func-wrapper approach.

[martindevans]

So far I didn't have to make any changes to this for my #116 draft PR, so I think it's probably safe to merge it as is. I don't mind if you'd rather wait and do it all in one go though :)

[martindevans]

I've swapped out the representation of v128 from a byte[] (requires an allocation, size must be runtime checked) to Vector128<byte> (no allocations, size is part of type) as proposed in #120. This change is required for #116 to move forward. (See below)

[martindevans]

Instead of using Vector128<byte> which is quite specifically tied to the hardware intrinsics API I've created a custom V128 type. I think this creates a cleaner API.

[peterhuene]

I agree, that is much nicer!

Sorry, I've been out sick this week, so I haven't been responding to GitHub notifications.

Please let me know if you want me to review this again and merge it if it's needed for the other PR.

[martindevans]

That's no problem, I hope you're feeling better now :)

Please go ahead and review this and merge it if you're happy with it. The other PR builds on top of both changes this introduces.

[peterhuene]

Just a comment or two and a few nits. Looking really great!

[martindevans]

I've addressed all of that feedback

[peterhuene]

Excellent, thanks again for this work!