From 91933d857da2d30692276ad3a3b043067c3ff568 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 6 Mar 2019 15:30:35 +0100 Subject: [PATCH] perf(ethcore): `micro-opt` (#10405) Mostly fixes that changes `eagerly eval` to `lazy eval` --- ethcore/light/src/on_demand/mod.rs | 6 +----- ethcore/src/client/client.rs | 2 +- ethcore/src/engines/authority_round/mod.rs | 24 ++++++++++++++------- ethcore/src/engines/validator_set/multi.rs | 4 ++-- ethcore/src/error.rs | 4 ++-- ethcore/src/executed.rs | 2 +- ethcore/src/externalities.rs | 2 +- ethcore/src/machine.rs | 2 +- ethcore/src/snapshot/consensus/authority.rs | 6 +++--- ethcore/src/snapshot/consensus/work.rs | 4 ++-- ethcore/src/snapshot/mod.rs | 4 ++-- ethcore/src/spec/spec.rs | 2 +- ethcore/src/state/mod.rs | 8 +++---- ethcore/src/verification/verification.rs | 2 +- ethcore/types/src/ids.rs | 2 +- 15 files changed, 39 insertions(+), 35 deletions(-) diff --git a/ethcore/light/src/on_demand/mod.rs b/ethcore/light/src/on_demand/mod.rs index 609868ecb..c04c68794 100644 --- a/ethcore/light/src/on_demand/mod.rs +++ b/ethcore/light/src/on_demand/mod.rs @@ -24,7 +24,6 @@ use std::marker::PhantomData; use std::sync::Arc; use std::time::Duration; -use ethcore::executed::{Executed, ExecutionError}; use futures::{Poll, Future, Async}; use futures::sync::oneshot::{self, Receiver}; use network::PeerId; @@ -41,10 +40,10 @@ use cache::Cache; use request::{self as basic_request, Request as NetworkRequest}; use self::request::CheckedRequest; +pub use ethcore::executed::ExecutionResult; pub use self::request::{Request, Response, HeaderRef, Error as ValidityError}; pub use self::request_guard::{RequestGuard, Error as RequestError}; pub use self::response_guard::{ResponseGuard, Error as ResponseGuardError, Inner as ResponseGuardInner}; - pub use types::request::ResponseError; #[cfg(test)] @@ -54,9 +53,6 @@ pub mod request; mod request_guard; mod response_guard; -/// The result of execution -pub type ExecutionResult = Result; - /// The initial backoff interval for OnDemand queries pub const DEFAULT_REQUEST_MIN_BACKOFF_DURATION: Duration = Duration::from_secs(10); /// The maximum request interval for OnDemand queries diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 0be48a636..9eab30f34 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1149,7 +1149,7 @@ impl Client { pub fn take_snapshot(&self, writer: W, at: BlockId, p: &snapshot::Progress) -> Result<(), EthcoreError> { let db = self.state_db.read().journal_db().boxed_clone(); let best_block_number = self.chain_info().best_block_number; - let block_number = self.block_number(at).ok_or(snapshot::Error::InvalidStartingBlock(at))?; + let block_number = self.block_number(at).ok_or_else(|| snapshot::Error::InvalidStartingBlock(at))?; if db.is_pruned() && self.pruning_info().earliest_state > block_number { return Err(snapshot::Error::OldBlockPrunedDB.into()); diff --git a/ethcore/src/engines/authority_round/mod.rs b/ethcore/src/engines/authority_round/mod.rs index 2a42acfe2..ef439b5d4 100644 --- a/ethcore/src/engines/authority_round/mod.rs +++ b/ethcore/src/engines/authority_round/mod.rs @@ -512,15 +512,19 @@ fn header_expected_seal_fields(header: &Header, empty_steps_transition: u64) -> } fn header_step(header: &Header, empty_steps_transition: u64) -> Result { - let expected_seal_fields = header_expected_seal_fields(header, empty_steps_transition); - Rlp::new(&header.seal().get(0).expect( - &format!("was either checked with verify_block_basic or is genesis; has {} fields; qed (Make sure the spec file has a correct genesis seal)", expected_seal_fields))).as_val() + Rlp::new(&header.seal().get(0).unwrap_or_else(|| + panic!("was either checked with verify_block_basic or is genesis; has {} fields; qed (Make sure the spec + file has a correct genesis seal)", header_expected_seal_fields(header, empty_steps_transition)) + )) + .as_val() } fn header_signature(header: &Header, empty_steps_transition: u64) -> Result { - let expected_seal_fields = header_expected_seal_fields(header, empty_steps_transition); - Rlp::new(&header.seal().get(1).expect( - &format!("was checked with verify_block_basic; has {} fields; qed", expected_seal_fields))).as_val::().map(Into::into) + Rlp::new(&header.seal().get(1).unwrap_or_else(|| + panic!("was checked with verify_block_basic; has {} fields; qed", + header_expected_seal_fields(header, empty_steps_transition)) + )) + .as_val::().map(Into::into) } // extracts the raw empty steps vec from the header seal. should only be called when there are 3 fields in the seal @@ -934,8 +938,12 @@ impl Engine for AuthorityRound { return BTreeMap::default(); } - let step = header_step(header, self.empty_steps_transition).as_ref().map(ToString::to_string).unwrap_or("".into()); - let signature = header_signature(header, self.empty_steps_transition).as_ref().map(ToString::to_string).unwrap_or("".into()); + let step = header_step(header, self.empty_steps_transition).as_ref() + .map(ToString::to_string) + .unwrap_or_default(); + let signature = header_signature(header, self.empty_steps_transition).as_ref() + .map(ToString::to_string) + .unwrap_or_default(); let mut info = map![ "step".into() => step, diff --git a/ethcore/src/engines/validator_set/multi.rs b/ethcore/src/engines/validator_set/multi.rs index 8aa7aa3de..b9ef67747 100644 --- a/ethcore/src/engines/validator_set/multi.rs +++ b/ethcore/src/engines/validator_set/multi.rs @@ -74,7 +74,7 @@ impl Multi { impl ValidatorSet for Multi { fn default_caller(&self, block_id: BlockId) -> Box { self.correct_set(block_id).map(|set| set.default_caller(block_id)) - .unwrap_or(Box::new(|_, _| Err("No validator set for given ID.".into()))) + .unwrap_or_else(|| Box::new(|_, _| Err("No validator set for given ID.".into()))) } fn on_epoch_begin(&self, _first: bool, header: &Header, call: &mut SystemCall) -> Result<(), ::error::Error> { @@ -141,7 +141,7 @@ impl ValidatorSet for Multi { *self.block_number.write() = Box::new(move |id| client .upgrade() .ok_or_else(|| "No client!".into()) - .and_then(|c| c.block_number(id).ok_or("Unknown block".into()))); + .and_then(|c| c.block_number(id).ok_or_else(|| "Unknown block".into()))); } } diff --git a/ethcore/src/error.rs b/ethcore/src/error.rs index c3ffc9043..d5aa45ba0 100644 --- a/ethcore/src/error.rs +++ b/ethcore/src/error.rs @@ -37,7 +37,7 @@ use engines::EngineError; pub use executed::{ExecutionError, CallError}; -#[derive(Debug, PartialEq, Clone, Copy, Eq)] +#[derive(Debug, PartialEq, Clone, Eq)] /// Errors concerning block processing. pub enum BlockError { /// Block has too many uncles. @@ -88,7 +88,7 @@ pub enum BlockError { /// Timestamp header field is too far in future. TemporarilyInvalid(OutOfBounds), /// Log bloom header field is invalid. - InvalidLogBloom(Mismatch), + InvalidLogBloom(Box>), /// Number field of header is invalid. InvalidNumber(Mismatch), /// Block number isn't sensible. diff --git a/ethcore/src/executed.rs b/ethcore/src/executed.rs index f9be002ff..0f46ee1de 100644 --- a/ethcore/src/executed.rs +++ b/ethcore/src/executed.rs @@ -197,4 +197,4 @@ impl fmt::Display for CallError { } /// Transaction execution result. -pub type ExecutionResult = Result; +pub type ExecutionResult = Result, ExecutionError>; diff --git a/ethcore/src/externalities.rs b/ethcore/src/externalities.rs index a2f35b6de..23a4a83c3 100644 --- a/ethcore/src/externalities.rs +++ b/ethcore/src/externalities.rs @@ -117,7 +117,7 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> { fn initial_storage_at(&self, key: &H256) -> vm::Result { if self.state.is_base_storage_root_unchanged(&self.origin_info.address)? { - self.state.checkpoint_storage_at(0, &self.origin_info.address, key).map(|v| v.unwrap_or(H256::zero())).map_err(Into::into) + self.state.checkpoint_storage_at(0, &self.origin_info.address, key).map(|v| v.unwrap_or_default()).map_err(Into::into) } else { warn!(target: "externalities", "Detected existing account {:#x} where a forced contract creation happened.", self.origin_info.address); Ok(H256::zero()) diff --git a/ethcore/src/machine.rs b/ethcore/src/machine.rs index a14cb3bd2..c0eae63d1 100644 --- a/ethcore/src/machine.rs +++ b/ethcore/src/machine.rs @@ -173,7 +173,7 @@ impl EthereumMachine { origin: SYSTEM_ADDRESS, gas, gas_price: 0.into(), - value: value.unwrap_or(ActionValue::Transfer(0.into())), + value: value.unwrap_or_else(|| ActionValue::Transfer(0.into())), code, code_hash, data, diff --git a/ethcore/src/snapshot/consensus/authority.rs b/ethcore/src/snapshot/consensus/authority.rs index 909e00281..4423e0740 100644 --- a/ethcore/src/snapshot/consensus/authority.rs +++ b/ethcore/src/snapshot/consensus/authority.rs @@ -76,7 +76,7 @@ impl SnapshotComponents for PoaSnapshot { } let header = chain.block_header_data(&transition.block_hash) - .ok_or(Error::BlockNotFound(transition.block_hash))?; + .ok_or_else(|| Error::BlockNotFound(transition.block_hash))?; let entry = { let mut entry_stream = RlpStream::new_list(2); @@ -101,12 +101,12 @@ impl SnapshotComponents for PoaSnapshot { let (block, receipts) = chain.block(&block_at) .and_then(|b| chain.block_receipts(&block_at).map(|r| (b, r))) - .ok_or(Error::BlockNotFound(block_at))?; + .ok_or_else(|| Error::BlockNotFound(block_at))?; let block = block.decode()?; let parent_td = chain.block_details(block.header.parent_hash()) .map(|d| d.total_difficulty) - .ok_or(Error::BlockNotFound(block_at))?; + .ok_or_else(|| Error::BlockNotFound(block_at))?; rlps.push({ let mut stream = RlpStream::new_list(5); diff --git a/ethcore/src/snapshot/consensus/work.rs b/ethcore/src/snapshot/consensus/work.rs index 0293d1a2f..106fe4474 100644 --- a/ethcore/src/snapshot/consensus/work.rs +++ b/ethcore/src/snapshot/consensus/work.rs @@ -116,7 +116,7 @@ impl<'a> PowWorker<'a> { let (block, receipts) = self.chain.block(&self.current_hash) .and_then(|b| self.chain.block_receipts(&self.current_hash).map(|r| (b, r))) - .ok_or(Error::BlockNotFound(self.current_hash))?; + .ok_or_else(|| Error::BlockNotFound(self.current_hash))?; let abridged_rlp = AbridgedBlock::from_block_view(&block.view()).into_inner(); @@ -160,7 +160,7 @@ impl<'a> PowWorker<'a> { let (last_header, last_details) = self.chain.block_header_data(&last) .and_then(|n| self.chain.block_details(&last).map(|d| (n, d))) - .ok_or(Error::BlockNotFound(last))?; + .ok_or_else(|| Error::BlockNotFound(last))?; let parent_number = last_header.number() - 1; let parent_hash = last_header.parent_hash(); diff --git a/ethcore/src/snapshot/mod.rs b/ethcore/src/snapshot/mod.rs index c05d0de6a..19a5f8ce6 100644 --- a/ethcore/src/snapshot/mod.rs +++ b/ethcore/src/snapshot/mod.rs @@ -157,7 +157,7 @@ pub fn take_snapshot( processing_threads: usize, ) -> Result<(), Error> { let start_header = chain.block_header_data(&block_at) - .ok_or(Error::InvalidStartingBlock(BlockId::Hash(block_at)))?; + .ok_or_else(|| Error::InvalidStartingBlock(BlockId::Hash(block_at)))?; let state_root = start_header.state_root(); let number = start_header.number(); @@ -512,7 +512,7 @@ fn rebuild_accounts( // fill out the storage trie and code while decoding. let (acc, maybe_code) = { let mut acct_db = AccountDBMut::from_hash(db, hash); - let storage_root = known_storage_roots.get(&hash).cloned().unwrap_or(H256::zero()); + let storage_root = known_storage_roots.get(&hash).cloned().unwrap_or_default(); account::from_fat_rlp(&mut acct_db, fat_rlp, storage_root)? }; diff --git a/ethcore/src/spec/spec.rs b/ethcore/src/spec/spec.rs index 4bd46b2bb..2726ed4fd 100644 --- a/ethcore/src/spec/spec.rs +++ b/ethcore/src/spec/spec.rs @@ -515,7 +515,7 @@ fn load_from(spec_params: SpecParams, s: ethjson::spec::Spec) -> Result), } #[derive(Eq, PartialEq, Clone, Copy, Debug)] @@ -218,7 +218,7 @@ pub fn check_proof( let options = TransactOptions::with_no_tracing().save_output_from_contract(); match state.execute(env_info, machine, transaction, options, true) { - Ok(executed) => ProvedExecution::Complete(executed), + Ok(executed) => ProvedExecution::Complete(Box::new(executed)), Err(ExecutionError::Internal(_)) => ProvedExecution::BadProof, Err(e) => ProvedExecution::Failed(e), } @@ -1254,7 +1254,7 @@ impl State { let trie = TrieDB::new(db, &self.root)?; let maybe_account: Option = { let panicky_decoder = |bytes: &[u8]| { - ::rlp::decode(bytes).expect(&format!("prove_account, could not query trie for account key={}", &account_key)) + ::rlp::decode(bytes).unwrap_or_else(|_| panic!("prove_account, could not query trie for account key={}", &account_key)) }; let query = (&mut recorder, panicky_decoder); trie.get_with(&account_key, query)? diff --git a/ethcore/src/verification/verification.rs b/ethcore/src/verification/verification.rs index 3f5008a2b..2ce9f9de3 100644 --- a/ethcore/src/verification/verification.rs +++ b/ethcore/src/verification/verification.rs @@ -274,7 +274,7 @@ pub fn verify_block_final(expected: &Header, got: &Header) -> Result<(), Error> return Err(From::from(BlockError::InvalidGasUsed(Mismatch { expected: *expected.gas_used(), found: *got.gas_used() }))) } if expected.log_bloom() != got.log_bloom() { - return Err(From::from(BlockError::InvalidLogBloom(Mismatch { expected: *expected.log_bloom(), found: *got.log_bloom() }))) + return Err(From::from(BlockError::InvalidLogBloom(Box::new(Mismatch { expected: *expected.log_bloom(), found: *got.log_bloom() })))) } if expected.receipts_root() != got.receipts_root() { return Err(From::from(BlockError::InvalidReceiptsRoot(Mismatch { expected: *expected.receipts_root(), found: *got.receipts_root() }))) diff --git a/ethcore/types/src/ids.rs b/ethcore/types/src/ids.rs index dccb240d9..1f099be57 100644 --- a/ethcore/types/src/ids.rs +++ b/ethcore/types/src/ids.rs @@ -17,7 +17,7 @@ //! Unique identifiers. use ethereum_types::H256; -use {BlockNumber}; +use BlockNumber; /// Uniquely identifies block. #[derive(Debug, PartialEq, Copy, Clone, Hash, Eq)]