Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Persistent executor #5082

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dima74
Copy link
Contributor

@dima74 dima74 commented Sep 17, 2024

Context

Meta issue: optimizing single peer tps #4727.
It was identified that executor related things takes noticeable amount of time (#3716 (comment)).
Fixes #3716

Solution

Single executor WASM instance will be used for validating all transactions of a block. It gives approximately 10-15% improvement of single peer tps (from 2900 to 3300). However there is a problem with lifetimes and I have to use a hack with std::mem::transmute to bypass borrow checker. Would be glad to hear opinions/suggestions about it.

Review notes (optional)

Primary change is that data stored in wasmtime::Store was changed from CommonState<...> to Option<CommonState<...>>

Checklist

  • I've read CONTRIBUTING.md.
  • (optional) I've written unit tests for the code changes.
  • All review comments have been resolved.
  • All CI checks pass.

@Erigara
Copy link
Contributor

Erigara commented Sep 17, 2024

Not really comfortable with using unsafe.

What is going under the hood is that WasmCache contain inside state transaction with lifetime <'world, 'block, 'state>, but than we actually pass inside StateTransaction with a lifetimes shorter than that with a promise that we will clear state after transaction is validated, right?

@dima74
Copy link
Contributor Author

dima74 commented Sep 17, 2024

Currently WasmCache stores &'world mut StateTransaction<'block, 'state> inside. We create WasmCache before obtaining any StateTransaction, so the first problem is that there is no 'world lifetime when we create WasmCache.

I tried to eliminate 'world lifetime by storing Arc<RefCell<StateTransaction<'block, 'state>>> inside WasmCache. In this case problem will be in categorize_transactions function. Basically:

fn categorize_transactions<'block, 'state>(
    transactions: Vec<AcceptedTransaction>,
    state_block: &'block mut StateBlock<'state>,
) -> Vec<CommittedTransaction> {
    let wasm_cache: WasmCache<'block, 'state> = WasmCache::new();
    validate(tx1, state_block, &mut wasm_cache);
    validate(tx2, state_block, &mut wasm_cache);  // here borrow check error
    ...
}

fn validate(
    tx: AcceptedTransaction,
    state_block: &'block mut StateBlock<'state>,
    wasm_cache: &mut WasmCache<'block, 'state>,
) {
    ...
}

&mut WasmCache<'block, 'state> is invariant over 'block thus calling validate multiple times will fail borrow check

@dima74 dima74 force-pushed the diralik/persistent-executor branch 4 times, most recently from eb45eca to c949df8 Compare October 2, 2024 12:17
@dima74
Copy link
Contributor Author

dima74 commented Oct 2, 2024

Rebased after #5113

DCNick3
DCNick3 previously approved these changes Oct 8, 2024
@dima74 dima74 force-pushed the diralik/persistent-executor branch 2 times, most recently from 3be87fd to 3cfa2da Compare October 16, 2024 19:48
@dima74 dima74 force-pushed the diralik/persistent-executor branch 2 times, most recently from c90171f to b911cba Compare October 18, 2024 11:57
Signed-off-by: Dmitry Murzin <[email protected]>
@dima74 dima74 force-pushed the diralik/persistent-executor branch from b911cba to ad5e300 Compare October 22, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[suggestion] Persistent Executor
4 participants