adambratschikaye opened PR #4924 from memoize-can_optimize_var_lookup
to main
:
This fixes issue https://github.com/bytecodealliance/wasmtime/issues/4923.
can_optimize_var_lookup
can have quadratic behavior if there is a chain of blocks each containing a local.get instruction because each run can walk up the entire chain. This change memoizes the results ofcan_optimize_var_lookup
so that we can stop following the chain of predecessors when we hit a block that has previously been handled (making the operation linear again).For the example wasm in issue https://github.com/bytecodealliance/wasmtime/issues/4923 this reduces the compile time from 2.4 seconds to 80 milliseconds on my machine.
I haven't added any test cases because this is an optimization, but happy to add some if needed.
Running this on the
bz2
,spidermonkey
, andpulldown-cmark
benchmarks insightglass
showed a slight slowdown onspidermonkey
and no change on the others. But after I switched thevisited
field from aHashSet
toSecondaryMap
there's no regression overall:Running `target/debug/sightglass-cli benchmark --stop-after compilation --engine /tmp/wasmtime_main.so --engine /tmp/wasmtime_all_secondarymap.so -- benchmarks/bz2/benchmark.wasm benchmarks/pulldown-cmark/benchmark.wasm benchmarks/spidermonkey/benchmark.wasm` compilation :: cycles :: benchmarks/spidermonkey/benchmark.wasm Δ = 760212555.60 ± 294917212.14 (confidence = 99%) all_secondarymap.so is 1.01x to 1.03x faster than main.so! [28814332410 35120475949.50 36287395200] all_secondarymap.so [34284188130 35880688505.10 36647154330] main.so compilation :: cycles :: benchmarks/bz2/benchmark.wasm No difference in performance. [505446060 603389897.40 1110277230] all_secondarymap.so [476744010 579360685.20 853570110] main.so compilation :: cycles :: benchmarks/pulldown-cmark/benchmark.wasm No difference in performance. [1189612080 1520043626.70 2771829660] all_secondarymap.so [1347713850 1514585763.90 1904475450] main.so
abrown requested fitzgen for a review on PR #4924.
fitzgen submitted PR review.
abrown merged PR #4924.
Last updated: Dec 23 2024 at 12:05 UTC