refactor(trimming): implement pre-trimming #687

niklasad1 · 2023-09-20T16:19:55Z

This commit refactors trimming such that the staking-miner is responsible to ensure that trimming doesn't have to be done be on-chain.

The pre-trimming is implemented as follows:

Weight trimming: Initially the voters are iteratively removed until Weight < MaxWeight by using MinerConfig::solution_weight for this part a "new solution" doesn't need to be "mined" and is much more efficient than part 2 and only has to be performed once.
Length Trimming: Instead of removing one voter at the time and then “mine a new solution” as the initial implementation did. Now the implementation removes the number of voters in a binary search manner to find "the biggest set of voters" where no trimming occurs such that it doesn't get trimmed.

By “binary search manner” I mean:

let  mut  l  =  1;
let  mut  h  =  voters.len();
let  mut  best_solution  =  None;

while  l  <=  h {
   let  mid  = ((h  -  l) /  2) +  l;
   let next_state = voters.trim(mid)?;
   let solution = mine_solution(next_state);
	 
   if !solution.is_trimmed() {
     best_solution  =  Some((solution, score, solution_or_snapshot_size));
     h  =  mid  -  1;
   } else {
     l  =  mid  +  1;
   }
}

This makes things much faster than the iterative approach I implemented first

This PR also adds a prometheous metric to count the number of "pre-trimming length ops"

This commit refactors trimming such that the staking-miner is responsible to ensure that trimming doesn't have to be done be on-chain. Instead the staking-miner iteratively removes the votes by one voter at the time and re-computes the solution. Once a solution with no trimming is found that solution is submitted to the chain

niklasad1 · 2023-09-21T13:01:02Z

staking-miner-playground/node/src/chain_spec.rs

-	static ref NOMINATORS: u32 = std::env::var("N").unwrap_or("1000".to_string()).parse().unwrap();
-	static ref CANDIDATES: u32 = std::env::var("C").unwrap_or("500".to_string()).parse().unwrap();
-	static ref VALIDATORS: u32 = std::env::var("V").unwrap_or("100".to_string()).parse().unwrap();
-	static ref NOMINATION_DEGREE: NominationDegree = NominationDegree::from_str(std::env::var("ND").unwrap_or("full".to_string()).as_ref()).unwrap();


I removed this nomination degree because I don't want have randomness in how many validators a nominator supports and I don't understand partial and full

My assumption was that full and partial would be the other way around but I understand why it was added.

niklasad1 · 2023-09-21T15:46:17Z

staking-miner-playground/runtime/src/lib.rs

@@ -343,7 +332,7 @@ impl<const PERIOD: BlockNumber> ShouldEndSession<BlockNumber>
 	}
 }

-const SESSION: BlockNumber = 1 * MINUTES;
+const SESSION: BlockNumber = 3 * MINUTES;


Because the trimming is slower with "pre-trimming", I increased the SignPhase a bit...

niklasad1 · 2023-09-21T15:47:00Z

staking-miner-playground/runtime/src/lib.rs

-		Perbill::from_rational(8u32, 10) * <Runtime as frame_system::Config>::BlockWeights::get().get(DispatchClass::Normal).max_total.unwrap(),
-		WeightTrimming::get()
-	);
+	pub MinerMaxLength: u32 = Perbill::from_percent(60) * *(<<Runtime as frame_system::Config>::BlockLength as Get<limits::BlockLength>>::get()).max.get(DispatchClass::Normal);


modified these values to ensure trimming always occurs when we run it.

Cargo.toml

gpestana

Added a few comments that could optimise the number of trimming round before reaching the appropriate solution weight/length, but I think that should not be a blocker for this PR if it's too much work.

Thanks for implementing this!

src/epm.rs

gpestana · 2023-09-26T09:09:59Z

src/epm.rs

+		Self { voters, voters_by_stake }
+	}
+
+	/// Trim the next voter with the least amount of stake.


Have we tried to trim more than one voter in each iteration? This may reduce the number of iterations required to finalise the trimming. Maybe we could have an heuristic where depending on how much the solution exceeds the limit, trim 1 or more voters in one go.

This probably should not block this PR, but perhaps addressed in a subsequent PR if helpful.

We could also calculate the difference between the weight_trimmed/length_trimmed calculated in prepare_election_result_with_snapshot and use that as an heuristic of how much trimming will be required. Then we can use that to request the trimming of N voters in one go, if there's still a long way to trim.

That's a good point, I think we should merge this PR and create a issue for your idea.

If there's much trimming to do then yeah this would be very slow as it does it one-by-one.

src/epm.rs

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

niklasad1 · 2023-09-29T20:53:43Z

src/epm.rs

+			let desired_targets = desired_targets;
+			let targets = targets.len() as u32;
+
+			let est_weight: Weight = tokio::task::spawn_blocking(move || {


decided to add this in the constructor/new because it should only be needed to do once

niklasad1 · 2023-09-29T20:55:11Z

src/epm.rs

+	let mut h = voters.len();
+	let mut best_solution = None;
+
+	while l <= h {


binary search to find the best "pre-trim"

niklasad1 added 4 commits September 20, 2023 18:13

cargo fmt

729fa5a

cargo fmt

130d558

update to polkadot-sdk

3b7e090

niklasad1 commented Sep 21, 2023

View reviewed changes

cleanup the pre-trimming code

4987cbc

niklasad1 commented Sep 21, 2023

View reviewed changes

niklasad1 changed the title ~~WIP refactor(trimming): implement pre-trimming~~ refactor(trimming): implement pre-trimming Sep 21, 2023

remove checking solution score

57513ef

niklasad1 marked this pull request as ready for review September 22, 2023 12:56

niklasad1 commented Sep 22, 2023

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

niklasad1 added 2 commits September 25, 2023 10:14

update polkadot-sdk

e56c755

cleaup

235c42c

niklasad1 requested review from gpestana and kianenigma September 25, 2023 09:54

gpestana approved these changes Sep 26, 2023

View reviewed changes

niklasad1 and others added 6 commits September 26, 2023 12:02

Update src/epm.rs

871593a

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

add promethous counter for trimming

1130dfc

Merge remote-tracking branch 'origin/main' into na-refactor-trimming

274e7f2

cargo fmt

81608fb

fix nits

3fd404d

pre-trimming: impl bsearch to find pre-trim

3322085

niklasad1 commented Sep 29, 2023

View reviewed changes

src/epm.rs

let mut h = voters.len();

let mut best_solution = None;

while l <= h {

Copy link

Member Author

niklasad1 Sep 29, 2023

Choose a reason for hiding this comment

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

binary search to find the best "pre-trim"

fix nit in weight trimmming

aee2163

niklasad1 merged commit 2ea305e into main Nov 13, 2023
8 checks passed

niklasad1 deleted the na-refactor-trimming branch November 13, 2023 13:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(trimming): implement pre-trimming #687

refactor(trimming): implement pre-trimming #687

niklasad1 commented Sep 20, 2023 •

edited

Loading

niklasad1 Sep 21, 2023 •

edited

Loading

niklasad1 Sep 21, 2023

niklasad1 Sep 21, 2023

gpestana left a comment

gpestana Sep 26, 2023

gpestana Sep 26, 2023

niklasad1 Sep 26, 2023 •

edited

Loading

niklasad1 Sep 29, 2023

niklasad1 Sep 29, 2023

refactor(trimming): implement pre-trimming #687

refactor(trimming): implement pre-trimming #687

Conversation

niklasad1 commented Sep 20, 2023 • edited Loading

niklasad1 Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

niklasad1 Sep 21, 2023

Choose a reason for hiding this comment

niklasad1 Sep 21, 2023

Choose a reason for hiding this comment

gpestana left a comment

Choose a reason for hiding this comment

gpestana Sep 26, 2023

Choose a reason for hiding this comment

gpestana Sep 26, 2023

Choose a reason for hiding this comment

niklasad1 Sep 26, 2023 • edited Loading

Choose a reason for hiding this comment

niklasad1 Sep 29, 2023

Choose a reason for hiding this comment

niklasad1 Sep 29, 2023

Choose a reason for hiding this comment

niklasad1 commented Sep 20, 2023 •

edited

Loading

niklasad1 Sep 21, 2023 •

edited

Loading

niklasad1 Sep 26, 2023 •

edited

Loading