This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Force rebag in relevant benchmarks for Voter Bags PR (#9081) #9455
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
/benchmark runtime pallet_staking |
|
Error running benchmark: zeke-prgn-nominator-unsorted-bags-rebag-benchmark stdoutIncomplete command. |
Contributor
Author
|
/benchmark runtime pallet pallet_staking |
|
Benchmark Runtime Pallet for branch "zeke-prgn-nominator-unsorted-bags-rebag-benchmark" with command cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs Results |
Contributor
Author
|
/benchmark runtime pallet pallet_staking |
|
Benchmark Runtime Pallet for branch "zeke-prgn-nominator-unsorted-bags-rebag-benchmark" with command cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs Results |
…path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs
…ttps://github.com/paritytech/substrate into zeke-prgn-nominator-unsorted-bags-rebag-benchmark
emostov
commented
Jul 30, 2021
| string: &'static str, | ||
| n: u32, | ||
| balance_factor: crate::BalanceOf<T>, | ||
| ) -> T::AccountId { |
Contributor
Author
There was a problem hiding this comment.
I made this just so we could take BalanceOf<T> for balance_factor instead of u32, because in theory the threshold we select could overflow a u32. But it seems strange because now create_funded_user just wraps this function (look above). We could replace create_funded_user with this and just call .into on all the existing balance_factor arguments, but that would make this diff a bit more annoying (although we could do a small pr to update master with this). @kianenigma thoughts?
…unsorted-bags-rebag-benchmark
Contributor
Author
|
/benchmark runtime pallet pallet_staking |
|
Benchmark Runtime Pallet for branch "zeke-prgn-nominator-unsorted-bags-rebag-benchmark" with command cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs Results |
…path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs
kianenigma
reviewed
Aug 1, 2021
| let src2_node = Node::<T>::from_id(&src2_stash).ok_or("node not found for src stash")?; | ||
| let weight_of = Staking::<T>::weight_of_fn(); | ||
| ensure!( | ||
| !src2_node.is_misplaced(&weight_of), |
Contributor
There was a problem hiding this comment.
let's not forget that we can get rid of weight_of_fn in this API
kianenigma
reviewed
Aug 1, 2021
kianenigma
reviewed
Aug 1, 2021
kianenigma
reviewed
Aug 1, 2021
kianenigma
reviewed
Aug 1, 2021
kianenigma
reviewed
Aug 1, 2021
Comment on lines
+214
to
+215
| // - The destination bag is not empty, because then we need to update the `next` pointer | ||
| // of the previous node in addition to the work we do otherwise. |
Contributor
There was a problem hiding this comment.
Suggested change
| // - The destination bag is not empty, because then we need to update the `next` pointer | |
| // of the previous node in addition to the work we do otherwise. | |
| // - The destination must also be such that the new node is inserted in between two other nodes (say, `A` and `B`), so that the `next` of `A` and `prev` of `B` is updated. |
is this what you mean?
Contributor
Author
There was a problem hiding this comment.
no - whenever we insert a node into a bag we always insert it as a tail, so we only update the next of the "old" tail to point at the node being inserted.
kianenigma
reviewed
Aug 1, 2021
kianenigma
reviewed
Aug 1, 2021
Contributor
kianenigma
left a comment
kianenigma
left a comment
There was a problem hiding this comment.
While we've learned a good amount from the outcome here, I think we should freeze this and first move the bags stuff into its own pallet. I've discussed it also with @shawntabrizi and we're both quite positive on the idea.
We can put on ice and pick this up again once the transition is done.
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
This was referenced Aug 1, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
target PR: #9081
target branch: prgn-nominator-unsorted-bags