`wasmparser`: Wasm validation optimization idea

[Robbepop]

Hi, I was thinking about how to further optimize the wasmparser Wasm validation performance because at least in wasmi it takes up quite a fraction of the overall time when compiling Wasm blobs.

The wasmparser crate has lots of runtime checks in place to enable or disable most Wasm proposals. Those checks in summary might be costly. (Needs proof.) The idea is that there usually are common and uncommon profiles of Wasm feature selections. E.g., many users probably still rely on Wasm MVP or simply enable all (stable) Wasm features.

For those common profiles we could specialize the FuncValidator type by making it generic over a trait:

pub trait WasmProfile {
    fn multi_value(&self) -> bool;
    fn reference_types(&self) -> bool;
    fn floats(&self) -> bool;
    // etc...
}

And now we could have a simple implementation for the generic fallback case, based on the WasmFeatures that simply forwards its fields for the trait methods. This should be as fast or as slow as the current implementation.

But if we pre-analyze one of the common Wasm profiles, e.g. where all features are enabled, we could provide the FuncValidator with a WasmProfile type that forwards constants with which the Rust/LLVM can fully optimize away all or most of those checks.

Is that feasible? Am I missing something important? What do you think about this? Unfortunately I have not yet made perf tests for this so my perfs claims might all be wrong because I underestimate the power of the branch predictor.

For example:

impl WasmProfile for DefaultWasmProfile {
    #[inline(always)] fn multi_value(&self) -> bool { true }
    #[inline(always)] fn reference_types(&self) -> bool { true }
    #[inline(always)] fn floats(&self) -> bool { true }
    // etc...
}

#[inline(always)] probably not even needed.

[alexcrichton]

This makes sense to me yeah as possible wins, but I think we'd definitely need data to back it up. This should be relatively easy to test though by basically removing the fields of WasmFeatures and replacing any checks against the fields with true to simulate everything being turned on and inlined.

[Robbepop]

I just implemented and tested this approach but it led to only very minor improvements (2-3%) that are probably within the noise range. So, probably not worth to implement this. We can close it. However, I think I found some instances, were checks were missing. E.g. for tail-calls to guard against operator usage when the feature is disabled. So this might also explain the indifference partially.

[alexcrichton]

Oh that validation is done here, which I suspect was disabled when you removed the fields of WasmFeatures?

[Robbepop]

Oh that validation is done here, which I suspect was disabled when you removed the fields of WasmFeatures?

Dang! No, I probably forgot that since I just used IDE capabilities to replace those fields but the IDE was probably not aware of macro internals. Furthermore I am not even sure if the current benchmark set will yield any differences. This would only affect benchmarks, if benchmarks actually use any of those Wasm proposals that can be disabled, right?

[alexcrichton]

Yeah I'm not sure if the current benchmarks would be affected all that much. I won't claim that we have a stellar suite of benchmarks though

[Robbepop]

Good Wasm benchmarks are kinda hard to come by ... :S We have similar problems for wasmi.