Skip to content

refactor: leader schedule generation#4662

Merged
jstarry merged 1 commit into
anza-xyz:masterfrom
jstarry:refactor/leader-schedule
Feb 6, 2025
Merged

refactor: leader schedule generation#4662
jstarry merged 1 commit into
anza-xyz:masterfrom
jstarry:refactor/leader-schedule

Conversation

@jstarry
Copy link
Copy Markdown

@jstarry jstarry commented Jan 28, 2025

Problem

Need to do a little refactoring before modifying the leader schedule in a follow up change (#4597)

Summary of Changes

Refactored leader schedule generation code so that parts can be shared with the new vote account keyed leader schedule algorithm

Fixes #

@jstarry jstarry force-pushed the refactor/leader-schedule branch 2 times, most recently from 6681859 to d2e5ead Compare February 5, 2025 12:44
@jstarry jstarry changed the title refactor: leader schedule refactor: leader schedule generation Feb 5, 2025
@jstarry jstarry force-pushed the refactor/leader-schedule branch 3 times, most recently from 778b92c to 568c040 Compare February 5, 2025 13:21
@jstarry jstarry force-pushed the refactor/leader-schedule branch from 568c040 to d19fad1 Compare February 5, 2025 13:26
Copy link
Copy Markdown

@AshwinSekar AshwinSekar left a comment

Choose a reason for hiding this comment

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

lgtm just some minor nits

// deterministic result.
// Note: Use unstable sort, because we dedup right after to remove the equal elements.
stakes.sort_unstable_by(|(l_pubkey, l_stake), (r_pubkey, r_stake)| {
if r_stake == l_stake {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: you could compare the tuples directly:

(r_stake, r_pubkey).cmp(&(l_stake, l_pubkey))

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This PR just moves this sort_stakes function from another file, I'm fine with leaving it as is


// Note: passing in zero stakers will cause a panic.
fn stake_weighted_slot_leaders(
mut keyed_stakes: Vec<(&Pubkey, u64)>,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: you could avoid the intermediate collection here by taking in an iter (flipped) Iterator<Item = (u64, &Pubkey)> and using the iter variants in sort_stakes:

stakes.sorted_unstable().dedup().reverse()

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

IterTools often allocates internally in its helper methods FYI:

fn sorted_unstable(..) {
        let mut v = Vec::from_iter(self);
        v.sort_unstable();
        v.into_iter()
}

@jstarry jstarry merged commit 2d4d7c1 into anza-xyz:master Feb 6, 2025
@jstarry jstarry deleted the refactor/leader-schedule branch February 6, 2025 02:21
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.

2 participants