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

[refactor]: Optimize executor #4013

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

Arjentix
Copy link
Contributor

Description

Originally this work was about #3991. But after implementing reference counting for ConstString I got these results:

  1. 7.5 - 13 % less memory footprint
  2. 50 - 75 % slower

I measured memory using the next command:

gtime -f 'Time passed: %E\nMax memory used: %M kbytes' ../../target/release/examples/apply_blocks

Where gtime is a time utility by GNU, which can be installed on mac using brew install gnu-time

So it defenelty doesn't worth it.

You can do your own research. Take a look at my branch: https://github.com/arjentix/iroha/tree/arc_name

  • Fixed apply_blocks benchmark
  • Made apply_blocks even more heavy with long ids
  • After that this benchmarks just stops woring on iroha2-dev, the issue was a very long transaction deallocation inside wasm executor, so fixed that with core::mem::forget()
  • Some minor refactoring

Benefits

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

@Arjentix Arjentix added iroha2-dev The re-implementation of a BFT hyperledger in RUST Refactor Improvement to overall code quality Optimization Something isn't working as well as it should labels Oct 24, 2023
@Arjentix Arjentix self-assigned this Oct 24, 2023
@coveralls
Copy link

coveralls commented Oct 24, 2023

Pull Request Test Coverage Report for Build 6705703228

  • 50 of 66 (75.76%) changed or added relevant lines in 6 files are covered.
  • 6965 unchanged lines in 129 files lost coverage.
  • Overall coverage decreased (-3.5%) to 55.901%

Changes Missing Coverage Covered Lines Changed/Added Lines %
smart_contract/derive/src/entrypoint.rs 0 1 0.0%
smart_contract/trigger/derive/src/entrypoint.rs 0 1 0.0%
core/src/query/store.rs 0 2 0.0%
smart_contract/executor/derive/src/entrypoint.rs 0 2 0.0%
core/src/smartcontracts/isi/asset.rs 3 13 23.08%
Files with Coverage Reduction New Missed Lines %
client/src/lib.rs 1 0.0%
config/base/derive/src/view.rs 1 99.37%
config/src/block_sync.rs 1 95.0%
config/src/network.rs 1 93.75%
config/src/torii.rs 1 96.0%
config/src/wasm.rs 1 87.5%
core/src/smartcontracts/isi/block.rs 1 87.5%
config/src/kura.rs 2 79.41%
config/src/lib.rs 2 0.0%
ffi/src/option.rs 2 71.43%
Totals Coverage Status
Change from base Build 5423219773: -3.5%
Covered Lines: 22143
Relevant Lines: 39611

💛 - Coveralls

Erigara
Erigara previously approved these changes Oct 27, 2023
mversic
mversic previously approved these changes Oct 31, 2023
Signed-off-by: Daniil Polyakov <[email protected]>
@Arjentix Arjentix dismissed stale reviews from mversic and Erigara via 1f91725 October 31, 2023 11:14
@Arjentix Arjentix merged commit 4fccf3a into hyperledger-iroha:iroha2-dev Oct 31, 2023
@Arjentix Arjentix deleted the optimize_executor branch October 31, 2023 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST Optimization Something isn't working as well as it should Refactor Improvement to overall code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants