Caller.TryGetMemorySpan

[martindevans]

This is one possible way to get access to Memory from a caller without any allocations. I think this is the simplest option (it's just one method) but the downside it doesn't give access to all of the convenience methods that are on Memory.

The other alternative I have come up with is to define a whole new struct Memory which has some/all of the Memory methods on it and the class Memory would just become a wrapper around that. That's a much bigger change though!

Thoughts?

[martindevans]

I presented this initially as one possible option, but for now I'd prefer to merge this if there are no objections? I might do the other alternative in the future, but it's a huge amount of work which I don't want to commit to at the moment.

[peterhuene]

I'll get this reviewed shortly, sorry for the wait.

[peterhuene]

Looks good 👍. Thanks for adding this.

One thing we can fix up on a follow-up PR is to expand the stackalloc conversion of the name string to the GetMemory and GetFunction methods as well.

We may also want to investigate changing Memory (and other store-related types) to value types too.

[martindevans]

We may also want to investigate changing Memory (and other store-related types) to value types too.

That's essentially the other option that I was suggesting (that would be a lot more work than this). My thought was we'd add a ref struct RefMemory which you could get from the Caller. Internally all of the class Memory functions would construct a RefMemory, use it and immediately dispose it - so all of the actual business logic would live in the struct except for lifetime management which would probably still need to live in a class (and also so we don't break the API).

[peterhuene]

I'm not too concerned with breaking the API when it makes sense to do so (we're rev'ing major versions to match Wasmtime's release numbers anyway); I think we should take the approach that makes the most sense for the API first and foremost; put another way, have good reason to break the API, but not be beholden to compatibility.

If we don't want to have to introduce an additional type for accessing a memory, given that the store related types currently implement the IExternal interface, it would probably be easier to make them struct rather than ref struct; they should also be "copyable" and escape the local frame without issue, as there's nothing being managed by their lifetimes (the store manages it).

Definitely worth looking into, at least. Happy to take either approach.

[martindevans]

Ah I was thinking Memory : IDisposable. Since that's not the case making this change might actually be easier than I had initially thought. I'll have a quick go at prototyping it later to see how it looks.

[kpreisser]

It seems you reverted back to the previous approach of using GetMaxByteCount when doing the force-push, was this deliberate? (Sorry for not catching this earlier).
Thanks!

[martindevans]

No it wasn't! I'm not sure quite how I managed that, I'll put in a PR to fix it now. Thanks for catching it 👍