Avoid temp allocations with slog (#2183)

## Issue Addressed

Which issue # does this PR address?

## Proposed Changes

Replaces use of `format!` in `slog` logging with it's special no-allocation `?` and `%` shortcuts. According to a `heaptrack` analysis today over about a period of an hour, this will reduce temporary allocations by at least 4%.

## Additional Info

NA
This commit is contained in:
Paul Hauner 2021-02-04 07:31:47 +00:00
parent ff35fbb121
commit e383ef3e91

View File

@ -676,7 +676,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
warn!( warn!(
self.log, self.log,
"Unable to load state at slot"; "Unable to load state at slot";
"error" => format!("{:?}", e), "error" => ?e,
"head_slot" => head_state_slot, "head_slot" => head_state_slot,
"requested_slot" => slot "requested_slot" => slot
); );
@ -1023,7 +1023,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
Ok(outcome) => trace!( Ok(outcome) => trace!(
self.log, self.log,
"Stored unaggregated attestation"; "Stored unaggregated attestation";
"outcome" => format!("{:?}", outcome), "outcome" => ?outcome,
"index" => attestation.data.index, "index" => attestation.data.index,
"slot" => attestation.data.slot.as_u64(), "slot" => attestation.data.slot.as_u64(),
), ),
@ -1042,7 +1042,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
error!( error!(
self.log, self.log,
"Failed to store unaggregated attestation"; "Failed to store unaggregated attestation";
"error" => format!("{:?}", e), "error" => ?e,
"index" => attestation.data.index, "index" => attestation.data.index,
"slot" => attestation.data.slot.as_u64(), "slot" => attestation.data.slot.as_u64(),
); );
@ -1125,7 +1125,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
&self.log, &self.log,
"Missing pivot block root for attestation"; "Missing pivot block root for attestation";
"slot" => pivot_slot, "slot" => pivot_slot,
"error" => format!("{:?}", e), "error" => ?e,
); );
return false; return false;
} }
@ -1151,7 +1151,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
&self.log, &self.log,
"Discarding attestation because of missing ancestor"; "Discarding attestation because of missing ancestor";
"pivot_slot" => pivot_slot.as_u64(), "pivot_slot" => pivot_slot.as_u64(),
"block_root" => format!("{:?}", block_root), "block_root" => ?block_root,
); );
false false
} }
@ -1399,7 +1399,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
"Successfully processed gossip block"; "Successfully processed gossip block";
"graffiti" => graffiti_string, "graffiti" => graffiti_string,
"slot" => slot, "slot" => slot,
"root" => format!("{:?}", verified.block_root()), "root" => ?verified.block_root(),
); );
Ok(verified) Ok(verified)
@ -1456,8 +1456,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
trace!( trace!(
self.log, self.log,
"Beacon block imported"; "Beacon block imported";
"block_root" => format!("{:?}", block_root), "block_root" => ?block_root,
"block_slot" => format!("{:?}", block.slot().as_u64()), "block_slot" => %block.slot(),
); );
// Increment the Prometheus counter for block processing successes. // Increment the Prometheus counter for block processing successes.
@ -1471,7 +1471,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
crit!( crit!(
self.log, self.log,
"Beacon block processing error"; "Beacon block processing error";
"error" => format!("{:?}", e), "error" => ?e,
); );
Err(BlockError::BeaconChainError(e)) Err(BlockError::BeaconChainError(e))
} }
@ -1591,12 +1591,12 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
crit!( crit!(
self.log, self.log,
"Weak subjectivity checkpoint verification failed while importing block!"; "Weak subjectivity checkpoint verification failed while importing block!";
"block_root" => format!("{:?}", block_root), "block_root" => ?block_root,
"parent_root" => format!("{:?}", block.parent_root), "parent_root" => ?block.parent_root,
"old_finalized_epoch" => format!("{:?}", old_finalized_checkpoint.epoch), "old_finalized_epoch" => ?old_finalized_checkpoint.epoch,
"new_finalized_epoch" => format!("{:?}", new_finalized_checkpoint.epoch), "new_finalized_epoch" => ?new_finalized_checkpoint.epoch,
"weak_subjectivity_epoch" => format!("{:?}", wss_checkpoint.epoch), "weak_subjectivity_epoch" => ?wss_checkpoint.epoch,
"error" => format!("{:?}", e), "error" => ?e,
); );
crit!(self.log, "You must use the `--purge-db` flag to clear the database and restart sync. You may be on a hostile network."); crit!(self.log, "You must use the `--purge-db` flag to clear the database and restart sync. You may be on a hostile network.");
shutdown_sender.try_send("Weak subjectivity checkpoint verification failed. Provided block root is not a checkpoint.") shutdown_sender.try_send("Weak subjectivity checkpoint verification failed. Provided block root is not a checkpoint.")
@ -1899,7 +1899,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
error!( error!(
self.log, self.log,
"Attestation did not transfer to op pool"; "Attestation did not transfer to op pool";
"reason" => format!("{:?}", e) "reason" => ?e
); );
} }
} }
@ -1962,7 +1962,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
trace!( trace!(
self.log, self.log,
"Produced beacon block"; "Produced beacon block";
"parent" => format!("{}", block.message.parent_root), "parent" => %block.message.parent_root,
"attestations" => block.message.body.attestations.len(), "attestations" => block.message.body.attestations.len(),
"slot" => block.message.slot "slot" => block.message.slot
); );
@ -2052,21 +2052,21 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
warn!( warn!(
self.log, self.log,
"Beacon chain re-org"; "Beacon chain re-org";
"previous_head" => format!("{}", current_head.block_root), "previous_head" => %current_head.block_root,
"previous_slot" => current_head.slot, "previous_slot" => current_head.slot,
"new_head_parent" => format!("{}", new_head.beacon_block.parent_root()), "new_head_parent" => %new_head.beacon_block.parent_root(),
"new_head" => format!("{}", beacon_block_root), "new_head" => %beacon_block_root,
"new_slot" => new_head.beacon_block.slot(), "new_slot" => new_head.beacon_block.slot(),
); );
} else { } else {
debug!( debug!(
self.log, self.log,
"Head beacon block"; "Head beacon block";
"justified_root" => format!("{}", new_head.beacon_state.current_justified_checkpoint.root), "justified_root" => %new_head.beacon_state.current_justified_checkpoint.root,
"justified_epoch" => new_head.beacon_state.current_justified_checkpoint.epoch, "justified_epoch" => new_head.beacon_state.current_justified_checkpoint.epoch,
"finalized_root" => format!("{}", new_head.beacon_state.finalized_checkpoint.root), "finalized_root" => %new_head.beacon_state.finalized_checkpoint.root,
"finalized_epoch" => new_head.beacon_state.finalized_checkpoint.epoch, "finalized_epoch" => new_head.beacon_state.finalized_checkpoint.epoch,
"root" => format!("{}", beacon_block_root), "root" => %beacon_block_root,
"slot" => new_head.beacon_block.slot(), "slot" => new_head.beacon_block.slot(),
); );
}; };
@ -2214,7 +2214,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
state: &BeaconState<T::EthSpec>, state: &BeaconState<T::EthSpec>,
) -> Result<(), BeaconChainError> { ) -> Result<(), BeaconChainError> {
let finalized_checkpoint = state.finalized_checkpoint; let finalized_checkpoint = state.finalized_checkpoint;
info!(self.log, "Verifying the configured weak subjectivity checkpoint"; "weak_subjectivity_epoch" => wss_checkpoint.epoch, "weak_subjectivity_root" => format!("{:?}", wss_checkpoint.root)); info!(self.log, "Verifying the configured weak subjectivity checkpoint"; "weak_subjectivity_epoch" => wss_checkpoint.epoch, "weak_subjectivity_root" => ?wss_checkpoint.root);
// If epochs match, simply compare roots. // If epochs match, simply compare roots.
if wss_checkpoint.epoch == finalized_checkpoint.epoch if wss_checkpoint.epoch == finalized_checkpoint.epoch
&& wss_checkpoint.root != finalized_checkpoint.root && wss_checkpoint.root != finalized_checkpoint.root
@ -2222,8 +2222,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
crit!( crit!(
self.log, self.log,
"Root found at the specified checkpoint differs"; "Root found at the specified checkpoint differs";
"weak_subjectivity_root" => format!("{:?}", wss_checkpoint.root), "weak_subjectivity_root" => ?wss_checkpoint.root,
"finalized_checkpoint_root" => format!("{:?}", finalized_checkpoint.root) "finalized_checkpoint_root" => ?finalized_checkpoint.root
); );
return Err(BeaconChainError::WeakSubjectivtyVerificationFailure); return Err(BeaconChainError::WeakSubjectivtyVerificationFailure);
} else if wss_checkpoint.epoch < finalized_checkpoint.epoch { } else if wss_checkpoint.epoch < finalized_checkpoint.epoch {
@ -2239,15 +2239,15 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
crit!( crit!(
self.log, self.log,
"Root found at the specified checkpoint differs"; "Root found at the specified checkpoint differs";
"weak_subjectivity_root" => format!("{:?}", wss_checkpoint.root), "weak_subjectivity_root" => ?wss_checkpoint.root,
"finalized_checkpoint_root" => format!("{:?}", finalized_checkpoint.root) "finalized_checkpoint_root" => ?finalized_checkpoint.root
); );
return Err(BeaconChainError::WeakSubjectivtyVerificationFailure); return Err(BeaconChainError::WeakSubjectivtyVerificationFailure);
} }
} }
None => { None => {
crit!(self.log, "The root at the start slot of the given epoch could not be found"; crit!(self.log, "The root at the start slot of the given epoch could not be found";
"wss_checkpoint_slot" => format!("{:?}", slot)); "wss_checkpoint_slot" => ?slot);
return Err(BeaconChainError::WeakSubjectivtyVerificationFailure); return Err(BeaconChainError::WeakSubjectivtyVerificationFailure);
} }
} }
@ -2636,7 +2636,7 @@ impl<T: BeaconChainTypes> Drop for BeaconChain<T> {
error!( error!(
self.log, self.log,
"Failed to persist on BeaconChain drop"; "Failed to persist on BeaconChain drop";
"error" => format!("{:?}", e) "error" => ?e
) )
} else { } else {
info!( info!(