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

bench pallet: only require Hash instead of Block #3244

Merged
merged 11 commits into from
Feb 8, 2024
Merged

bench pallet: only require Hash instead of Block #3244

merged 11 commits into from
Feb 8, 2024

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Feb 7, 2024 •
edited
Loading

Preparation for #2664

Changes:

  • Only require Hash instead of Block for the benchmarking
  • Refactor DB types to do the same

Integration

This breaking change can easily be integrated into your node via:

- cmd.run::<Block, ()>(config)
+ cmd.run::<HashingFor<Block>, ()>(config)

Status: waiting for CI checks

@ggwpez
Only require Hash instead of Block
03b1297 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez added the T12-benchmarks This PR/Issue is related to benchmarking and weights. label Feb 7, 2024
@ggwpez
Add prdoc
ef7e19e 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez changed the title Benchmarking: only require Hash instead of Block bench pallet: only require Hash instead of Block Feb 7, 2024
@ggwpez
Fix compile
ff75e90 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5143341

@ggwpez
Just remove the old run function
f3a9b6f 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez marked this pull request as ready for review February 7, 2024 22:26
@ggwpez ggwpez requested a review from cheme February 7, 2024 22:26
cheme
cheme approved these changes Feb 8, 2024
View reviewed changes
Copy link
Contributor

@cheme cheme left a comment

Choose a reason for hiding this comment

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

Sounds good to me.

polkadot/cli/src/command.rs Show resolved Hide resolved
substrate/client/db/src/bench.rs Outdated Show resolved Hide resolved
@bkchr @cheme
Update substrate/client/db/src/bench.rs
6d28c70 
Co-authored-by: cheme <emericchevalier.pro@gmail.com>
bkchr
bkchr approved these changes Feb 8, 2024
View reviewed changes
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.

Some nitpicks, otherwise looks good.

substrate/client/db/src/bench.rs Outdated Show resolved Hide resolved
prdoc/pr_3244.prdoc Outdated Show resolved Hide resolved
prdoc/pr_3244.prdoc Outdated Show resolved Hide resolved
ggwpez and others added 6 commits February 8, 2024 17:00
@ggwpez @bkchr
Update substrate/client/db/src/bench.rs
7f903a0 
Co-authored-by: Bastian Köcher <git@kchr.de>
@ggwpez @bkchr
Update prdoc/pr_3244.prdoc
4eba3ee 
Co-authored-by: Bastian Köcher <git@kchr.de>
@ggwpez
prdoc: mention patch
b78a983 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
prdoc: hash -> hasher
465e3bd 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
toml formatting
9f4ba8c 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
typos
4c6fdc5 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez enabled auto-merge February 8, 2024 16:15
@ggwpez ggwpez added this pull request to the merge queue Feb 8, 2024
Merged via the queue into master with commit c36c51c Feb 8, 2024
123 of 125 checks passed
@ggwpez ggwpez deleted the oty-bench-pallet-from-wasm branch February 8, 2024 18:06
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
@ggwpez @bkchr @cheme
bench pallet: only require Hash instead of Block (paritytech#3244)
1e07dfa 
Preparation for paritytech#2664

Changes:
- Only require `Hash` instead of `Block` for the benchmarking
- Refactor DB types to do the same

## Integration

This breaking change can easily be integrated into your node via:  
```patch
- cmd.run::<Block, ()>(config)
+ cmd.run::<HashingFor<Block>, ()>(config)
```

Status: waiting for CI checks

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: cheme <emericchevalier.pro@gmail.com>
This was referenced Jun 5, 2024
Closed
Closed
Closed
Closed
@ahmadkaouk ahmadkaouk mentioned this pull request Jun 20, 2024
Merged
@chungquantin chungquantin mentioned this pull request Aug 4, 2024
Closed
4 tasks
@chungquantin chungquantin mentioned this pull request Aug 12, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cheme cheme cheme approved these changes

@bkchr bkchr bkchr approved these changes

Assignees
No one assigned
Labels
T12-benchmarks This PR/Issue is related to benchmarking and weights.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ggwpez @paritytech-cicd-pr @cheme @bkchr