search

near / nearcore

Reference client for NEAR Protocol
https://near.org
GNU General Public License v3.0
2.31k stars 619 forks source link

refactor: make RuntimeExt: Send #11634

Closed nagisa closed 3 months ago

nagisa commented 3 months ago

In a world where we have pipelined compilation, instantiation and execution, VMLogic will have to move between threads, which requires that it becomes Send. It in turn has required some other types to become not only Send but also Sync due to them currently being stored as a & reference (which allows for multiple copies, there are better places to explain why Sync becomes necessary here...)

I'm not sure if all of these types will continue requiring Sync. In particular TrieUpdate that's stored in RuntimeExt is now by reference, but I eventually want to also make VMLogic: 'static, which would require finding some owning pointer structure that would work for TrieUpdate... Or I might be able to use scoped threads... in which case we're looking at Sync anyway...

I think the changes here are largely straightforward enough, but overall things are shaping to be pretty involved, eh?

Part of #11319

nagisa commented 3 months ago

I don't anticipate parallel writes to TrieUpdate at all. The pipelined compilation/instantiation only needs access to potentially read out the contract code. While this may happen in a different thread, the timeline is strictly disjoint between threads seeing that specific TrieUpdate...

In fact ideally we wouldn't deal with TrieUpdates here at all, but instantiation requires us to construct a full-featured VMLogic that is going to be used for contract execution (including the entirety of the knowledge of how to read and write to the storage), so it kinda needs to exist early on...

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 79.14439% with 39 lines in your changes missing coverage. Please review.

Project coverage is 71.64%. Comparing base (e5ad448) to head (571a314). Report is 11 commits behind head on master.

Files Patch % Lines
tools/state-parts-dump-check/src/cli.rs 0.00% 6 Missing :warning:
tools/state-viewer/src/commands.rs 0.00% 5 Missing :warning:
chain/client/src/debug.rs 0.00% 4 Missing :warning:
chain/client/src/sync/state.rs 50.00% 0 Missing and 3 partials :warning:
chain/epoch-manager/src/lib.rs 89.28% 2 Missing and 1 partial :warning:
chain/chain/src/test_utils/kv_runtime.rs 80.00% 2 Missing :warning:
core/store/src/trie/prefetching_trie_storage.rs 33.33% 2 Missing :warning:
runtime/runtime/src/ext.rs 88.88% 2 Missing :warning:
tools/fork-network/src/cli.rs 0.00% 2 Missing :warning:
tools/state-viewer/src/latest_witnesses.rs 0.00% 2 Missing :warning:
... and 8 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #11634 +/- ## ========================================== + Coverage 71.46% 71.64% +0.17% ========================================== Files 788 787 -1 Lines 160921 160945 +24 Branches 160921 160945 +24 ========================================== + Hits 115008 115308 +300 + Misses 40893 40594 -299 - Partials 5020 5043 +23 ``` | [Flag](https://app.codecov.io/gh/near/nearcore/pull/11634/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | Coverage Δ | | |---|---|---| | [backward-compatibility](https://app.codecov.io/gh/near/nearcore/pull/11634/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <0.00%> (-0.01%)` | :arrow_down: | | [db-migration](https://app.codecov.io/gh/near/nearcore/pull/11634/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <0.00%> (-0.01%)` | :arrow_down: | | [genesis-check](https://app.codecov.io/gh/near/nearcore/pull/11634/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.36% <0.00%> (+<0.01%)` | :arrow_up: | | [integration-tests](https://app.codecov.io/gh/near/nearcore/pull/11634/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `37.76% <64.17%> (+0.14%)` | :arrow_up: | | [linux](https://app.codecov.io/gh/near/nearcore/pull/11634/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `69.05% <77.00%> (+0.15%)` | :arrow_up: | | [linux-nightly](https://app.codecov.io/gh/near/nearcore/pull/11634/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.11% <79.14%> (+0.15%)` | :arrow_up: | | [macos](https://app.codecov.io/gh/near/nearcore/pull/11634/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `52.64% <51.07%> (+0.06%)` | :arrow_up: | | [pytests](https://app.codecov.io/gh/near/nearcore/pull/11634/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.59% <0.00%> (+<0.01%)` | :arrow_up: | | [sanity-checks](https://app.codecov.io/gh/near/nearcore/pull/11634/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.39% <0.00%> (+<0.01%)` | :arrow_up: | | [unittests](https://app.codecov.io/gh/near/nearcore/pull/11634/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `66.21% <66.84%> (+0.08%)` | :arrow_up: | | [upgradability](https://app.codecov.io/gh/near/nearcore/pull/11634/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.28% <0.00%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.