-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Performance-wise just for your reference, I have a few benchmarks laying around that I've created recently (when investigating the random performance regression on wasmtime bump); here they are:
coremark,wasmi_009_under_wasmtime_040,217
coremark,wasmi_013_under_wasmtime_040,203
coremark,wasmi_016_under_wasmtime_040,180
regexredux,wasmi_009_under_wasmtime_040,2280
regexredux,wasmi_013_under_wasmtime_040,2386
regexredux,wasmi_016_under_wasmtime_040,2645
noop,wasmi_009_under_wasmtime_040,9237
noop,wasmi_013_under_wasmtime_040,9208
noop,wasmi_016_under_wasmtime_040,4352
First column is the name of the benchmark, the second column is the executor, and the third is the score or time. For coremark
higher is better, for regexredux
and noop
lower is better. It seems that (at least in these benchmarks) the higher you bump wasmi the worse the performance gets in CPU-heavy benchmarks (coremark
and regexredux
) while the performance in an instantiation-heavy benchmark noop
gets better on 0.16.
/cmd queue -c bench-bot $ pallet dev pallet_contracts |
@athei https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1818470 was started for your command Comment |
Have you used [profile.release]
lto = "fat"
codegen-units = 1 for |
Yes. All of the WASM kernels were compiled on the following profile:
(AFAIK |
@koute Is there a way to check this? I often had problem properly setting up the profile for dependencies (in workspaces). The thing is that [profile.lto] Maybe forgot to set the
Yes, although less readable, so I'd propose changing all occurences to |
@Robbepop you conducted your benchmarks only with wasmi compiled to native, right? These benchmarks run it under wasmtime which could completely change the game. I am currently running a burn-in (syncing kusama) with this branch. When nothing comes up I will merge it. |
I am aware that Wasm compilation might change things but I just doubt that the difference between Wasm and native compilation run on a JIT compiler is as big as claimed in the benchmarks above. That's it. I'd like to have sources to those benchmarks to convince myself. Basically what @koute measured looks suspiciously similar to the numbers I am seeing when I chose the wrong profile (e.g. default |
@athei Command |
/cmd queue -c bench-bot $ pallet dev pallet_contracts |
@athei https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1820800 was started for your command Comment |
Not that I know of, although you could probably do something like this in
I might have screwed up something else, but this I shouldn't have. (: Here are the results for the standard
|
@Robbepop Okay, I've cleaned up my benchmarks just a little bit and pushed them into a repo; you can check them out here if you want: https://github.com/koute/wasm-bench |
@athei Command |
@koute Thanks a lot for making the benchmarks available. They provided me with some valuable insights. I was able to reproduce your results on my own machine where newer These are the results I got afterwards:
In conclusion, after My recommendation still is to use |
We don't. I have a branch where I tested this. However it is a bit experimental because |
bot merge |
* Upgrade wasm crate dependencies * New wasmi version changed error output a bit * ".git/.scripts/bench-bot.sh" pallet dev pallet_contracts * ".git/.scripts/bench-bot.sh" pallet dev pallet_contracts Co-authored-by: command-bot <>
This pull request has been mentioned on Polkadot Forum. There might be relevant details there: https://forum.polkadot.network/t/exploring-alternatives-to-wasm-for-smart-contracts/2434/20 |
This upgrades:
These do not contain big changes but merely big fixes. wasmi is upgraded to the last version before bigger changes were made. We will upgrade only until we did more testing: paritytech/roadmap#9
We should also do a burn in with wasmi as a execution engine.
cc @pepyakin @Robbepop