Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Optimise mutations in single-pass epoch processing #4573

Merged
merged 3 commits into from
Aug 8, 2023
Merged

Optimise mutations in single-pass epoch processing #4573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
62a64b0
Optimise mutations in single-pass epoch processing
michaelsproul Aug 7, 2023
690c366
Use safer Cow::make_mut
michaelsproul Aug 7, 2023
66f0160
Update to upstream milhouse
michaelsproul Aug 8, 2023
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
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion consensus/state_processing/src/common/initiate_validator_exit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub fn initiate_validator_exit<T: EthSpec>(
return Ok(());
}

let validator = validator.to_mut();
let validator = validator.into_mut()?;
validator.mutable.exit_epoch = exit_queue_epoch;
validator.mutable.withdrawable_epoch =
exit_queue_epoch.safe_add(spec.min_validator_withdrawability_delay)?;
Expand Down
2 changes: 1 addition & 1 deletion consensus/state_processing/src/genesis.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ pub fn process_activations<T: EthSpec>(
let (validators, balances, _) = state.validators_and_balances_and_progressive_balances_mut();
let mut validators_iter = validators.iter_cow();
while let Some((index, validator)) = validators_iter.next_cow() {
let validator = validator.to_mut();
let validator = validator.into_mut()?;
let balance = balances
.get(index)
.copied()
Expand Down
2 changes: 1 addition & 1 deletion consensus/state_processing/src/per_epoch_processing/effective_balance_updates.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub fn process_effective_balance_updates<T: EthSpec>(
}

if new_effective_balance != validator.effective_balance() {
validator.to_mut().mutable.effective_balance = new_effective_balance;
validator.into_mut()?.mutable.effective_balance = new_effective_balance;
}
}

Expand Down
64 changes: 34 additions & 30 deletions consensus/state_processing/src/per_epoch_processing/single_pass.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use types::{
NUM_FLAG_INDICES, PARTICIPATION_FLAG_WEIGHTS, TIMELY_HEAD_FLAG_INDEX,
TIMELY_TARGET_FLAG_INDEX, WEIGHT_DENOMINATOR,
},
milhouse::Cow,
ActivationQueue, BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, ExitCache, ForkName,
ParticipationFlags, ProgressiveBalancesCache, Unsigned, Validator,
};
Expand Down Expand Up @@ -180,20 +181,16 @@ pub fn process_epoch_single_pass<E: EthSpec>(
previous_epoch_participation.iter(),
current_epoch_participation.iter(),
) {
let (_, validator_cow) = validators_iter
let (_, mut validator) = validators_iter
.next_cow()
.ok_or(BeaconStateError::UnknownValidator(index))?;
let (_, balance_cow) = balances_iter
let (_, mut balance) = balances_iter
.next_cow()
.ok_or(BeaconStateError::UnknownValidator(index))?;
let (_, inactivity_score_cow) = inactivity_scores_iter
let (_, mut inactivity_score) = inactivity_scores_iter
.next_cow()
.ok_or(BeaconStateError::UnknownValidator(index))?;

let validator = validator_cow.to_mut();
let balance = balance_cow.to_mut();
let inactivity_score = inactivity_score_cow.to_mut();

let is_active_current_epoch = validator.is_active_at(current_epoch);
let is_active_previous_epoch = validator.is_active_at(previous_epoch);
let is_eligible = is_active_previous_epoch
Expand Down Expand Up @@ -222,7 +219,7 @@ pub fn process_epoch_single_pass<E: EthSpec>(
// `process_inactivity_updates`
if conf.inactivity_updates {
process_single_inactivity_update(
inactivity_score,
&mut inactivity_score,
validator_info,
state_ctxt,
spec,
Expand All @@ -232,8 +229,8 @@ pub fn process_epoch_single_pass<E: EthSpec>(
// `process_rewards_and_penalties`
if conf.rewards_and_penalties {
process_single_reward_and_penalty(
balance,
inactivity_score,
&mut balance,
&inactivity_score,
validator_info,
rewards_ctxt,
state_ctxt,
Expand All @@ -245,7 +242,7 @@ pub fn process_epoch_single_pass<E: EthSpec>(
// `process_registry_updates`
if conf.registry_updates {
process_single_registry_update(
validator,
&mut validator,
validator_info,
exit_cache,
activation_queue,
Expand All @@ -257,14 +254,14 @@ pub fn process_epoch_single_pass<E: EthSpec>(

// `process_slashings`
if conf.slashings {
process_single_slashing(balance, validator, slashings_ctxt, state_ctxt, spec)?;
process_single_slashing(&mut balance, &validator, slashings_ctxt, state_ctxt, spec)?;
}

// `process_effective_balance_updates`
if conf.effective_balance_updates {
process_single_effective_balance_update(
*balance,
validator,
&mut validator,
validator_info,
&mut next_epoch_total_active_balance,
&mut next_epoch_cache,
Expand All @@ -289,7 +286,7 @@ pub fn process_epoch_single_pass<E: EthSpec>(
}

fn process_single_inactivity_update(
inactivity_score: &mut u64,
inactivity_score: &mut Cow<u64>,
validator_info: &ValidatorInfo,
state_ctxt: &StateContext,
spec: &ChainSpec,
Expand All @@ -302,25 +299,27 @@ fn process_single_inactivity_update(
if validator_info.is_unslashed_participating_index(TIMELY_TARGET_FLAG_INDEX)? {
// Avoid mutating when the inactivity score is 0 and can't go any lower -- the common
// case.
if *inactivity_score == 0 {
if **inactivity_score == 0 {
return Ok(());
}
inactivity_score.safe_sub_assign(1)?;
inactivity_score.make_mut()?.safe_sub_assign(1)?;
} else {
inactivity_score.safe_add_assign(spec.inactivity_score_bias)?;
inactivity_score
.make_mut()?
.safe_add_assign(spec.inactivity_score_bias)?;
}

// Decrease the score of all validators for forgiveness when not during a leak
if !state_ctxt.is_in_inactivity_leak {
inactivity_score
.safe_sub_assign(min(spec.inactivity_score_recovery_rate, *inactivity_score))?;
let deduction = min(spec.inactivity_score_recovery_rate, **inactivity_score);
inactivity_score.make_mut()?.safe_sub_assign(deduction)?;
}

Ok(())
}

fn process_single_reward_and_penalty(
balance: &mut u64,
balance: &mut Cow<u64>,
inactivity_score: &u64,
validator_info: &ValidatorInfo,
rewards_ctxt: &RewardsAndPenaltiesContext,
Expand Down Expand Up @@ -349,8 +348,11 @@ fn process_single_reward_and_penalty(
spec,
)?;

balance.safe_add_assign(delta.rewards)?;
*balance = balance.saturating_sub(delta.penalties);
if delta.rewards != 0 || delta.penalties != 0 {
let balance = balance.make_mut()?;
balance.safe_add_assign(delta.rewards)?;
*balance = balance.saturating_sub(delta.penalties);
}

Ok(())
}
Expand Down Expand Up @@ -449,7 +451,7 @@ impl RewardsAndPenaltiesContext {
}

fn process_single_registry_update(
validator: &mut Validator,
validator: &mut Cow<Validator>,
validator_info: &ValidatorInfo,
exit_cache: &mut ExitCache,
activation_queue: &BTreeSet<usize>,
Expand All @@ -460,7 +462,7 @@ fn process_single_registry_update(
let current_epoch = state_ctxt.current_epoch;

if validator.is_eligible_for_activation_queue(spec) {
validator.mutable.activation_eligibility_epoch = current_epoch.safe_add(1)?;
validator.make_mut()?.mutable.activation_eligibility_epoch = current_epoch.safe_add(1)?;
}

if validator.is_active_at(current_epoch)
Expand All @@ -470,7 +472,8 @@ fn process_single_registry_update(
}

if activation_queue.contains(&validator_info.index) {
validator.mutable.activation_epoch = spec.compute_activation_exit_epoch(current_epoch)?;
validator.make_mut()?.mutable.activation_epoch =
spec.compute_activation_exit_epoch(current_epoch)?;
}

// Caching: add to speculative activation queue for next epoch.
Expand All @@ -485,7 +488,7 @@ fn process_single_registry_update(
}

fn initiate_validator_exit(
validator: &mut Validator,
validator: &mut Cow<Validator>,
exit_cache: &mut ExitCache,
state_ctxt: &StateContext,
spec: &ChainSpec,
Expand All @@ -506,6 +509,7 @@ fn initiate_validator_exit(
exit_queue_epoch.safe_add_assign(1)?;
}

let validator = validator.make_mut()?;
validator.mutable.exit_epoch = exit_queue_epoch;
validator.mutable.withdrawable_epoch =
exit_queue_epoch.safe_add(spec.min_validator_withdrawability_delay)?;
Expand Down Expand Up @@ -538,7 +542,7 @@ impl SlashingsContext {
}

fn process_single_slashing(
balance: &mut u64,
balance: &mut Cow<u64>,
validator: &Validator,
slashings_ctxt: &SlashingsContext,
state_ctxt: &StateContext,
Expand All @@ -556,7 +560,7 @@ fn process_single_slashing(
.safe_div(state_ctxt.total_active_balance)?
.safe_mul(increment)?;

*balance = balance.saturating_sub(penalty);
*balance.make_mut()? = balance.saturating_sub(penalty);
}
Ok(())
}
Expand All @@ -580,7 +584,7 @@ impl EffectiveBalancesContext {
#[allow(clippy::too_many_arguments)]
fn process_single_effective_balance_update(
balance: u64,
validator: &mut Validator,
validator: &mut Cow<Validator>,
validator_info: &ValidatorInfo,
next_epoch_total_active_balance: &mut u64,
next_epoch_cache: &mut PreEpochCache,
Expand Down Expand Up @@ -610,7 +614,7 @@ fn process_single_effective_balance_update(
}

if new_effective_balance != old_effective_balance {
validator.mutable.effective_balance = new_effective_balance;
validator.make_mut()?.mutable.effective_balance = new_effective_balance;

// Update progressive balances cache for the *current* epoch, which will soon become the
// previous epoch once the epoch transition completes.
Expand Down
Loading