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

Docs for Frame Benchmarking #7121

Merged
20 commits merged into from
Sep 21, 2020
Merged

Docs for Frame Benchmarking #7121

20 commits merged into from
Sep 21, 2020
+189 −385

Conversation

shawntabrizi
Copy link
Member

Here are some introductory docs for the frame_benchmarking pallet.

This is not meant to be a comprehensive tutorial for writing benchmarks, that probably should live outside of this repository, but is meant to go over the high level ideas, and how a user can get started using the benchmarks that already exist.

This PR also removes the pallet_benchmark crate which was just a test playground that really shouldn't be here.

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Sep 16, 2020
shawntabrizi
shawntabrizi commented Sep 16, 2020
View reviewed changes
@shawntabrizi shawntabrizi added 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. and removed D2-breaksapi labels Sep 17, 2020
kianenigma
kianenigma reviewed Sep 17, 2020
View reviewed changes
kianenigma
kianenigma reviewed Sep 17, 2020
View reviewed changes
kianenigma
kianenigma reviewed Sep 17, 2020
View reviewed changes
kianenigma
kianenigma reviewed Sep 17, 2020
View reviewed changes
kianenigma
kianenigma reviewed Sep 17, 2020
View reviewed changes
frame/benchmarking/README.md
```

This will output a file `pallet_name.rs` which implements the `WeightInfo` trait you should include
Copy link
Contributor

Choose a reason for hiding this comment

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

WeightInfo is never explained in this doc. I think could use some explanation further up in Writing Benchmarks section.

dvdplm
dvdplm reviewed Sep 17, 2020
View reviewed changes
frame/benchmarking/README.md
/// | name of your pallet's crate (as imported)
/// v v
add_benchmark!(params, batches, pallet_balances, Balances);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be a #[cfg(feature = "runtime_benchmarks")] before this?

Copy link
Member Author

Choose a reason for hiding this comment

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

ehh, its actually already wrapped in a runtime api which has this feature flag.

I have omitted a lot of code here for simplicity, but I think if you find this text in substrate, it would be clear what to do.

Copy link
Member Author

Choose a reason for hiding this comment

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

you can let me know otherwise

frame/benchmarking/README.md
Comment on lines +170 to +171
in your pallet. Each blockchain should generate their own benchmark file with their custom
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit unclear to me: the file is created in the crate root? Need to be copied into the runtime source file? Or…?

Do we have any tooling available to list un-weighted pallets or dispatchables? Like "test coverage" but for weights.

Copy link
Member Author

Choose a reason for hiding this comment

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

file will be created in your working directory where you executed the command. It will be a todo feature to support a custom path for the file.

No tools available atm for test coverage of weights. also a good idea for CI

joepetrowski
joepetrowski reviewed Sep 17, 2020
View reviewed changes
frame/benchmarking/README.md
the benchmark. (called "components")
* Executing the benchmark multiple times at each point in order isolate and remove outliers.
* Using the results of the benchmark to create a linear model of the function across its components.
Copy link
Contributor

Choose a reason for hiding this comment

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

It means that the results tend to be linear functions of their inputs. For example, if a DB read in a trie of depth 4 takes 100 ns, then we might reasonably expect a depth of 5 to take 125ns, 6 to take 150ns, etc. A nonlinearity would be if a depth of 7 made the read time jump to 280ns.

shawntabrizi and others added 3 commits September 17, 2020 15:52
@shawntabrizi @dvdplm
@joepetrowski @kianenigma
Apply suggestions from code review

Partially verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
a5ccafc 
Co-authored-by: David <dvdplm@gmail.com>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
@shawntabrizi @dvdplm @joepetrowski
Apply suggestions from code review

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
2155d55 
Co-authored-by: David <dvdplm@gmail.com>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
@shawntabrizi @joepetrowski @dvdplm
Apply suggestions from code review

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
5398000 
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-authored-by: David <dvdplm@gmail.com>
shawntabrizi and others added 2 commits September 17, 2020 16:00
@shawntabrizi @dvdplm @joepetrowski
Apply suggestions from code review

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
2f138de 
Co-authored-by: David <dvdplm@gmail.com>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
@shawntabrizi @dvdplm
Update frame/benchmarking/README.md

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
3fd0b03 
Co-authored-by: David <dvdplm@gmail.com>
shawntabrizi
shawntabrizi commented Sep 17, 2020
View reviewed changes
shawntabrizi and others added 5 commits September 17, 2020 16:01
@shawntabrizi
Update frame/benchmarking/README.md

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
a65ca30
@shawntabrizi @joepetrowski
Update frame/benchmarking/README.md

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
77cf3e0 
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
@shawntabrizi
Update README.md
ec27cdc
@shawntabrizi
Merge branch 'shawntabrizi-benchmarking-docs' of https://github.com/p…
246e374 
…aritytech/substrate into shawntabrizi-benchmarking-docs
@shawntabrizi
Update README.md
ec6c024
dvdplm
dvdplm approved these changes Sep 17, 2020
View reviewed changes
Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

minor nits

@shawntabrizi
Copy link
Member Author

bot merge

@ghost
Copy link

ghost commented Sep 21, 2020

Trying merge.

@ghost ghost merged commit 458e4a5 into master Sep 21, 2020
@ghost ghost deleted the shawntabrizi-benchmarking-docs branch September 21, 2020 21:10
rakanalh pushed a commit to rakanalh/substrate that referenced this pull request Sep 22, 2020
@shawntabrizi @dvdplm
@joepetrowski @kianenigma
Docs for Frame Benchmarking (paritytech#7121)

Unverified

No user is associated with the committer email.
Learn about vigilant mode
adeed50 
* remove test benchmark pallet

* docs

* finish docs

* Update README.md

* simplify intro

* introduce weight later

* Apply suggestions from code review

Co-authored-by: David <dvdplm@gmail.com>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: David <dvdplm@gmail.com>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-authored-by: David <dvdplm@gmail.com>

* Apply suggestions from code review

Co-authored-by: David <dvdplm@gmail.com>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Update frame/benchmarking/README.md

Co-authored-by: David <dvdplm@gmail.com>

* Update frame/benchmarking/README.md

* Update frame/benchmarking/README.md

Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Update README.md

* Update README.md

* Update README.md

* Update README.md

Co-authored-by: David <dvdplm@gmail.com>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@joepetrowski joepetrowski

@dvdplm dvdplm

@kianenigma kianenigma

@danforbes danforbes

@gavofyork gavofyork

Assignees
No one assigned
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@shawntabrizi @dvdplm @kianenigma @joepetrowski @paritytech-ci