-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Force rebag in relevant benchmarks for Voter Bags PR (#9081) #9455
Force rebag in relevant benchmarks for Voter Bags PR (#9081) #9455
Conversation
/benchmark runtime pallet_staking |
Error running benchmark: zeke-prgn-nominator-unsorted-bags-rebag-benchmark stdoutIncomplete command. |
/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
|
/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
string: &'static str, | ||
n: u32, | ||
balance_factor: crate::BalanceOf<T>, | ||
) -> T::AccountId { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
/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
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), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's not forget that we can get rid of weight_of_fn in this API
// - 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// - 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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
target PR: #9081
target branch: prgn-nominator-unsorted-bags