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

[suggestion] Persistent Executor #3716

Open
5 tasks
appetrosyan opened this issue Jul 18, 2023 · 4 comments · May be fixed by #5082
Open
5 tasks

[suggestion] Persistent Executor #3716

appetrosyan opened this issue Jul 18, 2023 · 4 comments · May be fixed by #5082
Assignees
Labels
Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST question Further information is requested

Comments

@appetrosyan
Copy link
Contributor

Feature request

The executor should persist between execution of different transactions. Ideally it should be brought up either after an upgrade or after a quick recovery (from a power outage). It should not have significant persistent storage (caching is fine, storing information that can affect the next verdict is not).

@mversic suspects that wasmtime could have an out-of-memory condition if WASM is not periodically blanked, so the suggestion is to start with a block scope and then extend it further to longer periods of time. I Suggest making it a configuration parameter.

In detail we need the following:

  • Persistence configuration parameter, in configs/peer/config.json that can be Every n transactions, every n blocks, until peer crashes, defaulting to every 1 transaction.
  • Infrastructure to periodically purge the Validator/executor every n of x entity.
  • Tests that produce an out-of-memory condition. Install guard rails to prevent Iroha from crashing in that situation.
  • Tests that verify that the current executor does not cause an out-of-memory condition under load.
  • A test that verifies that the current executor does not leak memory (by unloading itself, and comparing resident memory with the last executor).

Motivation

The operation of loading and unloading an executor affects the performance of regular transaction processing, so it makes sense to optimise the process and avoid unnecessary loading and blanking of memory if the old memory does a good-enough job.

Who can help?

@mversic @appetrosyan

@appetrosyan appetrosyan added Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST labels Jul 18, 2023
@mversic mversic added the question Further information is requested label Oct 19, 2023
@dima74 dima74 self-assigned this Aug 20, 2024
@dima74
Copy link
Contributor

dima74 commented Aug 20, 2024

So here is flamegraph for sumeragi main loop (I am using cargo flamegraph):

image

Looks like instantiate takes a lot of time, however it turns out that profiling doesn't take wasm code into account.


Here are measurements how much time some functions take for single transaction (1600 tps load with config as here). All values are in microseconds, values for try_create_block and categorize_transactions are normalized (divided by block size = 20).

try_create_block                          399
categorize_transactions                   371
TransactionExecutor::validate             370
TypedFunc::call                           328
Runtime::instantiate_module                12

So we can see that instantiate takes less then 5% and most of the time takes actual execution of wasm code. Need to investigate how to speed up wasm code, in particular #4803

Related: #4727

@mversic
Copy link
Contributor

mversic commented Aug 20, 2024

related #4914

@dima74
Copy link
Contributor

dima74 commented Sep 13, 2024

After #5048 merged, executor related things (linker initialization, module instantiation, memory free) started to take noticeable amount of flamegraph. Potentially we could get good tps improvement with persistent executor, but there are problems with its implementation related to lifetimes.

image

@DCNick3
Copy link
Contributor

DCNick3 commented Sep 13, 2024

Using Linker::instantiate_pre might help the performance without the downsides of re-using, since, AFAIU, it has all the imports resolved already and can be instantiated multiple times.

@dima74 dima74 linked a pull request Sep 17, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants