Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Force bag changes in relevant benchmarks (targets #9507) #9529

Merged
merged 46 commits into from
Aug 28, 2021
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
55cf648
force rebag for unbond, rebond, and bond_extra
emostov Aug 10, 2021
ff55aba
nit
emostov Aug 10, 2021
759e309
Merge branch 'zeke-voter-bags-module' into zeke-bags-list-staking-ben…
emostov Aug 10, 2021
37c8436
Improve utils
emostov Aug 10, 2021
f3b8c8a
fmt
emostov Aug 10, 2021
a96ce8e
Try merge origin master
emostov Aug 10, 2021
7b6686a
nits
emostov Aug 10, 2021
1a91ead
Merge branch 'zeke-voter-bags-module' into zeke-bags-list-staking-ben…
emostov Aug 10, 2021
804151c
Move generate_bags to its own pallet
emostov Aug 10, 2021
34e2194
Get runtime-benchmarks feature setup with prepare_on_update_benchmark
emostov Aug 10, 2021
dec1e8b
Withdraw unbonded kill working
emostov Aug 12, 2021
a354364
Nominate bench working
emostov Aug 13, 2021
02a218f
some cleanup
emostov Aug 13, 2021
77ab8af
WIP
emostov Aug 13, 2021
da38ccc
Try merge zeke-voter-bags-module
emostov Aug 13, 2021
ecdb7e8
update to check head pre & post conditions
emostov Aug 13, 2021
d3bbd18
Add some post condition verification stuff for on_remove
emostov Aug 13, 2021
1b82579
Update nominate
emostov Aug 13, 2021
19ebb8a
fmt
emostov Aug 13, 2021
e79ad98
Improvements
emostov Aug 13, 2021
cd3a63c
Fix build
emostov Aug 14, 2021
7d18281
fix build with polkadot companion
emostov Aug 14, 2021
644e3bf
Update frame/bags-list/src/list/tests.rs
emostov Aug 16, 2021
a2ddcb4
Try merge feature
emostov Aug 20, 2021
ded3b49
move generate-bag from frame to utils
emostov Aug 20, 2021
624cfe9
wip
emostov Aug 24, 2021
d34a1fd
Merge remote-tracking branch 'origin' into zeke-bags-list-staking-ben…
emostov Aug 24, 2021
d1f5d44
refactor WIP
emostov Aug 24, 2021
76688e7
Merge branch 'zeke-voter-bags-module' of github.com:paritytech/substr…
kianenigma Aug 24, 2021
dbb95db
WIP save
emostov Aug 24, 2021
0cd7103
Refactor working
emostov Aug 24, 2021
aea5554
some variable renaming
emostov Aug 24, 2021
033b717
WIP: prepare to remove head checks
emostov Aug 24, 2021
d08d858
Finish MvP refactor
emostov Aug 24, 2021
d5601ad
Merge branch 'zeke-voter-bags-module' into zeke-bags-list-staking-ben…
emostov Aug 24, 2021
8cacd0c
Some cleanup
emostov Aug 24, 2021
5aa980c
Soem more cleanup
emostov Aug 24, 2021
4f56f74
Merge branch 'zeke-bags-list-staking-benchmarks' of https://github.co…
emostov Aug 24, 2021
efbf22d
save
emostov Aug 26, 2021
532ffff
fix a lot of stuff
kianenigma Aug 26, 2021
24c2767
Update client/db/src/bench.rs
emostov Aug 26, 2021
6b91371
Apply suggestions from code review
emostov Aug 26, 2021
0317ea1
Apply suggestions from code review
emostov Aug 26, 2021
4faf565
Fix some issues that came from trying to merge comments on github
emostov Aug 27, 2021
41a50db
some small changes
emostov Aug 27, 2021
6277454
simplify it
kianenigma Aug 27, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
162 changes: 85 additions & 77 deletions Cargo.lock

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ members = [
"frame/utility",
"frame/vesting",
"frame/bags-list",
"frame/bags-list/generate-bags",
"primitives/api",
"primitives/api/proc-macro",
"primitives/api/test",
Expand Down Expand Up @@ -201,6 +200,8 @@ members = [
"utils/frame/try-runtime/cli",
"utils/frame/rpc/support",
"utils/frame/rpc/system",
"utils/frame/generate-bags",
"utils/frame/generate-bags/node-runtime",
"utils/prometheus",
"utils/wasm-builder",
]
Expand Down
2 changes: 1 addition & 1 deletion client/db/src/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ impl<B: BlockT> StateBackend<HashFor<B>> for BenchmarkingState<B> {

if tracker.writes > 0 {
writes += 1;
repeat_writes += tracker.reads - 1;
Copy link
Member

Choose a reason for hiding this comment

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

this looks like a bug. should be tracker.writes - 1, which should be okay

repeat_writes += tracker.reads.saturating_sub(1);
emostov marked this conversation as resolved.
Show resolved Hide resolved
}
}
});
Expand Down
15 changes: 2 additions & 13 deletions frame/bags-list/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,6 @@ sp-core = { version = "4.0.0-dev", path = "../../primitives/core", optional = tr
sp-io = { version = "4.0.0-dev", path = "../../primitives/io", optional = true, default-features = false }
sp-tracing = { version = "4.0.0-dev", path = "../../primitives/tracing", optional = true, default-features = false }

# optional imports for making voter bags lists
chrono = { version = "0.4.19", optional = true }
git2 = { version = "0.13.20", default-features = false, optional = true }
num-format = { version = "0.4.0", optional = true }
pallet-staking = { version = "4.0.0-dev", path = "../staking", optional = true }

[dev-dependencies]
sp-core = { version = "4.0.0-dev", path = "../../primitives/core"}
sp-io = { version = "4.0.0-dev", path = "../../primitives/io"}
Expand All @@ -66,11 +60,6 @@ runtime-benchmarks = [
"sp-io",
"pallet-balances",
"sp-tracing",
"frame-election-provider-support/runtime-benchmarks",
]
generate-bags = [
"chrono",
"git2",
"num-format",
"std",
"pallet-staking"
]

31 changes: 29 additions & 2 deletions frame/bags-list/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ use sp_std::prelude::*;
#[cfg(any(feature = "runtime-benchmarks", test))]
mod benchmarks;

#[cfg(feature = "generate-bags")]
pub mod generate_bags;
mod list;
#[cfg(test)]
mod mock;
Expand Down Expand Up @@ -267,4 +265,33 @@ impl<T: Config> SortedListProvider<T::AccountId> for Pallet<T> {
fn clear() {
List::<T>::clear()
}

#[cfg(feature = "runtime-benchmarks")]
fn is_in_pos(id: &T::AccountId, weight: VoteWeight, _: bool) -> bool {
list::Bag::<T>::get(list::notional_bag_for::<T>(weight)).unwrap().contains(id)
}

#[cfg(feature = "runtime-benchmarks")]
fn weight_update_worst_case(who: &T::AccountId, is_increase: bool) -> VoteWeight {
use frame_support::traits::Get as _;
let thresholds = T::BagThresholds::get();
let node = list::Node::<T>::get(who).unwrap();
let current_bag_idx = thresholds
.iter()
.chain(sp_std::iter::once(&VoteWeight::MAX))
.position(|w| w == &node.bag_upper)
.unwrap();

if is_increase {
let next_threshold_idx = current_bag_idx + 1;
log!(info, "next_threshold_idx {:#?}", next_threshold_idx);
assert!(thresholds.len() > next_threshold_idx);
thresholds[next_threshold_idx]
} else {
log!(info, "node bag upper {:#?}", node.bag_upper);
assert!(current_bag_idx != 0);
let prev_threshold_idx = current_bag_idx - 1;
thresholds[prev_threshold_idx]
}
}
}
9 changes: 7 additions & 2 deletions frame/bags-list/src/list/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ mod tests;
///
/// Note that even if the thresholds list does not have `VoteWeight::MAX` as its final member, this
/// function behaves as if it does.
fn notional_bag_for<T: Config>(weight: VoteWeight) -> VoteWeight {
pub(crate) fn notional_bag_for<T: Config>(weight: VoteWeight) -> VoteWeight {
let thresholds = T::BagThresholds::get();
let idx = thresholds.partition_point(|&threshold| weight > threshold);
thresholds.get(idx).copied().unwrap_or(VoteWeight::MAX)
Expand Down Expand Up @@ -586,6 +586,11 @@ impl<T: Config> Bag<T> {

Ok(())
}

#[cfg(feature = "runtime-benchmarks")]
pub(crate) fn contains(&self, id: &T::AccountId) -> bool {
self.iter().any(|n| n.id() == id)
}
}

/// A Node is the fundamental element comprising the doubly-linked list described by `Bag`.
Expand All @@ -596,7 +601,7 @@ pub struct Node<T: Config> {
id: T::AccountId,
prev: Option<T::AccountId>,
next: Option<T::AccountId>,
bag_upper: VoteWeight,
pub(crate) bag_upper: VoteWeight,
}

impl<T: Config> Node<T> {
Expand Down
4 changes: 2 additions & 2 deletions frame/bags-list/src/list/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ fn migrate_works() {
(15, vec![710]), // nodes in range 11 ..= 15 move from bag 20 to bag 15
(20, vec![711]),
(1000, vec![2, 3, 4]),
(10_000, vec![712]), /* nodes in range 1_001 ..= 2_000 move from bag 2_000
* to bag 10_000 */
// nodes in range 1_001 ..= 2_000 move from bag 2_000 to bag 10_000
(10_000, vec![712]),
]
);
});
Expand Down
13 changes: 13 additions & 0 deletions frame/election-provider-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,19 @@ pub trait SortedListProvider<AccountId> {
fn clear();
/// Sanity check internal state of list. Only meant for debug compilation.
fn sanity_check() -> Result<(), &'static str>;

/// Whether or not `id` is in a position in the list that corresponds to `weight`.
#[cfg(feature = "runtime-benchmarks")]
fn is_in_pos(_id: &AccountId, _weight: VoteWeight, mock: bool) -> bool {
mock
}
Copy link
Member

Choose a reason for hiding this comment

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

this looks very strange to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agreed; the default impl here assumes this is not actually relevant to the particular list provider impl and instead returns the mock value. The mock value should always just be the expected result. I would like something better, but not sure how


/// If `who` changes by the returned amount they are guaranteed to have a worst case change
/// in their list position.
#[cfg(feature = "runtime-benchmarks")]
fn weight_update_worst_case(_who: &AccountId, _is_increase: bool) -> VoteWeight {
VoteWeight::MAX
}
}

/// Something that can provide the `VoteWeight` of an account. Similar to [`ElectionProvider`] and
Expand Down
2 changes: 1 addition & 1 deletion frame/session/benchmarking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ fn check_membership_proof_setup<T: Config>(
pallet_staking::ValidatorCount::<T>::put(n);

// create validators and set random session keys
for (n, who) in create_validators::<T>(n, 1000).unwrap().into_iter().enumerate() {
for (n, who) in create_validators::<T>(n, 1000, 0).unwrap().into_iter().enumerate() {
use rand::{RngCore, SeedableRng};

let validator = T::Lookup::lookup(who).unwrap();
Expand Down
18 changes: 1 addition & 17 deletions frame/staking/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,14 @@ log = { version = "0.4.14", default-features = false }
frame-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../benchmarking", optional = true }
rand_chacha = { version = "0.2", default-features = false, optional = true }

# Optional imports for making voter bags lists
chrono = { version = "0.4.19", optional = true }
git2 = { version = "0.13.20", default-features = false, optional = true }
num-format = { version = "0.4.0", optional = true }

[dev-dependencies]
sp-tracing = { version = "4.0.0-dev", path = "../../primitives/tracing" }
sp-core = { version = "4.0.0-dev", path = "../../primitives/core" }
sp-npos-elections = { version = "4.0.0-dev", path = "../../primitives/npos-elections" }
pallet-balances = { version = "4.0.0-dev", path = "../balances" }
pallet-timestamp = { version = "4.0.0-dev", path = "../timestamp" }
pallet-staking-reward-curve = { version = "4.0.0-dev", path = "../staking/reward-curve" }
pallet-bags-list = { version = "4.0.0-dev", path = "../bags-list" }
pallet-bags-list = { version = "4.0.0-dev", features = ["runtime-benchmarks"], path = "../bags-list" }
substrate-test-utils = { version = "4.0.0-dev", path = "../../test-utils" }
frame-benchmarking = { version = "4.0.0-dev", path = "../benchmarking" }
frame-election-provider-support = { version = "4.0.0-dev", path = "../election-provider-support" }
Expand Down Expand Up @@ -81,14 +76,3 @@ runtime-benchmarks = [
"rand_chacha",
]
try-runtime = ["frame-support/try-runtime"]
generate-bags = [
"chrono",
"git2",
"num-format",
"pallet-staking-reward-curve",
"pallet-balances",
"pallet-timestamp",
"sp-core",
"sp-tracing",
"std",
]
Loading