Wasmtime GitHub notifications bot (Apr 21 2025 at 22:24): Wasmtime GitHub notifications bot (Apr 21 2025 at 22:24):frank-emrich requested wasmtime-core-reviewers for a review on PR #10626.
Wasmtime GitHub notifications bot (Apr 21 2025 at 22:24):frank-emrich requested alexcrichton for a review on PR #10626.
Wasmtime GitHub notifications bot (Apr 21 2025 at 22:24):frank-emrich opened PR #10626 from frank-emrich:entrystorecontext to bytecodealliance:main:
Currently, there are two places that perform some updates to the runtime when entering and exiting Wasm: In
runtime::vm::catch_traps(together withCallThreadState'sdrop) and infunc.rs(using theenter_wasm/exit_wasmfunctions there). I believe @alexcrichton mentioned that this split is mostly for legacy reasons due to how to things were separated into different crates until recently.As a result, both of these places need to store different parts of the runtime state, and then restore it at the right moment.
This PR consolidates all of this into one place using a new type,
EntryStoreContext, whoseenter_wasmandexit_wasmfunctions mimic the original functions, but also subsume what previously happened inCallThreadState. The code is just moved around with minimal changes. The name of the type reflects that we are storing a subset of theStoreContextupon entry into Wasm.The motivation for this refactoring is the following (discussed here): For the implementation of the stack switching proposal, we need to save and restore even more context. Using either of the places mentioned above lead to awkward code. Thus, this PR contains the necessary preparation work, without any stack-switching specific code.
Wasmtime GitHub notifications bot (Apr 21 2025 at 22:31):frank-emrich commented on PR #10626:
CC @fitzgen
Wasmtime GitHub notifications bot (Apr 22 2025 at 01:31):alexcrichton submitted PR review:
Nice :+1:
Wasmtime GitHub notifications bot (Apr 22 2025 at 01:31):alexcrichton created PR review comment:
Mind switching this to
pub(crate)to ensure we don't reexport it up to the crate root?
Wasmtime GitHub notifications bot (Apr 22 2025 at 01:31):alexcrichton created PR review comment:
Our current thinking for this is probably "no", unless this performs mutations behind
&EntryStoreContext(which this doesn't)
Wasmtime GitHub notifications bot (Apr 22 2025 at 01:31):alexcrichton created PR review comment:
Mind framing this as
if let Some(...)instead of.inspect(...)?
Wasmtime GitHub notifications bot (Apr 22 2025 at 16:23):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Apr 22 2025 at 16:23):fitzgen created PR review comment:
Yeah, sorry @frank-emrich because I put this idea in your head, but after thinking about it a little more and discussing with Alex, we came to the conclusion that
UnsafeCells are only necessary when violating Rust's mutable xor shared borrowing rules. That is, if the borrow rules are obeyed in JIT code from the perspective of the host (i.e. a JIT function that takes a&Tdoesn't mutate theT) we don't need cells. Similarly, if a JIT function takes an&mut Tit can freely mutate theTwithout the need for cells.
Wasmtime GitHub notifications bot (Apr 22 2025 at 17:01):fitzgen submitted PR review:
Looks great, will merge once our collective nitpicks are addressed. Thanks a ton for this clean up!
Wasmtime GitHub notifications bot (Apr 22 2025 at 17:01):fitzgen created PR review comment:
Pre-existing, but I think we probably want to use checked arithmetic here, because I can't come up with a solid correctness argument for why we can safely avoid it:
let wasm_stack_limit = stack_pointer .checked_sub(store.engine().config().max_wasm_stack) .unwrap();
Wasmtime GitHub notifications bot (Apr 22 2025 at 18:08):fitzgen commented on PR #10626:
Initial comparison to
mainfor sync and no-hook call benchmarks:<details>
$ cargo bench --profile profiling --bench call '\bsync/no-hook' -- --baseline main warning: /home/nick/wasmtime/Cargo.toml: unused manifest key: profile.profiling.debug-info Finished `profiling` profile [optimized] target(s) in 0.29s Running benches/call.rs (target/profiling/deps/call-874724d0870a98be) sync/no-hook/core - host-to-wasm - typed - nop time: [34.061 ns 34.238 ns 34.434 ns] change: [+5.6783% +7.7405% +10.097%] (p = 0.00 < 0.05) Performance has regressed. Found 8 outliers among 100 measurements (8.00%) 4 (4.00%) high mild 4 (4.00%) high severe sync/no-hook/core - host-to-wasm - untyped - nop time: [52.666 ns 52.991 ns 53.372 ns] change: [-3.9315% -2.5302% -1.1114%] (p = 0.00 < 0.05) Performance has improved. Found 8 outliers among 100 measurements (8.00%) 6 (6.00%) high mild 2 (2.00%) high severe sync/no-hook/core - host-to-wasm - unchecked - nop time: [34.399 ns 34.715 ns 35.041 ns] change: [-13.038% -9.5680% -6.0442%] (p = 0.00 < 0.05) Performance has improved. Found 14 outliers among 100 measurements (14.00%) 8 (8.00%) high mild 6 (6.00%) high severe sync/no-hook/core - host-to-wasm - typed - nop-params-and-results time: [37.347 ns 37.566 ns 37.799 ns] change: [-4.1421% -2.5482% -0.9583%] (p = 0.00 < 0.05) Change within noise threshold. Found 10 outliers among 100 measurements (10.00%) 7 (7.00%) high mild 3 (3.00%) high severe sync/no-hook/core - host-to-wasm - untyped - nop-params-and-results time: [104.24 ns 105.00 ns 105.80 ns] change: [-16.960% -14.185% -11.711%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 3 (3.00%) high mild 4 (4.00%) high severe sync/no-hook/core - host-to-wasm - unchecked - nop-params-and-results time: [63.288 ns 63.575 ns 63.879 ns] change: [-8.3622% -5.7145% -2.8859%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 2 (2.00%) high mild 5 (5.00%) high severe sync/no-hook/core - wasm-to-host - typed - nop time: [12.435 ns 12.486 ns 12.542 ns] change: [-14.441% -13.384% -12.279%] (p = 0.00 < 0.05) Performance has improved. Found 5 outliers among 100 measurements (5.00%) 3 (3.00%) high mild 2 (2.00%) high severe sync/no-hook/core - wasm-to-host - typed - nop-params-and-results time: [15.106 ns 15.179 ns 15.262 ns] change: [-13.430% -12.666% -11.742%] (p = 0.00 < 0.05) Performance has improved. Found 8 outliers among 100 measurements (8.00%) 5 (5.00%) high mild 3 (3.00%) high severe sync/no-hook/core - wasm-to-host - untyped - nop time: [22.624 ns 22.745 ns 22.893 ns] change: [-13.227% -11.994% -10.726%] (p = 0.00 < 0.05) Performance has improved. Found 4 outliers among 100 measurements (4.00%) 4 (4.00%) high severe sync/no-hook/core - wasm-to-host - untyped - nop-params-and-results time: [77.490 ns 77.954 ns 78.502 ns] change: [-15.116% -12.772% -10.263%] (p = 0.00 < 0.05) Performance has improved. Found 9 outliers among 100 measurements (9.00%) 2 (2.00%) high mild 7 (7.00%) high severe sync/no-hook/core - wasm-to-host - unchecked - nop time: [13.614 ns 13.661 ns 13.714 ns] change: [-12.737% -11.927% -11.093%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 4 (4.00%) high mild 3 (3.00%) high severe sync/no-hook/core - wasm-to-host - unchecked - nop-params-and-results time: [34.837 ns 35.124 ns 35.467 ns] change: [+3.0411% +4.5871% +6.0450%] (p = 0.00 < 0.05) Performance has regressed. Found 3 outliers among 100 measurements (3.00%) 3 (3.00%) high mild sync/no-hook/component - host-to-wasm - typed - nop time: [59.561 ns 59.850 ns 60.174 ns] change: [-13.754% -12.381% -11.006%] (p = 0.00 < 0.05) Performance has improved. Found 11 outliers among 100 measurements (11.00%) 6 (6.00%) high mild 5 (5.00%) high severe sync/no-hook/component - host-to-wasm - untyped - nop time: [102.94 ns 103.52 ns 104.12 ns] change: [-14.655% -12.756% -10.634%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 2 (2.00%) high mild 5 (5.00%) high severe sync/no-hook/component - host-to-wasm - typed - nop-params-and-results time: [64.160 ns 64.597 ns 65.076 ns] change: [-12.227% -10.714% -9.2881%] (p = 0.00 < 0.05) Performance has improved. Found 5 outliers among 100 measurements (5.00%) 4 (4.00%) high mild 1 (1.00%) high severe sync/no-hook/component - host-to-wasm - untyped - nop-params-and-results time: [229.44 ns 230.55 ns 231.75 ns] change: [-19.925% -18.499% -17.061%] (p = 0.00 < 0.05) Performance has improved. Found 5 outliers among 100 measurements (5.00%) 2 (2.00%) high mild 3 (3.00%) high severe sync/no-hook/component - wasm-to-host - typed - nop time: [39.102 ns 39.327 ns 39.587 ns] change: [-14.689% -13.266% -11.653%] (p = 0.00 < 0.05) Performance has improved. Found 10 outliers among 100 measurements (10.00%) 3 (3.00%) high mild 7 (7.00%) high severe sync/no-hook/component - wasm-to-host - typed - nop-params-and-results time: [47.793 ns 48.160 ns 48.536 ns] change: [-14.798% -13.406% -12.107%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 5 (5.00%) high mild 2 (2.00%) high severe sync/no-hook/component - wasm-to-host - untyped - nop time: [52.711 ns 52.988 ns 53.326 ns] change: [-11.892% -9.4876% -6.6402%] (p = 0.00 < 0.05) Performance has improved. Found 13 outliers among 100 measurements (13.00%) 5 (5.00%) high mild 8 (8.00%) high severe sync/no-hook/component - wasm-to-host - untyped - nop-params-and-results time: [239.26 ns 240.36 ns 241.52 ns] change: [-30.599% -29.181% -27.589%] (p = 0.00 < 0.05) Performance has improved. Found 6 outliers among 100 measurements (6.00%) 1 (1.00%) low mild 3 (3.00%) high mild 2 (2.00%) high severe</details>
Looks like an improvement for most things! Going to look into the regressions a little bit.
Wasmtime GitHub notifications bot (Apr 22 2025 at 19:45):frank-emrich updated PR #10626.
Wasmtime GitHub notifications bot (Apr 22 2025 at 19:47):fitzgen commented on PR #10626:
Okay I've got a few
#[inline]s that improve all the call benchmarks I've measured compared tomain. Will land them after this PR!<details>
$ cargo bench --profile profiling --bench call '\bsync/no-hook' -- --baseline main Finished `profiling` profile [optimized + debuginfo] target(s) in 0.28s Running benches/call.rs (target/profiling/deps/call-b0a2bedd3336ad76) sync/no-hook/core - host-to-wasm - typed - nop time: [27.334 ns 27.499 ns 27.668 ns] change: [-16.388% -14.870% -13.479%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 5 (5.00%) high mild 2 (2.00%) high severe sync/no-hook/core - host-to-wasm - untyped - nop time: [44.141 ns 44.429 ns 44.757 ns] change: [-18.380% -17.041% -15.670%] (p = 0.00 < 0.05) Performance has improved. Found 3 outliers among 100 measurements (3.00%) 1 (1.00%) high mild 2 (2.00%) high severe sync/no-hook/core - host-to-wasm - unchecked - nop time: [29.731 ns 29.983 ns 30.262 ns] change: [-25.104% -22.176% -19.159%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 5 (5.00%) high mild 2 (2.00%) high severe sync/no-hook/core - host-to-wasm - typed - nop-params-and-results time: [28.990 ns 29.143 ns 29.303 ns] change: [-25.804% -24.562% -23.372%] (p = 0.00 < 0.05) Performance has improved. Found 6 outliers among 100 measurements (6.00%) 3 (3.00%) high mild 3 (3.00%) high severe sync/no-hook/core - host-to-wasm - untyped - nop-params-and-results time: [110.00 ns 110.65 ns 111.46 ns] change: [-11.967% -9.0070% -6.1347%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 2 (2.00%) high mild 5 (5.00%) high severe sync/no-hook/core - host-to-wasm - unchecked - nop-params-and-results time: [58.828 ns 59.089 ns 59.418 ns] change: [-15.596% -13.573% -11.484%] (p = 0.00 < 0.05) Performance has improved. Found 3 outliers among 100 measurements (3.00%) 3 (3.00%) high severe sync/no-hook/core - wasm-to-host - typed - nop time: [6.6209 ns 6.6615 ns 6.7077 ns] change: [-53.555% -52.878% -52.116%] (p = 0.00 < 0.05) Performance has improved. Found 6 outliers among 100 measurements (6.00%) 5 (5.00%) high mild 1 (1.00%) high severe sync/no-hook/core - wasm-to-host - typed - nop-params-and-results time: [7.9783 ns 8.0173 ns 8.0611 ns] change: [-54.341% -53.947% -53.505%] (p = 0.00 < 0.05) Performance has improved. Found 3 outliers among 100 measurements (3.00%) 3 (3.00%) high severe sync/no-hook/core - wasm-to-host - untyped - nop time: [18.306 ns 18.393 ns 18.491 ns] change: [-29.104% -28.127% -27.171%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 3 (3.00%) high mild 4 (4.00%) high severe sync/no-hook/core - wasm-to-host - untyped - nop-params-and-results time: [67.741 ns 68.120 ns 68.601 ns] change: [-26.453% -25.061% -23.663%] (p = 0.00 < 0.05) Performance has improved. Found 12 outliers among 100 measurements (12.00%) 6 (6.00%) high mild 6 (6.00%) high severe sync/no-hook/core - wasm-to-host - unchecked - nop time: [6.8379 ns 6.8915 ns 6.9566 ns] change: [-55.623% -55.062% -54.481%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 5 (5.00%) high mild 2 (2.00%) high severe sync/no-hook/core - wasm-to-host - unchecked - nop-params-and-results time: [27.856 ns 28.024 ns 28.214 ns] change: [-17.320% -16.103% -15.038%] (p = 0.00 < 0.05) Performance has improved. Found 10 outliers among 100 measurements (10.00%) 6 (6.00%) high mild 4 (4.00%) high severe sync/no-hook/component - host-to-wasm - typed - nop time: [55.126 ns 55.506 ns 55.932 ns] change: [-19.458% -18.098% -16.736%] (p = 0.00 < 0.05) Performance has improved. Found 8 outliers among 100 measurements (8.00%) 2 (2.00%) high mild 6 (6.00%) high severe sync/no-hook/component - host-to-wasm - untyped - nop time: [101.42 ns 102.06 ns 102.82 ns] change: [-15.679% -14.108% -12.523%] (p = 0.00 < 0.05) Performance has improved. Found 9 outliers among 100 measurements (9.00%) 7 (7.00%) high mild 2 (2.00%) high severe sync/no-hook/component - host-to-wasm - typed - nop-params-and-results time: [61.482 ns 62.017 ns 62.591 ns] change: [-16.576% -15.100% -13.595%] (p = 0.00 < 0.05) Performance has improved. Found 10 outliers among 100 measurements (10.00%) 9 (9.00%) high mild 1 (1.00%) high severe sync/no-hook/component - host-to-wasm - untyped - nop-params-and-results time: [223.50 ns 224.72 ns 226.05 ns] change: [-21.732% -20.178% -18.679%] (p = 0.00 < 0.05) Performance has improved. Found 4 outliers among 100 measurements (4.00%) 1 (1.00%) high mild 3 (3.00%) high severe sync/no-hook/component - wasm-to-host - typed - nop time: [39.115 ns 39.295 ns 39.500 ns] change: [-15.139% -13.886% -12.721%] (p = 0.00 < 0.05) Performance has improved. Found 8 outliers among 100 measurements (8.00%) 1 (1.00%) low mild 2 (2.00%) high mild 5 (5.00%) high severe sync/no-hook/component - wasm-to-host - typed - nop-params-and-results time: [47.234 ns 47.458 ns 47.745 ns] change: [-13.833% -11.951% -9.8784%] (p = 0.00 < 0.05) Performance has improved. Found 9 outliers among 100 measurements (9.00%) 3 (3.00%) high mild 6 (6.00%) high severe sync/no-hook/component - wasm-to-host - untyped - nop time: [52.311 ns 52.556 ns 52.817 ns] change: [-12.736% -11.712% -10.693%] (p = 0.00 < 0.05) Performance has improved. Found 9 outliers among 100 measurements (9.00%) 4 (4.00%) high mild 5 (5.00%) high severe sync/no-hook/component - wasm-to-host - untyped - nop-params-and-results time: [239.71 ns 241.59 ns 244.11 ns] change: [-29.804% -28.173% -26.415%] (p = 0.00 < 0.05) Performance has improved. Found 9 outliers among 100 measurements (9.00%) 4 (4.00%) high mild 5 (5.00%) high severe</details>
Wasmtime GitHub notifications bot (Apr 22 2025 at 19:48):frank-emrich submitted PR review.
Wasmtime GitHub notifications bot (Apr 22 2025 at 19:48):frank-emrich created PR review comment:
Following the reasoning about
UnsafeCellelsewhere, I guess the answer is "no" here then as well
Wasmtime GitHub notifications bot (Apr 22 2025 at 19:54):fitzgen commented on PR #10626:
Okay I've got a few
#[inline]s that improve all the call benchmarks I've measured compared tomain. Will land them after this PR!
Wasmtime GitHub notifications bot (Apr 22 2025 at 19:57):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Apr 22 2025 at 19:57):fitzgen created PR review comment:
Correct, because nothing is mutating this pointer in such a way that violates Rust's regular borrowing discipline.
Wasmtime GitHub notifications bot (Apr 22 2025 at 19:58):frank-emrich submitted PR review.
Wasmtime GitHub notifications bot (Apr 22 2025 at 19:58):frank-emrich created PR review comment:
Actually, I remember now why I put this comment here: The previous
old_*fields wereCells. Maybe that was just some leftover.
Wasmtime GitHub notifications bot (Apr 22 2025 at 19:59):frank-emrich updated PR #10626.
Wasmtime GitHub notifications bot (Apr 22 2025 at 19:59):frank-emrich updated PR #10626.
Wasmtime GitHub notifications bot (Apr 22 2025 at 21:15):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Apr 22 2025 at 21:15):fitzgen created PR review comment:
Yeah, should be fine to remove.
Wasmtime GitHub notifications bot (Apr 22 2025 at 21:17):fitzgen submitted PR review:
Thanks!
Wasmtime GitHub notifications bot (Apr 22 2025 at 21:39):fitzgen merged PR #10626.
Last updated: Dec 06 2025 at 07:03 UTC