Stream: git-wasmtime

Topic: wasmtime / PR #9682 Put MemoryPool and MmapMemory mmaps b...

Wasmtime GitHub notifications bot (Nov 26 2024 at 08:04):

sunshowers requested pchickey for a review on PR #9682.

Wasmtime GitHub notifications bot (Nov 26 2024 at 08:04):

sunshowers opened PR #9682 from sunshowers:move-arc to bytecodealliance:main:

In upcoming work we're going to centralize memory management inside Mmap. In order to do that, we need memory that logically borrows from the Mmap to have access to it. That turns out to be possible in some situations but very difficult in others (see #9681 for an attempt that was a real hassle to get working).

To prepare for this work, start moving these uses to be behind Arc. These aren't cloned at the moment, but they will be in the future.

Wasmtime GitHub notifications bot (Nov 26 2024 at 08:04):

sunshowers requested wasmtime-core-reviewers for a review on PR #9682.

Wasmtime GitHub notifications bot (Nov 26 2024 at 08:06):

sunshowers submitted PR review.

Wasmtime GitHub notifications bot (Nov 26 2024 at 08:06):

sunshowers created PR review comment:

This was somewhat annoying but I think this approach makes sense -- basically ensure that any &mut references are very short-lived as required by Rust's aliasing rules.

Wasmtime GitHub notifications bot (Nov 26 2024 at 08:08):

sunshowers edited PR #9682:

In upcoming work we're going to centralize memory management inside Mmap. In order to do that, we need memory that logically borrows from the Mmap to have access to it. That turns out to be possible to do by passing in &Mmap in some situations but very difficult in others (see #9681 for an attempt that was a real hassle to get working and still had a bunch of incomplete questions within it).

To prepare for this work, start moving these uses to be behind Arc. These aren't cloned at the moment, but they will be in the future.

Wasmtime GitHub notifications bot (Nov 26 2024 at 15:50):

alexcrichton commented on PR #9682:

I think this can make sense but I'm having a hard time understanding the end goal without the second piece here of using the Arc itself. Would you be ok delaying merging this until the second half is in place? It'd be easier for me to review once that's there (I'm ok reviewing the two together as two separate commits myself)

Wasmtime GitHub notifications bot (Dec 03 2024 at 15:47):

alexcrichton submitted PR review:

Ok thanks for #9687! That looks reasonable to me so let's go ahead and land this :+1:

Wasmtime GitHub notifications bot (Dec 03 2024 at 16:07):

alexcrichton merged PR #9682.

Last updated: Dec 23 2024 at 12:05 UTC