Stream: git-wasmtime

Topic: wasmtime / PR #11148 Avoid cloning and dropping `Arc`s in...

Wasmtime GitHub notifications bot (Jun 27 2025 at 17:19):

fitzgen opened PR #11148 from fitzgen:no-clone-drop-arc-in-vmmemory-method to bytecodealliance:main:

Getting the base pointer from the underlying dyn RuntimeLinearMemory involved getting a MemoryBase which is potentially an MmapOffset which itself contains an Arc<Mmap>, and we would then call base.as_non_null() to turn this into a raw pointer, and then we would drop the MemoryBase which ultimately drops the Arc<Mmap>. So that's an Arc clone and drop just to get a VMMemoryDefinition, which is just a pointer and a length, essentially a slice of the linear memory. And, among other places, we call LocalMemory::vmmemory to get the GC heap's memory base and bound every time we access a GC object from Rust (for example, during collections).

Altogether, this removes another 30% of runtime from the testcase in https://github.com/bytecodealliance/wasmtime/issues/11141

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Jun 27 2025 at 17:19):

fitzgen requested dicej for a review on PR #11148.

Wasmtime GitHub notifications bot (Jun 27 2025 at 17:19):

fitzgen requested wasmtime-core-reviewers for a review on PR #11148.

Wasmtime GitHub notifications bot (Jun 27 2025 at 17:27):

fitzgen commented on PR #11148:

All that said, I don't love how this PR introduces a second version of the "same" method. It might be nicer to instead add a vmmemory method to RuntimeLinearMemory and push VMMemoryDefinition construction down to implementations of that trait, rather than tying together the results of different RuntimeLinearMemory methods in LocalMemory.

That would actually let us avoid an indirect call too:

• Now:
  • LocalMemory indirect-calls the byte_size method through a dyn RuntimeLinearMemory
  • LocalMemory indirect-calls the base (now base_non_null in this PR) method through a dyn RuntimeLinearMemory
  • LocalMemory puts them together into a VMMemoryDefinition
• Alternative:
  • LocalMemory indirect-calls the hypothetical vmmemory method through a dyn RuntimeLinearMemory
  • The RuntimeLinearMemory implementation gets its byte size and base, which can both be done directly/inlined/etc because we aren't crossing a dyn boundary anymore
  • The RuntimeLinearMemory implementation puts them together into a VMMemoryDefinition
  • The VMMemoryDefinition is returned to the LocalMemory, which returns it to its caller in turn

Note that the "now" sequence has two indirect calls while the alternative has only one.

I'm going to whip this alternative up real quick and then update this PR.

Wasmtime GitHub notifications bot (Jun 27 2025 at 17:48):

fitzgen updated PR #11148.

Wasmtime GitHub notifications bot (Jun 27 2025 at 17:56):

fitzgen commented on PR #11148:

I'm going to whip this alternative up real quick and then update this PR.

I've pushed a commit that implements this.

Wasmtime GitHub notifications bot (Jun 27 2025 at 18:50):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jun 27 2025 at 19:15):

alexcrichton merged PR #11148.

Last updated: Dec 06 2025 at 06:05 UTC