paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.8k stars 652 forks source link

Substrate loading runtime wasm blob in Native execution mode. #5669

Closed nickvikeras closed 2 weeks ago

nickvikeras commented 2 weeks ago

Is there an existing issue?

Experiencing problems? Have you tried our Stack Exchange first?

Description of bug

Hello,

I recently noticed a bottleneck in my project's unit tests where substrate was loading the wasm blob (takes ~1s) when my test was configured to run in NativeWhenPossible mode. This is happening because here substrate is delegating to the wasm blob to load the runtime version. It looks like we can get the runtime version from the NativeVersion struct field of the NativeElseWasmExecutor struct instead of the wasm blob.

With this small change, the wasm load is avoided and my test (which spins up many nodes as a test network) runs 15s faster:

    diff --git a/client/executor/src/executor.rs b/client/executor/src/executor.rs
    index a3717f4d2..90d05b41c 100644
    --- a/client/executor/src/executor.rs
    +++ b/client/executor/src/executor.rs
    @@ -603,7 +603,7 @@ impl<D: NativeExecutionDispatch> RuntimeVersionOf for NativeElseWasmExecutor<D>
            ext: &mut dyn Externalities,
            runtime_code: &RuntimeCode,
        ) -> Result<RuntimeVersion> {
    -       self.wasm.runtime_version(ext, runtime_code)
    +       Ok(self.native_version.runtime_version.clone())
        }
     }

We are on an older fork of substrate, but it looks like this is still an issue in the new repo as well. https://github.com/paritytech/polkadot-sdk/blob/master/substrate/client/executor/src/executor.rs#L650

Does substrate need to go to the wasm blob to cross-check its version with the native version, or is this just an oversight in the implementation? Would a new "AlwaysNative" ExecutionStrategy be a better solution?

Let me know and I can send a PR.

nickvikeras commented 2 weeks ago

I heard that Native execution mode is actually on the deprecation path, and hence it probably doesn't make sense to introduce such an execution mode. Also, I figured out that enabling compiler optimizations for tests for some crates largely solves the problem anyway.