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

Faster pruner #2376

Merged
merged 36 commits into from
Feb 24, 2025
Merged

Faster pruner #2376

merged 36 commits into from
Feb 24, 2025

Conversation

Harrm
Copy link
Contributor

@Harrm Harrm commented Feb 13, 2025

Referenced issues

resolves #1683

Description of the Change

  • Move caching of computed merkle values inside trie nodes. That way the computation of merkle values can be reused between different stages, like pruning and merkle root calculation.
  • Add a benchmark for trie pruner.
  • Disable -fsanitize-ignorelist for GCC, because it doesn't have this option.
  • Move pruning to a separate thread, which still kinda blocks block execution because block execution requires registering the new state with the pruner, which is a mutually exclusive operation with pruning. However, it allows pruning to happen while other block execution stuff is in progress, and in the future allows for more flexible optimizations.
  • Add query-db command to storage explorer.
  • Fix db-editor which has been broken for a while.
  • Ignore empty child tries on Polkadot network.

Possible Drawbacks

  • Caching of merkle values inside trie nodes may be a pessimization in some cases. If such cases are discovered in the future, it is possible to refactor this cache to make it optional at compilation stage.

Checklist Before Opening a PR

Before you open a Pull Request (PR), please make sure you've completed the following steps and confirm by answering 'Yes' to each item:

  1. Code is formatted: Have you run your code through clang-format to ensure it adheres to the project's coding standards? Yes
  2. Code is documented: Have you added comments and documentation to your code according to the guidelines in the project's contributing guidelines? Yes
  3. Self-review: Have you reviewed your own code to ensure it is free of typos, syntax errors, logical errors, and unresolved TODOs or FIXME without linking to an issue? Yes
  4. Zombienet Tests: Have you ensured that the zombienet tests are passing? Zombienet is a network simulation and testing tool used in this project. It's important to ensure that these tests pass to maintain the stability and reliability of the project. No

@Harrm Harrm changed the title Refactor/faster pruner Faster pruner Feb 20, 2025
@Harrm Harrm marked this pull request as ready for review February 20, 2025 11:44
@@ -6,6 +6,7 @@

#pragma once

#include <boost/asio/defer.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use defer?
How is it different from post?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The use of defer(), rather than post , indicates the caller's preference that the executor defer the queueing of the function object."
https://live.boost.org/doc/libs/1_76_0/doc/html/boost_asio/reference/defer/overload1.html


// use optional because MerkleValue doesn't have a default constructor
std::optional<MerkleValue> merkle;
if (policy == TraversePolicy::UncachedOnly
Copy link
Contributor

@turuslan turuslan Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Names if (uncached) return cache look strange?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like TraversePolicy naming very much, bui it's the best I came up with. I'll leave a comment in this place.

[self = shared_from_this()](auto &io_ctx) {
// to let new blocks pass through, otherwise new blocks wait too long
// for pruner mutex
auto timer = std::make_shared<boost::asio::steady_timer>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we create new timer each time or reuse existing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a bit more convenient code-wise to just create a new one, so since this operation is performed rarely, I'd leave it as is.

Copy link
Contributor

@igor-egorov igor-egorov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💪

@Harrm Harrm merged commit f258af6 into master Feb 24, 2025
13 checks passed
@Harrm Harrm deleted the refactor/faster-pruner branch February 24, 2025 13:32
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.

Fix trie pruner's encoder cache
3 participants