From cc4b778b1fdf42c52ffeede95987a7d4517dbac0 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 31 May 2022 06:09:12 +0000 Subject: [PATCH] Inline `safe_arith` methods (#3229) ## Proposed Changes Speed up epoch processing by around 10% by inlining methods from the `safe_arith` crate. The Rust standard library uses `#[inline]` for the `checked_` functions that we're wrapping, so it makes sense for us to inline them too. ## Additional Info I conducted a brief statistical test on the block at slot [3858336](https://beaconcha.in/block/3858336) applied to the state at slot 3858335, which requires an epoch transition. The command used for testing was: ``` lcli transition-blocks --testnet-dir ./common/eth2_network_config/built_in_network_configs/mainnet --no-signature-verification state.ssz block.ssz output.ssz ``` The testing found that inlining reduced the epoch transition time from 398ms to 359ms, a reduction of 9.77%, which was found to be statistically significant with a two-tailed t-test (p < 0.01). Data and intermediate calculations can be found here: https://docs.google.com/spreadsheets/d/1tlf3eFjz3dcXeb9XVOn21953uYpc9RdQapPtcHGH1PY --- consensus/safe_arith/src/lib.rs | 8 ++++++ lcli/src/main.rs | 8 +++++- lcli/src/transition_blocks.rs | 50 +++++++++++++++++++++++++++++++-- 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/consensus/safe_arith/src/lib.rs b/consensus/safe_arith/src/lib.rs index ab5985a6e..c1dbff4c7 100644 --- a/consensus/safe_arith/src/lib.rs +++ b/consensus/safe_arith/src/lib.rs @@ -20,6 +20,7 @@ macro_rules! assign_method { #[doc = "Safe variant of `"] #[doc = $doc_op] #[doc = "`."] + #[inline] fn $name(&mut self, other: $rhs_ty) -> Result<()> { *self = self.$op(other)?; Ok(()) @@ -68,30 +69,37 @@ macro_rules! impl_safe_arith { const ZERO: Self = 0; const ONE: Self = 1; + #[inline] fn safe_add(&self, other: Self) -> Result { self.checked_add(other).ok_or(ArithError::Overflow) } + #[inline] fn safe_sub(&self, other: Self) -> Result { self.checked_sub(other).ok_or(ArithError::Overflow) } + #[inline] fn safe_mul(&self, other: Self) -> Result { self.checked_mul(other).ok_or(ArithError::Overflow) } + #[inline] fn safe_div(&self, other: Self) -> Result { self.checked_div(other).ok_or(ArithError::DivisionByZero) } + #[inline] fn safe_rem(&self, other: Self) -> Result { self.checked_rem(other).ok_or(ArithError::DivisionByZero) } + #[inline] fn safe_shl(&self, other: u32) -> Result { self.checked_shl(other).ok_or(ArithError::Overflow) } + #[inline] fn safe_shr(&self, other: u32) -> Result { self.checked_shr(other).ok_or(ArithError::Overflow) } diff --git a/lcli/src/main.rs b/lcli/src/main.rs index 0a36768d1..c440f5000 100644 --- a/lcli/src/main.rs +++ b/lcli/src/main.rs @@ -103,7 +103,13 @@ fn main() { .required(true) .default_value("./output.ssz") .help("Path to output a SSZ file."), - ), + ) + .arg( + Arg::with_name("no-signature-verification") + .long("no-signature-verification") + .takes_value(false) + .help("Disable signature verification.") + ) ) .subcommand( SubCommand::with_name("pretty-ssz") diff --git a/lcli/src/transition_blocks.rs b/lcli/src/transition_blocks.rs index f78c6b005..74be1e628 100644 --- a/lcli/src/transition_blocks.rs +++ b/lcli/src/transition_blocks.rs @@ -7,6 +7,7 @@ use state_processing::{ use std::fs::File; use std::io::prelude::*; use std::path::{Path, PathBuf}; +use std::time::Instant; use types::{BeaconState, ChainSpec, EthSpec, SignedBeaconBlock}; pub fn run_transition_blocks( @@ -31,6 +32,13 @@ pub fn run_transition_blocks( .parse::() .map_err(|e| format!("Failed to parse output path: {}", e))?; + let no_signature_verification = matches.is_present("no-signature-verification"); + let signature_strategy = if no_signature_verification { + BlockSignatureStrategy::NoVerification + } else { + BlockSignatureStrategy::VerifyIndividual + }; + info!("Using {} spec", T::spec_name()); info!("Pre-state path: {:?}", pre_state_path); info!("Block path: {:?}", block_path); @@ -43,7 +51,9 @@ pub fn run_transition_blocks( let block: SignedBeaconBlock = load_from_ssz_with(&block_path, spec, SignedBeaconBlock::from_ssz_bytes)?; - let post_state = do_transition(pre_state, block, spec)?; + let t = Instant::now(); + let post_state = do_transition(pre_state, block, signature_strategy, spec)?; + println!("Total transition time: {}ms", t.elapsed().as_millis()); let mut output_file = File::create(output_path).map_err(|e| format!("Unable to create output file: {:?}", e))?; @@ -58,31 +68,58 @@ pub fn run_transition_blocks( fn do_transition( mut pre_state: BeaconState, block: SignedBeaconBlock, + signature_strategy: BlockSignatureStrategy, spec: &ChainSpec, ) -> Result, String> { + let t = Instant::now(); pre_state .build_all_caches(spec) .map_err(|e| format!("Unable to build caches: {:?}", e))?; + println!("Build caches: {}ms", t.elapsed().as_millis()); + + let t = Instant::now(); + pre_state + .update_tree_hash_cache() + .map_err(|e| format!("Unable to build tree hash cache: {:?}", e))?; + println!("Initial tree hash: {}ms", t.elapsed().as_millis()); // Transition the parent state to the block slot. + let t = Instant::now(); for i in pre_state.slot().as_u64()..block.slot().as_u64() { per_slot_processing(&mut pre_state, None, spec) .map_err(|e| format!("Failed to advance slot on iteration {}: {:?}", i, e))?; } + println!("Slot processing: {}ms", t.elapsed().as_millis()); + let t = Instant::now(); + pre_state + .update_tree_hash_cache() + .map_err(|e| format!("Unable to build tree hash cache: {:?}", e))?; + println!("Pre-block tree hash: {}ms", t.elapsed().as_millis()); + + let t = Instant::now(); pre_state .build_all_caches(spec) .map_err(|e| format!("Unable to build caches: {:?}", e))?; + println!("Build all caches (again): {}ms", t.elapsed().as_millis()); + let t = Instant::now(); per_block_processing( &mut pre_state, &block, None, - BlockSignatureStrategy::VerifyIndividual, + signature_strategy, VerifyBlockRoot::True, spec, ) .map_err(|e| format!("State transition failed: {:?}", e))?; + println!("Process block: {}ms", t.elapsed().as_millis()); + + let t = Instant::now(); + pre_state + .update_tree_hash_cache() + .map_err(|e| format!("Unable to build tree hash cache: {:?}", e))?; + println!("Post-block tree hash: {}ms", t.elapsed().as_millis()); Ok(pre_state) } @@ -97,5 +134,12 @@ pub fn load_from_ssz_with( let mut bytes = vec![]; file.read_to_end(&mut bytes) .map_err(|e| format!("Unable to read from file {:?}: {:?}", path, e))?; - decoder(&bytes, spec).map_err(|e| format!("Ssz decode failed: {:?}", e)) + let t = Instant::now(); + let result = decoder(&bytes, spec).map_err(|e| format!("Ssz decode failed: {:?}", e)); + println!( + "SSZ decoding {}: {}ms", + path.display(), + t.elapsed().as_millis() + ); + result }