Skip to content
New issue

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

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

Already on GitHub? # to your account

perf: Persistent executor #5082

Merged
merged 1 commit into from
Nov 26, 2024

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 from 6849085 to 31950d8 Compare September 23, 2024 18:56
@dima74 dima74 force-pushed the diralik/persistent-executor branch from 31950d8 to 309e6cf Compare September 26, 2024 08:19
@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 3 times, most recently from b911cba to ad5e300 Compare October 22, 2024 16:05
@dima74 dima74 force-pushed the diralik/persistent-executor branch from ad5e300 to a70da30 Compare November 13, 2024 14:04
Signed-off-by: Dmitry Murzin <diralik@yandex.ru>
@mversic mversic force-pushed the diralik/persistent-executor branch from a70da30 to 898bec5 Compare November 26, 2024 07:36
@mversic mversic merged commit 33cb188 into hyperledger-iroha:main Nov 26, 2024
10 checks passed
@dima74 dima74 deleted the diralik/persistent-executor branch November 26, 2024 09:21
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[suggestion] Persistent Executor
5 participants