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

Improve Benchmarks for Pallet Election Provider Multiphase #446

Open
shawntabrizi opened this issue Nov 8, 2022 · 3 comments
Open

Improve Benchmarks for Pallet Election Provider Multiphase #446

shawntabrizi opened this issue Nov 8, 2022 · 3 comments
Assignees

Comments

@shawntabrizi
Copy link
Member

shawntabrizi commented Nov 8, 2022

There are still a few benches which previously incorrectly generated with zero weights and/or now use the minimum times as the base weight and have one of their components start at a high number (mainly from pallet_election_provider_multi_phase), and I'd still like to do something about them (which is not trivial because the components' ranges cannot overlap for those benchmarks to correctly execute so you can't naively bring them down to zero), but looking at the numbers the minimums were bumped by only like 50%~70% so it's not the end of the world.

Originally posted by @koute in paritytech/substrate#12325 (review)

@koute
Copy link
Contributor

koute commented Nov 9, 2022

One way of fixing this would be to add a new mechanism to the benchmarking machinery to introduce simple greater-than/less-than constraints between the components.

For example, the benchmarks use these constants:

const VOTERS: [u32; 2] = [1000, 2000];
const ACTIVE_VOTERS: [u32; 2] = [500, 800];

Obviously ACTIVE_VOTERS cannot be bigger than the total number of VOTERS.

In one of the benchmarks we have this:

let a in (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1];
let v = T::BenchmarkingConfig::VOTERS[1];

So we could add something like this to constraint these (imaginary syntax):

let a in (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1] where a <= v;

And then we could extend the bounds of the benchmarks, and the benchmarking machinery would only call the benchmark only in cases where a <= v. (It'd still loop through the full range, but at each combination of parameters it'll check whether those fulfill the constraints and chose not to run them.)

Not sure if we actually want to do this, but it is a possibility.

@ggwpez
Copy link
Member

ggwpez commented Nov 9, 2022

Somehow the FEPS weights were not updated in the last MR paritytech/substrate#12325 election-provider-support. Did you use the script or do it manually?

@kianenigma kianenigma assigned gpestana and unassigned kianenigma Nov 20, 2022
@kianenigma
Copy link
Contributor

@gpestana we can also now look into the cost of verifying an un-reduced solution.

@juangirini juangirini transferred this issue from paritytech/substrate Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ⌛️ Sometime-soon
Development

No branches or pull requests

5 participants