wasmparser: Optimize Locals::get type query
#2197
Conversation
We rename because this field will no longer store _all_ locals, but only the ones that are not also stored in the `first` field.
|
CI tests fail because this PR uses https://github.com/bytecodealliance/wasm-tools/blob/main/Cargo.toml#L89-L95 edit: For now I went with the |
There was a problem hiding this comment.
Thanks for this! I'll go ahead and flag this for merge.
As for MSRV, I'm not 100% sure how best to handle this. I feel that Wasmtime's 2-latest-rustc-version is a bit too aggressive for this repository given the broader use that's intended here. Freezing to just one version though is also something I don't think is tenable. I've roughly been thinking that "current - 10" is a reasonable enough window for this workspace, but I haven't set up automation or anything like that to such effect.
|
@alexcrichton thanks!
|
When looking through the code I saw that
Locals::allfield unnecessarily pushes all locals, even the ones that have already been covered in thefirstvector. This further slows down thebinary_searchcall due to having to search through all the unnecessary indices.This led to a modest speed-up of ~1-2% though I doubt that the current set of benchmarks really cover this scenario and therefore the gains might be higher for some real world Wasm files with many large functions.
I changed the types of
MAX_WASM_FUNCTION_LOCALSandMAX_LOCALS_TO_TRACKbecause they are only used at those locations and the new typing no longer necessitates casts.