jameysharp commented on issue #5335:
FWIW, I looked this over and I think it makes sense, but I'd definitely prefer to have somebody else's review on it.
fitzgen commented on issue #5335:
It looks like the prior PR accidentally enabled bounds checks by default in Wasmtime, so I'm sort of surprised we didn't catch that until now. Does sightglass show regressions here or improvements after this commit? If so it might be good to use sightglass on future changes to
heap_{addr,store,load}
to double-check the intended effect.Yeah would be great if we had nightly sightglass benchmarks set up. I set up some smoke tests for
heap_addr
legalization with vs without sufficient guard pages to avoid bounds checks (should tide us over untilheap_addr
is gone).Sightglass results for this PR:
execution :: instructions-retired :: benchmarks/spidermonkey/benchmark.wasm Δ = 3362887399.64 ± 42143.94 (confidence = 99%) with-guard-pages.so is 2.05x to 2.05x faster than main.so! [6566493438 6566656053.70 6567012773] main.so [3203651702 3203768654.06 3204149983] with-guard-pages.so execution :: instructions-retired :: benchmarks/bz2/benchmark.wasm Δ = 253455489.92 ± 1.49 (confidence = 99%) with-guard-pages.so is 1.79x to 1.79x faster than main.so! [573123166 573123173.64 573123183] main.so [319667678 319667683.72 319667699] with-guard-pages.so execution :: instructions-retired :: benchmarks/pulldown-cmark/benchmark.wasm Δ = 16418573.09 ± 99.14 (confidence = 99%) with-guard-pages.so is 1.62x to 1.62x faster than main.so! [42813022 42813207.84 42813926] main.so [26394395 26394634.75 26395352] with-guard-pages.so compilation :: instructions-retired :: benchmarks/bz2/benchmark.wasm Δ = 321257.88 ± 54303.80 (confidence = 99%) with-guard-pages.so is 1.11x to 1.15x faster than main.so! [2784403 2856173.33 3298299] main.so [2460682 2534915.45 3047605] with-guard-pages.so compilation :: instructions-retired :: benchmarks/spidermonkey/benchmark.wasm Δ = 25520928.63 ± 108236.81 (confidence = 99%) with-guard-pages.so is 1.12x to 1.12x faster than main.so! [232258160 232762746.46 233555099] main.so [206468796 207241817.83 207933331] with-guard-pages.so compilation :: instructions-retired :: benchmarks/pulldown-cmark/benchmark.wasm Δ = 676224.25 ± 51124.52 (confidence = 99%) with-guard-pages.so is 1.07x to 1.08x faster than main.so! [9483066 9609230.75 10037142] main.so [8828280 8933006.50 9376518] with-guard-pages.so
alexcrichton commented on issue #5335:
Nah I don't think it's worthwhile to add more than what's already here given that this is all intended to be short-lived anyway. It's good though that sightglass clearly detects this either way!
Last updated: Nov 22 2024 at 16:03 UTC