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

Apply patch for a parachains inclusion pallet benchmarks #567

Conversation

bkontur
Copy link
Contributor

@bkontur bkontur commented Jan 29, 2025

Description

Addressing @ordian's comment:

Previously, the weight of enact_candidate was included in the weight of availability bitfields, but this was a bug, because the number of bitfields depends on number of validators, whereas number of enacted candidates depends on the number of cores. This was later (accidentally) fixed, but the enact_candidate weight was unaccounted for. paritytech/polkadot-sdk#5270 was supposed to fix that.

The base weight of enact_candidate still looks excessive and it was fixed in paritytech/polkadot-sdk#5526, but looks like it didn't make into the same release.

That being said, even with 1.5ms of enactment weight per candidate, this shouldn't be a blocker to scale to 200 parachains weight-wise. So I'm fine with having these excessive weights for now.

Addressing @bkontur's comment:

@ordian The next patch, stable2409-4, is planned for this week. I have backported your fix here: paritytech/polkadot-sdk#7145. Once it is released, I will re-run the benchmarks for this within another PR.

Weights diff

Kusama

--method asymptotic does not work:

subweight compare commits          --path-pattern "./relay/kusama/**/weights/**/*.rs"          --format markdown --no-color           --change added changed          --method asymptotic --ignore-errors --strip-path-prefix="relay/kusama/src/weights/"          remotes/polkadot-fellows/main          origin/bko-patch-parachains-inclusion
File Extrinsic Old New Change [%]
runtime_parachains_inclusion.rs enact_candidate - - ERROR

but --method base works:

subweight compare commits          --path-pattern "./relay/kusama/**/weights/**/*.rs"          --format markdown --no-color           --change added changed          --method base --ignore-errors --strip-path-prefix="relay/kusama/src/weights/"          remotes/polkadot-fellows/main          origin/bko-patch-parachains-inclusion
File Extrinsic Old New Change [%]
runtime_parachains_inclusion.rs enact_candidate 2.97ms 939.10us -68.35

Polkadot

--method asymptotic does not work:

subweight compare commits          --path-pattern "./relay/polkadot/**/weights/**/*.rs"          --format markdown --no-color           --change added changed          --method asymptotic --ignore-errors --strip-path-prefix="relay/polkadot/src/weights/"          remotes/polkadot-fellows/main          origin/bko-patch-parachains-inclusion
File Extrinsic Old New Change [%]
runtime_parachains_inclusion.rs enact_candidate - - ERROR

but --method base works:

subweight compare commits          --path-pattern "./relay/polkadot/**/weights/**/*.rs"          --format markdown --no-color           --change added changed          --method base --ignore-errors --strip-path-prefix="relay/polkadot/src/weights/"          remotes/polkadot-fellows/main          origin/bko-patch-parachains-inclusion
File Extrinsic Old New Change [%]
runtime_parachains_inclusion.rs enact_candidate 2.92ms 974.24us -66.60
  • Does not require a CHANGELOG entry

@bkontur bkontur mentioned this pull request Jan 30, 2025
6 tasks
@acatangiu
Copy link
Contributor

/merge

@fellowship-merge-bot fellowship-merge-bot bot enabled auto-merge (squash) January 30, 2025 12:55
@fellowship-merge-bot
Copy link
Contributor

Enabled auto-merge in Pull Request

Available commands
  • /merge: Enables auto-merge for Pull Request
  • /merge cancel: Cancels auto-merge for Pull Request
  • /merge help: Shows this menu

For more information see the documentation

@fellowship-merge-bot fellowship-merge-bot bot merged commit ffc67cb into polkadot-fellows:main Jan 30, 2025
47 of 48 checks passed
@bkontur bkontur deleted the bko-patch-parachains-inclusion branch January 31, 2025 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants