Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Update wasmtime to 0.33.0 #10674

Merged
merged 2 commits into from
Jan 17, 2022
Merged

Update wasmtime to 0.33.0 #10674

merged 2 commits into from
Jan 17, 2022

Conversation

nazar-pc
Copy link
Contributor

From release notes besides various fixes, there is SIMD support enabled by default.

The primary reason to upgrade was to make Substrate compile on latest nightly, see bytecodealliance/rustix#142 for details.

@bkchr bkchr requested review from koute and pepyakin January 16, 2022 18:23
@bkchr bkchr added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Jan 16, 2022
@bkchr
Copy link
Member

bkchr commented Jan 16, 2022

I think we should probably disable the simd stuff? So that nothing depends on this by accident?

pepyakin
pepyakin suggested changes Jan 16, 2022
View reviewed changes
Copy link
Contributor

@pepyakin pepyakin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is correct. Leaving it as Request changes until it's done.

@nazar-pc
Copy link
Contributor Author

Just curious, is there a roadmap for enabling those various WASM features in wasmtime?
They seem useful for performance, maybe there is a ticket I can follow (I have not been able to find on in this repo)?

@koute
Copy link
Contributor

koute commented Jan 17, 2022

Just curious, is there a roadmap for enabling those various WASM features in wasmtime? They seem useful for performance, maybe there is a ticket I can follow (I have not been able to find on in this repo)?

I don't think there is. But it's good to stay conservative unless we can show that enabling them will actually be beneficial in some way. If there's no benefit then there's no point in turning them on just for the sake of turning them on.

@nazar-pc
Copy link
Contributor Author

Just noticed SIMD was already disabled, so there was no need to change anything.

@koute
Copy link
Contributor

koute commented Jan 17, 2022 •
edited
Loading

Also, @nazar-pc FYI we tend to not force-push in our PRs to clean up the history; we just squash everything on merge.

@nazar-pc
Copy link
Contributor Author

Will keep that in mind going forward 👍

@koute
Copy link
Contributor

koute commented Jan 17, 2022 •
edited
Loading

Just noticed SIMD was already disabled, so there was no need to change anything.

Indeed; I can confirm that we currently disable wasm_simd. (It was false by default in the previous release and they just switched the default to true, but we were already disabling it so nothing changes for us.)

One extra thing we should add is to also disable wasm_memory64. That is false by default for now, but we should just proactively disable it anyway (since they'll probably turn it on by default in the future once the spec is finalized).

koute
koute approved these changes Jan 17, 2022
View reviewed changes
Copy link
Contributor

@koute koute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see no major performance regressions in benchmarks after this PR, so looks good to me.

@pepyakin pepyakin merged commit 249dbbb into paritytech:master Jan 17, 2022
@nazar-pc nazar-pc deleted the wasmtime-0.33.0 branch January 17, 2022 14:59
@svyatonik svyatonik mentioned this pull request Jan 31, 2022
Merged
grishasobol pushed a commit to gear-tech/substrate that referenced this pull request Mar 28, 2022
@nazar-pc @grishasobol
Update wasmtime to 0.33.0 (paritytech#10674)
ebd019f 
Also disable memory64 support in wasmtime
AurevoirXavier pushed a commit to darwinia-network/substrate that referenced this pull request Aug 4, 2022
@nazar-pc @AurevoirXavier
Update wasmtime to 0.33.0 (paritytech#10674)
69ac763 
Also disable memory64 support in wasmtime
@AurevoirXavier AurevoirXavier mentioned this pull request Aug 4, 2022
Merged
AurevoirXavier pushed a commit to darwinia-network/substrate that referenced this pull request Aug 31, 2022
@nazar-pc @AurevoirXavier
Update wasmtime to 0.33.0 (paritytech#10674)
c2c3926 
Also disable memory64 support in wasmtime
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@nazar-pc @ark0f
Update wasmtime to 0.33.0 (paritytech#10674)
23625ec 
Also disable memory64 support in wasmtime
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@koute koute koute approved these changes

@pepyakin pepyakin pepyakin approved these changes

@bkchr bkchr bkchr approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nazar-pc @bkchr @koute @pepyakin