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.

Sign up for GitHub

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

Jump to bottom

Avoid unnecessary hashing in transaction application #14643

Closed
wants to merge 1 commit into from
Closed

Avoid unnecessary hashing in transaction application #14643

wants to merge 1 commit into from

Conversation

georgeee
Copy link
Member

@georgeee georgeee commented Nov 30, 2023 •
edited
Loading

This PR contains three changes:

  • Removes hash recomputation on Ledger.commit
    • It alone improves performance by ~20% (measured on a laptop in a test generating 9-account-update transactions)
  • Introduces handling for non-existent accounts when Ledger.unsafe_preload_accounts_from_parent is used
    • As of now, DB will never be accessed during transaction application, only once before the procedure starts
  • Removes unnecessary hash computation when setting accounts
    • Batches updating hashes of a few subsequent account updates
    • It alone improves performance by ~60% (measured on a laptop in a test generating 9-account-update transactions)

Total improvement of this PR is around 70% comparing to base branch.

Explain how you tested your changes:

Checklist:

  • Dependency versions are unchanged
    • Notify Velocity team if dependencies must change in CI
  • Modified the current draft of release notes with details on what is completed or incomplete within this project
  • Document code purpose, how to use it
    • Mention expected invariants, implicit constraints
  • Tests were added for the new behavior
    • Document test purpose, significance of failures
    • Test names should reflect their purpose
  • All tests pass (CI will check this if you didn't)
  • Serialized types are in stable-versioned modules
  • Does this close issues? None

@georgeee georgeee requested a review from a team as a code owner November 30, 2023 14:23
@georgeee georgeee force-pushed the georgeee/avoid-slowdown-of-multiple-masks branch from ac44c5c to d05242c Compare December 1, 2023 21:09
@georgeee georgeee force-pushed the georgeee/optimize-hash-computation branch from 5045183 to 9adcdf4 Compare December 7, 2023 18:43
Base automatically changed from georgeee/avoid-slowdown-of-multiple-masks to rampup December 7, 2023 20:27
@georgeee georgeee force-pushed the georgeee/optimize-hash-computation branch from 9adcdf4 to 0d0f7fd Compare December 9, 2023 00:02
@georgeee
Copy link
Member Author

georgeee commented Dec 9, 2023

!ci-build-me

@georgeee
Copy link
Member Author

georgeee commented Dec 9, 2023

!ci-nightly-me

@georgeee georgeee force-pushed the georgeee/optimize-hash-computation branch from 034388c to 12cb6b7 Compare December 13, 2023 10:21
@georgeee
Copy link
Member Author

!ci-build-me

@georgeee
Copy link
Member Author

!ci-nightly-me

@georgeee georgeee mentioned this pull request Dec 20, 2023
Open
@georgeee georgeee changed the base branch from rampup to compatible August 25, 2024 22:53
@georgeee georgeee force-pushed the georgeee/optimize-hash-computation branch from 4014c48 to 05c6ea5 Compare August 25, 2024 23:24
@georgeee
Copy link
Member Author

Confirm 2x-2.5x performance improvement of staged ledger diff application as measured by #14582

@georgeee georgeee mentioned this pull request Aug 26, 2024
Open
@georgeee
Copy link
Member Author

!ci-build-me

@georgeee georgeee force-pushed the georgeee/optimize-hash-computation branch 5 times, most recently from cf02bd8 to 349cebb Compare August 26, 2024 12:54
@georgeee georgeee changed the base branch from compatible to georgeee/defer-hashing-in-staged-ledger-diff-application August 26, 2024 12:54
@georgeee georgeee force-pushed the georgeee/defer-hashing-in-staged-ledger-diff-application branch from 03e7683 to d8b44f3 Compare August 26, 2024 13:18
@georgeee
Small optimizations
3dc68ec 
1. Preload empty hash paths (ensures no hash db lookups even in edge
   cases)
2. Use `t.current_location` instead of `last_filled t` and check against
   `non_existent_accounts` in `get_or_create_account`
@georgeee georgeee force-pushed the georgeee/optimize-hash-computation branch from 349cebb to 3dc68ec Compare August 26, 2024 13:21
@georgeee
Copy link
Member Author

Closing in favor of PRs #15978, #15979, #15980

@georgeee georgeee closed this Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@georgeee