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

Custom Blockchains > Benchmarking #121

Merged
merged 15 commits into from
Nov 14, 2024
Merged

Custom Blockchains > Benchmarking #121

merged 15 commits into from
Nov 14, 2024

Conversation

CrackTheCode016
Copy link
Collaborator

Adds the Benchmarking page, which is based on:

Only relevant content was ported, the rest was written to reflect the latest in benchmarking.

@CrackTheCode016 CrackTheCode016 self-assigned this Oct 24, 2024
@CrackTheCode016 CrackTheCode016 requested a review from a team as a code owner October 24, 2024 18:41
@CrackTheCode016 CrackTheCode016 added B0 - Needs Review Pull request is ready for review and removed In Progress labels Oct 24, 2024
@CrackTheCode016
Copy link
Collaborator Author

@nhussein11 @0xLucca Added some barebones content here, could you take a look and see what is missing / what is unclear? Thanks!

0xLucca
0xLucca previously approved these changes Oct 24, 2024
develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
@0xLucca 0xLucca dismissed their stale review October 24, 2024 21:35

Approved by error

@kianenigma
Copy link
Contributor

I have very recently added a new reference doc on this:

https://paritytech.github.io/polkadot-sdk/master/polkadot_sdk_docs/reference_docs/frame_benchmarking_weight/index.html

it is very short, but covers the fundamentals pretty well.

@CrackTheCode016
Copy link
Collaborator Author

@kianenigma Oh perfect, I have not used the omni_bencher yet, do you think it is ready to be implemented as the primary method here?

CrackTheCode016 and others added 2 commits October 25, 2024 15:42
Co-authored-by: 0xLucca <95830307+0xLucca@users.noreply.github.com>
@CrackTheCode016
Copy link
Collaborator Author

CrackTheCode016 commented Oct 28, 2024

I spent some time with frame-omni-benchmark - it seems like it needs a little more time before including it in a doc like this, namely ensuring it works with templates (right now, the blocker is having naturally occuring presets in a template, see: paritytech/polkadot-sdk-parachain-template#19)

For now, will stick to the "old" approach. It will not take much to replace with omni-bencher once everything is synced up.

Edit: we should make a note for a follow up PR as per this: paritytech/polkadot-sdk-parachain-template#19 (comment) cc @kapetan3sid

@kianenigma
Copy link
Contributor

kianenigma commented Oct 28, 2024

@kianenigma Oh perfect, I have not used the omni_bencher yet, do you think it is ready to be implemented as the primary method here?

One major PR is missing before it can reach feature parity with the old one: paritytech/polkadot-sdk#5303

But for 99% of the teams, it is already enough; and it is more future proof.

Copy link
Collaborator

@nhussein11 nhussein11 left a comment

Choose a reason for hiding this comment

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

LGTM besides those small details! thank you!!


```rust
frame_benchmarking::define_benchmarks!(
Copy link
Contributor

Choose a reason for hiding this comment

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

https://paritytech.github.io/polkadot-sdk/master/frame_benchmarking/macro.define_benchmarks.html

Is this good enough? can you contribute back anything to this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe an example + slightly clearer description of what that looks like syntactically to better illustrate what is going on, I can open a PR and do this if wanted

CrackTheCode016 and others added 3 commits November 7, 2024 14:42
Co-authored-by: Nicolás Hussein <80422357+nhussein11@users.noreply.github.com>
@dawnkelly09 dawnkelly09 self-requested a review November 12, 2024 19:56
@dawnkelly09 dawnkelly09 requested review from eshaben and removed request for eshaben and dawnkelly09 November 13, 2024 15:51
@dawnkelly09
Copy link
Collaborator

@polkadot-developers/docs-content-team: I did some formatting and grammar work on this page. Two asks:

  1. Please take a quick look and make sure this is all still accurate, etc.
  2. Please let me know if all the discussions/decisions in the convos are resolved so we know if content is done with this and we can merge once we're happy with styling/grammar stuff
    Thank you!

Copy link
Collaborator

@dawnkelly09 dawnkelly09 left a comment

Choose a reason for hiding this comment

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

Made grammar and formatting updates. Needs approval from content and then final look from Erin or another formatting/grammar reviewer

@eshaben eshaben requested a review from a team November 14, 2024 04:41
Copy link
Collaborator

@eshaben eshaben left a comment

Choose a reason for hiding this comment

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

Looks good! Just some minor formatting things 🙂

develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
develop/blockchains/custom-blockchains/benchmarking.md Outdated Show resolved Hide resolved
@dawnkelly09 dawnkelly09 requested a review from eshaben November 14, 2024 17:28
Copy link
Collaborator

@eshaben eshaben left a comment

Choose a reason for hiding this comment

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

LGTM!

@eshaben eshaben merged commit f02cb6c into master Nov 14, 2024
@eshaben eshaben deleted the bd-benchmarking branch November 14, 2024 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B0 - Needs Review Pull request is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants