Add `benchmark machine` by ggwpez · Pull Request #11198 · paritytech/substrate

ggwpez · 2022-04-11T09:42:02Z

👉 Please put feedback about this command either in here or ping me on matrix @oliver.tale-yazdi:matrix.parity.io.

First version of a benchmark machine command using the sc-sysinfo crate (:pray: @koute).
@shawntabrizi wanted to have a MVP version handy.

Changes:

Add benchmark machine command
Integrate it into the substrate node and node-template
Move new_rng to shared
Add --no-hardware-benchmarks where applicable

Output:

$ substrate benchmark machine --dev

+----------+----------------+-------+------+
| Category | Function       | Score | Unit |
+----------+----------------+-------+------+
| CPU      | BLAKE2-256     | 1029  | MB/s |
+----------+----------------+-------+------+
| CPU      | SR25519 Verify | 666.6 | KB/s |
+----------+----------------+-------+------+
| Memory   | Copy           | 14668 | MB/s |
+----------+----------------+-------+------+
| Disk     | Seq Write      | 502   | MB/s |
+----------+----------------+-------+------+
| Disk     | Rnd Write      | 227   | MB/s |
+----------+----------------+-------+------+

Next steps:

Add check that the build profile was production
Extract more info from sc-sysinfo (eg. total time, reps, input len)
Pass in thresholds such that every chain can have theirs own requirements
Print results relative to thresholds
Measure on ref-hardware to get polkadot thresholds

Polkadot companion: paritytech/polkadot#5326
Cumulus companion: paritytech/cumulus#1172

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

This reverts commit d4ee982.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

utils/frame/benchmarking-cli/src/machine/mod.rs

koute · 2022-04-12T06:27:49Z

utils/frame/benchmarking-cli/src/machine/mod.rs

+	pub fn run(&self) -> Result<()> {
+		info!("Running machine benchmarks...");
+		let tmp_dir = tempdir()?;
+		let info = gather_hwbench(Some(tmp_dir.path()));


It'd probably be better to modify sc-sysinfo to expose the benchmark_* functions separately and call them one by one here instead of relying the gather_hwbench.

The gather_hwbench is deliberately simple (only takes a path as an argument) and ignores error (if the disk benches fail it'll set their results to None and only print out a warning in the logs) since it's meant to be run at startup to gather the data for telemetry.

Here however it'd be good to 1) have direct access to the errors (which the benchmark_* functions do return), and 2) be able to configure the benchmarks to be able to override the limit of how long they can run and for how many iterations since this subcommand doesn't have the same timing constraints as the telemetry (so we can just add an extra argument to the benchmark_* functions which would allow you to override that).

Yea getting some more configurations in there would be good.

utils/frame/benchmarking-cli/src/shared/mod.rs

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

client/sysinfo/src/sysinfo.rs

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

gilescope · 2022-04-14T15:18:04Z

In the spirit of pits of success, maybe add an extra line in the output if they're running in debug mode warning them that they should only trust the results if they are running in release mode?

client/sysinfo/src/sysinfo.rs

kianenigma · 2022-04-14T16:03:45Z

client/sysinfo/src/sysinfo.rs

+		.map(|s| s as u64)
+}
+
+pub fn benchmark_sr25519_verify(limit: ExecutionLimit) -> f64 {


nit: missing doc on public function. Does it need to be public?

Will add 😄
Currently it needs to be public since I call these functions directly. IMHO it would be better to have a gather function which calls all of them and returns a struct,
but thats probably next.

IMHO it would be better to have a gather function which calls all of them and returns a struct,

Hmm... although the downside of that is that you won't be able to configure them separately (set a different ExecutionLimit for each benchmark), unless you add another struct which will have multiple ExecutionLimits in it, but then if you're going to do that you might as well have them as a separate functions I guess?

(And it might make sense to set different limits on each benchmark because e.g. the disk benchmarks are going to require more time than the memory benchmark to make sure their result isn't noisy, if you absolutely want to minimize their error.)

kianenigma

LGTM!

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

kianenigma · 2022-04-15T12:09:08Z

bot merge

* Move new_rng to shared code Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Add bechmark machine command Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Use sc-sysinfo Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Add --no-hardware-benchmarks Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Lockfile Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Do not create components if not needed Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fix tests Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Revert "Add --no-hardware-benchmarks" This reverts commit d4ee982. * Fix tests Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Update Cargo deps Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Move sr255119::verify bench to sc-sysinfo Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Move sr255119::verify bench to sc-sysinfo Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Switch benchmarks to return f64 Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Review fixes Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * fmt Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Hide command until completed Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Use concrete rand implementation Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Put clobber into a function Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Add test Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Add comment Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Update cargo to match polkadot Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Remove doc that does not format in the console Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Limit benchmark by time Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Add ExecutionLimit and make function infallible Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * CI Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Add doc Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez added 2 commits April 9, 2022 15:05

Move new_rng to shared code

4660595

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Add bechmark machine command

e3241ad

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Apr 11, 2022

ggwpez added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels Apr 11, 2022

ggwpez added 8 commits April 11, 2022 13:00

Merge remote-tracking branch 'origin/master' into oty-benchmark-machine

9301a05

Use sc-sysinfo

0e8bfa2

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Add --no-hardware-benchmarks

d4ee982

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Lockfile

b49dc41

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Do not create components if not needed

dbb1e99

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Fix tests

dc8a89f

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Revert "Add --no-hardware-benchmarks"

279789e

This reverts commit d4ee982.

Fix tests

2cb3bab

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

koute reviewed Apr 12, 2022

View reviewed changes

ggwpez added 7 commits April 12, 2022 14:47

Update Cargo deps

d625bc8

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Move sr255119::verify bench to sc-sysinfo

38d7679

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Move sr255119::verify bench to sc-sysinfo

1b9eb13

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Switch benchmarks to return f64

ea4b8fe

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Review fixes

025fbb1

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

fmt

0330f52

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Hide command until completed

74988b2

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Apr 13, 2022

Merge remote-tracking branch 'origin/master' into oty-benchmark-machine

9b53207

ggwpez added 5 commits April 14, 2022 11:59

Add test

aa89d52

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Add comment

7bc3083

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Update cargo to match polkadot

2c4ba75

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Remove doc that does not format in the console

69ab6a5

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Limit benchmark by time

b53f8f9

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez mentioned this pull request Apr 14, 2022

Co #11198: Expose benchmark machine command paritytech/polkadot#5326

Merged

koute approved these changes Apr 14, 2022

View reviewed changes

client/sysinfo/src/sysinfo.rs Outdated Show resolved Hide resolved

ggwpez mentioned this pull request Apr 14, 2022

Co #11198: Expose benchmark machine command paritytech/cumulus#1172

Merged

ggwpez added 2 commits April 14, 2022 13:28

Add ExecutionLimit and make function infallible

b478436

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

CI

14b36a5

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

kianenigma reviewed Apr 14, 2022

View reviewed changes

client/sysinfo/src/sysinfo.rs Show resolved Hide resolved

kianenigma reviewed Apr 14, 2022

View reviewed changes

kianenigma approved these changes Apr 14, 2022

View reviewed changes

Add doc

1ad85ea

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

paritytech-processbot bot merged commit 1e0807f into master Apr 15, 2022

paritytech-processbot bot deleted the oty-benchmark-machine branch April 15, 2022 12:09

xlc mentioned this pull request Apr 15, 2022

Add benchmark machine AcalaNetwork/Acala#2042

Closed

ggwpez changed the title ~~Add benchmark machine placeholder~~ Add benchmark machine Apr 19, 2022

Kailai-Wang mentioned this pull request Apr 19, 2022

Expose benchmark machine command litentry/heima#489

Closed

This was referenced Apr 20, 2022

Stand-alone benchmark machine command #11146

Closed

benchmark machine follow ups #11245

Closed

Garandor mentioned this pull request May 3, 2022

Provide easy mechanism for collator runners to benchmark their machines Manta-Network/Manta#390

Closed

yrong mentioned this pull request May 31, 2022

Bump to polkadot-v0.9.22 parallel-finance/parallel#1778

Merged

4 tasks

Dengjianping mentioned this pull request Jul 7, 2022

Bump deps to v0.9.22 Manta-Network/Manta#571

Merged

10 tasks

xlc mentioned this pull request Aug 7, 2022

host-perf-check sub command #10818

Closed

ggwpez mentioned this pull request Sep 28, 2022

Member Request polkadot-fellows/seeding#21

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `benchmark machine`#11198

Add `benchmark machine`#11198
paritytech-processbot[bot] merged 28 commits intomasterfrom
oty-benchmark-machine

ggwpez commented Apr 11, 2022 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

koute Apr 12, 2022

Uh oh!

ggwpez Apr 12, 2022

Uh oh!

Uh oh!

Uh oh!

gilescope commented Apr 14, 2022

Uh oh!

Uh oh!

kianenigma Apr 14, 2022

Uh oh!

ggwpez Apr 14, 2022

Uh oh!

koute Apr 15, 2022

Uh oh!

kianenigma left a comment

Uh oh!

kianenigma commented Apr 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ggwpez commented Apr 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

koute Apr 12, 2022

Choose a reason for hiding this comment

Uh oh!

ggwpez Apr 12, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gilescope commented Apr 14, 2022

Uh oh!

Uh oh!

kianenigma Apr 14, 2022

Choose a reason for hiding this comment

Uh oh!

ggwpez Apr 14, 2022

Choose a reason for hiding this comment

Uh oh!

koute Apr 15, 2022

Choose a reason for hiding this comment

Uh oh!

kianenigma left a comment

Choose a reason for hiding this comment

Uh oh!

kianenigma commented Apr 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ggwpez commented Apr 11, 2022 •

edited

Loading