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

Expose benchmark component ranges #11397

Closed
ggwpez opened this issue May 11, 2022 · 2 comments · Fixed by #11545
Closed

Expose benchmark component ranges #11397

ggwpez opened this issue May 11, 2022 · 2 comments · Fixed by #11545
Labels
J0-enhancement An additional feature request. U3-nice_to_have Issue is worth doing eventually. Z0-trivial Writing the issue is of the same difficulty as patching the code.

Comments

@ggwpez
Copy link
Member

ggwpez commented May 11, 2022

Problem

The range of a benchmarking component is not written to the weight file.
Any downstream software (like substrate-weight-compare) can therefore not use this information.

Context

A benchmarking component is defined in the form:

let n in 0..100;

Substrate then samples some values in this range and measures the weight at each.
The final weight file does not contain any information about the 0..100 range from above:

fn nominate(n: u32, ) -> Weight {
		(45_566_000 as Weight)
			.saturating_add((3_063_000 as Weight).saturating_mul(n as Weight))
}

Solution

One way would be to add attribute macros. Eg:

fn nominate(#[range(min=0, max=100, step=20)] n: u32, ) -> Weight {
		(45_566_000 as Weight)
			.saturating_add((3_063_000 as Weight).saturating_mul(n as Weight))
}

... or even easier with comments:

/// n: 0..100 with step size 20
fn nominate(n: u32, ) -> Weight {
		(45_566_000 as Weight)
			.saturating_add((3_063_000 as Weight).saturating_mul(n as Weight))
}

The step size of 20 in this example is picked by Substrate.

Before implementing the doc solution, it would be wise to double-check that syn can parse this.

@ggwpez ggwpez added J0-enhancement An additional feature request. U3-nice_to_have Issue is worth doing eventually. Z0-trivial Writing the issue is of the same difficulty as patching the code. labels May 11, 2022
@ggwpez
Copy link
Member Author

ggwpez commented May 26, 2022

I tested the parsing and this works fine. It only works with triple dashes or other kind of doc comments.

/// The range of component `c` is `[1_337, 2000]`.
/// The range of component `d` is `[42, 999999]`.
fn ext(c: u32, d: u32, ) -> Weight {
	(5 as Weight)
}

are you ok with the general syntax of this @shawntabrizi ?

@shawntabrizi
Copy link
Member

looks reasonable to me :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J0-enhancement An additional feature request. U3-nice_to_have Issue is worth doing eventually. Z0-trivial Writing the issue is of the same difficulty as patching the code.
Projects
Development

Successfully merging a pull request may close this issue.

2 participants