Stream: git-wasmtime

Topic: wasmtime / PR #9652 move most of runtime/vm/cow.rs over t...

Wasmtime GitHub notifications bot (Nov 22 2024 at 06:09):

sunshowers opened PR #9652 from sunshowers:cow-aligned to bytecodealliance:main:

As part of attempting to move some of these operations over to Mmap instances, it is nice to have type-level checking for aligned sizes. In upcoming PRs, APIs like map_at will be switched to using Mmap instances with aligned counts.

There are a couple of spots where I have questions -- will flag them in review comments.

Wasmtime GitHub notifications bot (Nov 22 2024 at 06:09):

sunshowers requested alexcrichton for a review on PR #9652.

Wasmtime GitHub notifications bot (Nov 22 2024 at 06:09):

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

Wasmtime GitHub notifications bot (Nov 22 2024 at 06:10):

sunshowers submitted PR review.

Wasmtime GitHub notifications bot (Nov 22 2024 at 06:10):

sunshowers created PR review comment:

Thoughts about this? I tried to be super cautious about signs this time.

Wasmtime GitHub notifications bot (Nov 22 2024 at 06:10):

sunshowers created PR review comment:

Wasn't clear to me whether this was possible -- there are several offsets here that all interact with each other.

Wasmtime GitHub notifications bot (Nov 22 2024 at 06:12):

sunshowers updated PR #9652.

Wasmtime GitHub notifications bot (Nov 22 2024 at 06:18):

sunshowers edited PR review comment.

Wasmtime GitHub notifications bot (Nov 22 2024 at 18:04):

alexcrichton submitted PR review:

Thanks for this!

Wasmtime GitHub notifications bot (Nov 22 2024 at 18:04):

alexcrichton created PR review comment:

I think this is safe to be .unwrap() since the bounds checks above I think rule out the underflow in this case

Wasmtime GitHub notifications bot (Nov 22 2024 at 18:04):

alexcrichton created PR review comment:

Would it be possible to keep the same < condition as before? I find that one a bit easier to understand in terms of the diagrams below personally.

Wasmtime GitHub notifications bot (Nov 22 2024 at 18:04):

alexcrichton created PR review comment:

Could these stay defined where they were below to keep their definition close to the diagram explaining what they are?

Wasmtime GitHub notifications bot (Nov 22 2024 at 23:06):

sunshowers updated PR #9652.

Wasmtime GitHub notifications bot (Nov 22 2024 at 23:07):

sunshowers submitted PR review.

Wasmtime GitHub notifications bot (Nov 22 2024 at 23:07):

sunshowers created PR review comment:

Yeah good idea, much more understandable with this and the below change.

Wasmtime GitHub notifications bot (Nov 22 2024 at 23:07):

sunshowers created PR review comment:

Okay -- to be honest I'm not entirely sure that this is the case in reality, it seems like in configuration you could define keep_resident to be more then the byte size. But in any case this should panic (and if there is such a condition it should be handled at a higher level)

Wasmtime GitHub notifications bot (Nov 22 2024 at 23:10):

sunshowers commented on PR #9652:

ugh, attempting to fix the no-std build

Wasmtime GitHub notifications bot (Nov 22 2024 at 23:16):

sunshowers updated PR #9652.

Wasmtime GitHub notifications bot (Nov 22 2024 at 23:20):

sunshowers submitted PR review.

Wasmtime GitHub notifications bot (Nov 22 2024 at 23:20):

sunshowers created PR review comment:

Done.

Wasmtime GitHub notifications bot (Nov 22 2024 at 23:30):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Nov 22 2024 at 23:46):

alexcrichton merged PR #9652.

Last updated: Dec 23 2024 at 12:05 UTC