chore(lib/runtime/wasmer): common setupVM function
#2685
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
qdm12
commented
Jul 20, 2022
| sync.Mutex | ||
| mutex sync.Mutex |
There was a problem hiding this comment.
Note this was changed since I was having trouble finding references to the Lock/Unlock methods.
As a side note, I think we should avoid embedding things due to that, and also because it exports methods we might not want exported actually (like here, the mutex should be private).
qdm12
requested review from
noot,
edwardmack,
timwu20,
EclesioMeloJunior,
jimjbrettj and
kishansagathiya
as code owners
Codecov Report
@@ Coverage Diff @@
## development #2685 +/- ##
===============================================
- Coverage 62.82% 62.66% -0.17%
===============================================
Files 211 211
Lines 27000 26992 -8
===============================================
- Hits 16964 16915 -49
- Misses 8489 8533 +44
+ Partials 1547 1544 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
qdm12
force-pushed
the
qdm12/wasmer/setupvm-function
branch
from
649b9ad
to
8375fdf
Compare
qdm12
commented
Jul 21, 2022
| } | ||
|
|
||
| in.Lock() | ||
| defer in.Unlock() | ||
| in.ctx.Allocator = allocator // TODO we should no change the allocator of the parent instance |
There was a problem hiding this comment.
This line does the same thing as it did before.
It also looks like some very dangerous risky replacement, but it's changed in the next PR #2687 where CheckRuntimeVersion no longer depends on an existing instance.
EclesioMeloJunior
approved these changes
Jul 22, 2022
- Fix: decompress WASM when checking runtime version - `setupVM` only takes in the wasm code bytes and returns an instance+allocator - WASM instance context data is set outside setupVM - Sentinel error for 'code is empty' - Remove useless debug log (just logs pointer addresses) - OOS: name mutex field to find references easily
- Add `close` method on `Instance` (without mutex) - Use `close` method in `Stop` method (thread safe)
qdm12
force-pushed
the
qdm12/wasmer/setupvm-function
branch
from
8375fdf
to
2ee6693
Compare
timwu20
approved these changes
Jul 26, 2022
EclesioMeloJunior
pushed a commit
that referenced
this pull request
Aug 10, 2022
- Fix: decompress WASM when checking runtime version - Fix: clear allocator when closing instance - Fix: add mutex protection for the entire `UpdateRuntimeCode` body - Add and use new `close` method on `Instance` (without mutex) - Use `close` method in `Stop` method - `setupVM` only takes in the WASM code bytes and returns an instance+allocator - WASM instance context data is set outside setupVM - Minor changes - Sentinel error for 'code is empty' - Remove useless debug log (just logs pointer addresses) - `Instance` named field `mutex` to find references easily - Explicit runtime `Stop` call in `dot/state/initialize.go`
rrtti
pushed a commit
to polkadot-fellows/seeding
that referenced
this pull request
Sep 29, 2022
I am Quentin Mc Gaw, a software engineer working the Go Polkadot host **Gossamer**. I have been working full time on Gossamer since October 2021, mostly on the state trie and storage. I have also made a [few minor pull requests](https://github.com/w3f/polkadot-spec/pulls?q=is%3Apr+is%3Aclosed+author%3Aqdm12) to the Polkadot specification repository. I am requesting to join the Fellowship at rank 1. ## Main contributions ### Gossamer - Fix memory leaks - Trie encoding buffer pools usage fixed [#2009](ChainSafe/gossamer#2009) - Fix state map of tries memory leak [#2286](ChainSafe/gossamer#2286) - Fix sync benchmark [#2234](ChainSafe/gossamer#2234) - Trie proof fixes ([#2604](ChainSafe/gossamer#2604), [#2661](ChainSafe/gossamer#2661)) - Fix end to end tests orchestration ([#2470](ChainSafe/gossamer#2470), [#2452](ChainSafe/gossamer#2452), [#2385](ChainSafe/gossamer#2385), [#2370](ChainSafe/gossamer#2370)) - State trie statistics ([#2378](ChainSafe/gossamer#2378), [#2310](ChainSafe/gossamer#2310), [#2272](ChainSafe/gossamer#2272)) - State trie fixes and improvements - Only deep copy nodes when mutation is certain [#2352](ChainSafe/gossamer#2352) and [#2223](ChainSafe/gossamer#2223) - Only deep copy necessary fields of a node [#2384](ChainSafe/gossamer#2384) - Use Merkle values for database keys instead of always hash [#2725](ChainSafe/gossamer#2725) - Opportunistic parallel Merkle value commputing [#2081](ChainSafe/gossamer#2081) - Grandpa capped number of tracked messages ([#2490](ChainSafe/gossamer#2490), [#2485](ChainSafe/gossamer#2485)) - Add pprof HTTP service for profiling [#1991](ChainSafe/gossamer#1991) Ongoing work: - State trie lazy loading and caching - State trie v1 support ([#2736](ChainSafe/gossamer#2736), [#2747](ChainSafe/gossamer#2747), [#2687](ChainSafe/gossamer#2687), [#2686](ChainSafe/gossamer#2686), [#2685](ChainSafe/gossamer#2685), [#2673](ChainSafe/gossamer#2673), [#2611](ChainSafe/gossamer#2611), [#2530](ChainSafe/gossamer#2530)) ### Polkadot specification ➡️ [Pull requests from qdm12](https://github.com/w3f/polkadot-spec/pulls?q=is%3Apr+is%3Aclosed+author%3Aqdm12)
|
🎉 This PR is included in version 0.7.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
The main goal (not yet achieved) is to get the runtime version from the wasm code byte slice only.
This moves one step closer, by having a shared
setupVMfunction taking in only the wasm code.UpdateRuntimeCodebodyclosemethod onInstance(without mutex)closemethod inStopmethodsetupVMonly takes in the WASM code bytes and returns an instance+allocatorInstancenamed fieldmutexto find references easilyStopcall indot/state/initialize.goTests
Issues
#2418
Primary Reviewer
@timwu20