Opt-out from fast instance reuse and foundation for other refactorings #8394
Conversation
Seed it with the existing contents of the `util` module.
pepyakin
added
A0-please_review
| @@ -0,0 +1,109 @@ | |||
| // This file is part of Substrate. | |||
|
|
|||
| // Copyright (C) 2020-2021 Parity Technologies (UK) Ltd. | |||
There was a problem hiding this comment.
That file is technically moved, not created, so I left this.
| /// The runtime is instantiated using the given runtime blob. | ||
| Verbatim { | ||
| // Rationale to take the `RuntimeBlob` here is so that the client will be able to reuse | ||
| // the blob e.g. if they did a prevalidation. If they didn't they can `RuntimeBlob` |
There was a problem hiding this comment.
If they didn't they can
RuntimeBlob
Needs to be fixed.
In general I'm a little bit confused by this whole docs in here.
There was a problem hiding this comment.
What is confusing? Do you have a suggestions how to improve that?
So what I am trying to say there, is that there are two use cases how sc-executor-wasmtime is going to be invoked: the traditional substrate path that uses this fast instance reuse thing and PVF execution. Both of those require to do some instrumentation under the hood, meaning that RuntimeBlob will be required.
However, I cannot say that this will be the case forever, at some point we may not need to do instrumentation nor prevalidation in which case creation of the runtime blob will be superfluous. In that case I suggest in the comment we can introduce a Cow like thing, which will create RuntimeBlob only if it's not provided and if it is required.
| /// of limit: some may count the number of calls or values, others would rely on the host machine | ||
| /// stack and trap on reaching a guard page. | ||
| /// | ||
| /// This obviously is a source of non-determinism during execution. This feature can be used |
There was a problem hiding this comment.
First you say it is non deterministic and than you speak about it being able to be setup in a deterministic way?
There was a problem hiding this comment.
Yeah, that's right. The stack depth is not deterministic, but if you enable that feature (i.e. set stack_depth_metering) it will become deterministic.
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
paritytech#8394) * Establish the runtime_blob module Seed it with the existing contents of the `util` module. * Port wasmtime mutable globals instrumentation into runtime blob APIs * Opt-out from fast instance reuse * Minor clean up * Spaces * Docs clean up * Apply suggestions from code review Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Factor out the expects * Fix the suggestion Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
paritytech#8394) * Establish the runtime_blob module Seed it with the existing contents of the `util` module. * Port wasmtime mutable globals instrumentation into runtime blob APIs * Opt-out from fast instance reuse * Minor clean up * Spaces * Docs clean up * Apply suggestions from code review Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Factor out the expects * Fix the suggestion Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
paritytech#8394) * Establish the runtime_blob module Seed it with the existing contents of the `util` module. * Port wasmtime mutable globals instrumentation into runtime blob APIs * Opt-out from fast instance reuse * Minor clean up * Spaces * Docs clean up * Apply suggestions from code review Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Factor out the expects * Fix the suggestion Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
This PR proposes several changes to the executor. Mostly it is motivated by #8153.
First change is establishing a single place where inspections and instrumentation would live:
runtime_blobmodule. Basically, it is a wrapper around a parity-wasm'sModulewith instrumentation and inspection related functionality. As a nice side-effect, a place where we deserialized into a pwasm module, then serialized and deserialized in the wasmtime backend, was fixed and now we deserialize and serialize a module only once.Then, we move a custom instrumentation that is needed for mutable globals (introduced by #6759) into the runtime_blob module.
And finally, two most important changes in the wasmtime backend: