From 80a8f76c4c36c5afc0ad6b0fef95623e448065c8 Mon Sep 17 00:00:00 2001 From: hashmap Date: Tue, 10 Sep 2019 14:38:36 +0200 Subject: [PATCH] Optimize Option to Error conversion (#3036) To convert option to error we generate an error message. In some places it contains header or block hash code or other data which is costly to produce. So during the initial header sync we spend 12% of all time on generating those messages (in 99% cases we don't use it). This PR introduces a lazy generation of error messages which completely eliminates CPU load during the header sync. --- chain/src/store.rs | 40 ++++++++++++++++++++++++---------------- p2p/src/store.rs | 15 +++++++-------- store/src/lmdb.rs | 7 +++++-- 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/chain/src/store.rs b/chain/src/store.rs index 22691db07..ea06b1ee9 100644 --- a/chain/src/store.rs +++ b/chain/src/store.rs @@ -54,12 +54,12 @@ impl ChainStore { impl ChainStore { /// The current chain head. pub fn head(&self) -> Result { - option_to_not_found(self.db.get_ser(&vec![HEAD_PREFIX]), "HEAD") + option_to_not_found(self.db.get_ser(&vec![HEAD_PREFIX]), || "HEAD".to_owned()) } /// The current chain "tail" (earliest block in the store). pub fn tail(&self) -> Result { - option_to_not_found(self.db.get_ser(&vec![TAIL_PREFIX]), "TAIL") + option_to_not_found(self.db.get_ser(&vec![TAIL_PREFIX]), || "TAIL".to_owned()) } /// Header of the block at the head of the block chain (not the same thing as header_head). @@ -69,19 +69,23 @@ impl ChainStore { /// Head of the header chain (not the same thing as head_header). pub fn header_head(&self) -> Result { - option_to_not_found(self.db.get_ser(&vec![HEADER_HEAD_PREFIX]), "HEADER_HEAD") + option_to_not_found(self.db.get_ser(&vec![HEADER_HEAD_PREFIX]), || { + "HEADER_HEAD".to_owned() + }) } /// The "sync" head. pub fn get_sync_head(&self) -> Result { - option_to_not_found(self.db.get_ser(&vec![SYNC_HEAD_PREFIX]), "SYNC_HEAD") + option_to_not_found(self.db.get_ser(&vec![SYNC_HEAD_PREFIX]), || { + "SYNC_HEAD".to_owned() + }) } /// Get full block. pub fn get_block(&self, h: &Hash) -> Result { option_to_not_found( self.db.get_ser(&to_key(BLOCK_PREFIX, &mut h.to_vec())), - &format!("BLOCK: {}", h), + || format!("BLOCK: {}", h), ) } @@ -94,7 +98,7 @@ impl ChainStore { pub fn get_block_sums(&self, h: &Hash) -> Result { option_to_not_found( self.db.get_ser(&to_key(BLOCK_SUMS_PREFIX, &mut h.to_vec())), - &format!("Block sums for block: {}", h), + || format!("Block sums for block: {}", h), ) } @@ -108,7 +112,7 @@ impl ChainStore { option_to_not_found( self.db .get_ser(&to_key(BLOCK_HEADER_PREFIX, &mut h.to_vec())), - &format!("BLOCK HEADER: {}", h), + || format!("BLOCK HEADER: {}", h), ) } @@ -148,7 +152,7 @@ impl ChainStore { COMMIT_POS_HGT_PREFIX, &mut commit.as_ref().to_vec(), )), - &format!("Output position for: {:?}", commit), + || format!("Output position for: {:?}", commit), ) } @@ -169,12 +173,12 @@ pub struct Batch<'a> { impl<'a> Batch<'a> { /// The head. pub fn head(&self) -> Result { - option_to_not_found(self.db.get_ser(&vec![HEAD_PREFIX]), "HEAD") + option_to_not_found(self.db.get_ser(&vec![HEAD_PREFIX]), || "HEAD".to_owned()) } /// The tail. pub fn tail(&self) -> Result { - option_to_not_found(self.db.get_ser(&vec![TAIL_PREFIX]), "TAIL") + option_to_not_found(self.db.get_ser(&vec![TAIL_PREFIX]), || "TAIL".to_owned()) } /// Header of the block at the head of the block chain (not the same thing as header_head). @@ -184,12 +188,16 @@ impl<'a> Batch<'a> { /// Head of the header chain (not the same thing as head_header). pub fn header_head(&self) -> Result { - option_to_not_found(self.db.get_ser(&vec![HEADER_HEAD_PREFIX]), "HEADER_HEAD") + option_to_not_found(self.db.get_ser(&vec![HEADER_HEAD_PREFIX]), || { + "HEADER_HEAD".to_owned() + }) } /// Get "sync" head. pub fn get_sync_head(&self) -> Result { - option_to_not_found(self.db.get_ser(&vec![SYNC_HEAD_PREFIX]), "SYNC_HEAD") + option_to_not_found(self.db.get_ser(&vec![SYNC_HEAD_PREFIX]), || { + "SYNC_HEAD".to_owned() + }) } /// Save body head to db. @@ -228,7 +236,7 @@ impl<'a> Batch<'a> { pub fn get_block(&self, h: &Hash) -> Result { option_to_not_found( self.db.get_ser(&to_key(BLOCK_PREFIX, &mut h.to_vec())), - &format!("Block with hash: {}", h), + || format!("Block with hash: {}", h), ) } @@ -316,7 +324,7 @@ impl<'a> Batch<'a> { COMMIT_POS_HGT_PREFIX, &mut commit.as_ref().to_vec(), )), - &format!("Output position for commit: {:?}", commit), + || format!("Output position for commit: {:?}", commit), ) } @@ -348,7 +356,7 @@ impl<'a> Batch<'a> { option_to_not_found( self.db .get_ser(&to_key(BLOCK_HEADER_PREFIX, &mut h.to_vec())), - &format!("BLOCK HEADER: {}", h), + || format!("BLOCK HEADER: {}", h), ) } @@ -376,7 +384,7 @@ impl<'a> Batch<'a> { pub fn get_block_sums(&self, h: &Hash) -> Result { option_to_not_found( self.db.get_ser(&to_key(BLOCK_SUMS_PREFIX, &mut h.to_vec())), - &format!("Block sums for block: {}", h), + || format!("Block sums for block: {}", h), ) } diff --git a/p2p/src/store.rs b/p2p/src/store.rs index c7ab70725..be659e924 100644 --- a/p2p/src/store.rs +++ b/p2p/src/store.rs @@ -129,10 +129,9 @@ impl PeerStore { } pub fn get_peer(&self, peer_addr: PeerAddr) -> Result { - option_to_not_found( - self.db.get_ser(&peer_key(peer_addr)[..]), - &format!("Peer at address: {}", peer_addr), - ) + option_to_not_found(self.db.get_ser(&peer_key(peer_addr)[..]), || { + format!("Peer at address: {}", peer_addr) + }) } pub fn exists_peer(&self, peer_addr: PeerAddr) -> Result { @@ -179,10 +178,10 @@ impl PeerStore { pub fn update_state(&self, peer_addr: PeerAddr, new_state: State) -> Result<(), Error> { let batch = self.db.batch()?; - let mut peer = option_to_not_found( - batch.get_ser::(&peer_key(peer_addr)[..]), - &format!("Peer at address: {}", peer_addr), - )?; + let mut peer = + option_to_not_found(batch.get_ser::(&peer_key(peer_addr)[..]), || { + format!("Peer at address: {}", peer_addr) + })?; peer.flags = new_state; if new_state == State::Banned { peer.last_banned = Utc::now().timestamp(); diff --git a/store/src/lmdb.rs b/store/src/lmdb.rs index 12b5fb0bb..b35b21c11 100644 --- a/store/src/lmdb.rs +++ b/store/src/lmdb.rs @@ -54,9 +54,12 @@ impl From for Error { } /// unwraps the inner option by converting the none case to a not found error -pub fn option_to_not_found(res: Result, Error>, field_name: &str) -> Result { +pub fn option_to_not_found(res: Result, Error>, field_name: F) -> Result +where + F: Fn() -> String, +{ match res { - Ok(None) => Err(Error::NotFoundErr(field_name.to_owned())), + Ok(None) => Err(Error::NotFoundErr(field_name())), Ok(Some(o)) => Ok(o), Err(e) => Err(e), }