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/skip commit queue #7571

Merged
merged 16 commits into from
Oct 10, 2024
Merged

Perf/skip commit queue #7571

merged 16 commits into from
Oct 10, 2024

Conversation

asdacap
Copy link
Contributor

@asdacap asdacap commented Oct 8, 2024

  • Turns out, when catching up, the time taking for commit patricia trie is about 20%
  • This PR optimize that a bit by changing the interface for committing and skipping commit queue within PatriciaTrie.
  • Also, because of the interface change, some checks are removed and it can use ref TreePath for the path.
  • Also parallelize the commit.
  • This improve overall block rate by about 5-10%.
  • Graph is after, before, after, before.
  • Commit storage time need a higher level change to parallelize. Maybe next week.
    Screenshot from 2024-10-08 22-57-45

Changes

  • Remove CommitNode/FinishBlockCommit to a BeginCommit which returns a disposable ICommitter.
  • PatriciaTrie call ICommitter.CommitNode instead of putting to a queue first

Types of changes

What types of changes does your code introduce?

  • Optimization

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

  • Still....

@asdacap asdacap marked this pull request as ready for review October 8, 2024 14:59
@asdacap asdacap marked this pull request as draft October 8, 2024 15:13
@asdacap asdacap marked this pull request as ready for review October 8, 2024 23:02
@LukaszRozmej LukaszRozmej mentioned this pull request Oct 10, 2024
16 tasks
@asdacap asdacap merged commit 8296752 into master Oct 10, 2024
73 checks passed
@asdacap asdacap deleted the perf/skip-commit-queue branch October 10, 2024 09:25
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.

3 participants