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

[subsystem-benchmark] Add approval-voting benchmark to CI #4216

Merged
merged 8 commits into from
Apr 22, 2024

Conversation

AndreiEres
Copy link
Contributor

No description provided.

@AndreiEres AndreiEres requested a review from a team as a code owner April 19, 2024 13:41
@AndreiEres AndreiEres requested review from alexggh and removed request for a team April 19, 2024 13:41
@paritytech-review-bot paritytech-review-bot bot requested a review from a team April 19, 2024 13:42
@paritytech-review-bot paritytech-review-bot bot requested a review from a team April 19, 2024 14:13
@AndreiEres AndreiEres requested a review from sandreim April 19, 2024 16:12
@AndreiEres AndreiEres added R0-silent Changes should not be mentioned in any release notes T12-benchmarks This PR/Issue is related to benchmarking and weights. labels Apr 19, 2024
Copy link
Contributor

@sandreim sandreim left a comment

Choose a reason for hiding this comment

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

Before merging we should be very confident it's not flaky.

// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

// TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any TODO left ?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, it shouldn't be.

// TODO
//! approval-voting throughput test
//!
//! Approval Voting benchmark based on Kusama parameters and scale.
Copy link
Contributor

Choose a reason for hiding this comment

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

These are future parameters, we're not there yet, but hopefully we'll be there soon.

let mut messages = vec![];
let mut config = TestConfiguration::default();
// A single node effort roughly
config.n_cores = 100;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we approving 100 candidates per block ? If so, then the comment above is not correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this would generate assignments and approvals for 100 occupied core each block

@AndreiEres AndreiEres added this pull request to the merge queue Apr 22, 2024
Merged via the queue into master with commit a2a049d Apr 22, 2024
137 of 138 checks passed
@AndreiEres AndreiEres deleted the AndreiEres/approval-votings-benches-ci branch April 22, 2024 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T12-benchmarks This PR/Issue is related to benchmarking and weights.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants