Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Make trie-cache resettable from backend #14516

Merged
merged 6 commits into from
Jul 18, 2023

Conversation

skunert
Copy link
Contributor

@skunert skunert commented Jul 4, 2023

I have some benchmarks in cumulus where I want to reset the trie cache in between runs.

@skunert skunert added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T0-node This PR/Issue is related to the topic “node”. labels Jul 4, 2023
@skunert skunert requested a review from a team July 4, 2023 18:14
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

In general, can you not just recreate the backend? As we do in the Substrate based benchmarks. Then you don't need this function.

client/db/src/lib.rs Outdated Show resolved Hide resolved
Co-authored-by: Bastian Köcher <git@kchr.de>
@skunert
Copy link
Contributor Author

skunert commented Jul 5, 2023

In general, can you not just recreate the backend? As we do in the Substrate based benchmarks. Then you don't need this function.

You mean like here?

let backend = create_backend(config, &path);

The substrate benchmarks reset the backend once for each benchmark. For my purposes I want to reset per iteration, so I would need to create a whole new client (not only backend) for each iteration. Being able to reset the trie cache seemed like the more elegant option here. But if you prefer I can look into that.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Yeah, let's do it this way.

Not ultra great, but I also don't see any better way for now.

@skunert skunert requested a review from a team July 5, 2023 10:36
Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍

@skunert
Copy link
Contributor Author

skunert commented Jul 18, 2023

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot
Copy link

Merge cancelled due to error. Error: Statuses failed for efaeb51

@skunert skunert merged commit 839cf0c into paritytech:master Jul 18, 2023
4 checks passed
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* Add ability to reset trie-cache

* comment

* Update client/db/src/lib.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Ank4n pushed a commit that referenced this pull request Jul 22, 2023
* Add ability to reset trie-cache

* comment

* Update client/db/src/lib.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants