From 8a22fb516a29c274da2bc5984a621a8f2bd70125 Mon Sep 17 00:00:00 2001 From: hashmap Date: Thu, 30 Apr 2020 17:47:44 +0200 Subject: [PATCH] Reduce number of allocations in to_key calls (#3311) We have to make an extra allocation per db get request because key generation function to_key takes Vec. Taking byte slice (AsRef<[u8]> to be precise) also simplifes the code a bit. --- chain/src/store.rs | 112 +++++++++++++++++--------------------------- p2p/src/store.rs | 6 +-- store/src/lib.rs | 10 ++-- store/tests/lmdb.rs | 8 ++-- 4 files changed, 55 insertions(+), 81 deletions(-) diff --git a/chain/src/store.rs b/chain/src/store.rs index 028f21e6e..5c27e0a58 100644 --- a/chain/src/store.rs +++ b/chain/src/store.rs @@ -88,23 +88,21 @@ impl ChainStore { /// 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), - ) + option_to_not_found(self.db.get_ser(&to_key(BLOCK_PREFIX, h)), || { + format!("BLOCK: {}", h) + }) } /// Does this full block exist? pub fn block_exists(&self, h: &Hash) -> Result { - self.db.exists(&to_key(BLOCK_PREFIX, &mut h.to_vec())) + self.db.exists(&to_key(BLOCK_PREFIX, h)) } /// Get block_sums for the block hash. 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), - ) + option_to_not_found(self.db.get_ser(&to_key(BLOCK_SUMS_PREFIX, h)), || { + format!("Block sums for block: {}", h) + }) } /// Get previous header. @@ -114,11 +112,9 @@ impl ChainStore { /// Get block header. pub fn get_block_header(&self, h: &Hash) -> Result { - option_to_not_found( - self.db - .get_ser(&to_key(BLOCK_HEADER_PREFIX, &mut h.to_vec())), - || format!("BLOCK HEADER: {}", h), - ) + option_to_not_found(self.db.get_ser(&to_key(BLOCK_HEADER_PREFIX, h)), || { + format!("BLOCK HEADER: {}", h) + }) } /// Get PMMR pos for the given output commitment. @@ -134,8 +130,7 @@ impl ChainStore { /// Get PMMR pos and block height for the given output commitment. pub fn get_output_pos_height(&self, commit: &Commitment) -> Result, Error> { - self.db - .get_ser(&to_key(OUTPUT_POS_PREFIX, &mut commit.as_ref().to_vec())) + self.db.get_ser(&to_key(OUTPUT_POS_PREFIX, commit)) } /// Builds a new batch to be used with this store. @@ -192,41 +187,35 @@ impl<'a> Batch<'a> { /// get 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 with hash: {}", h), - ) + option_to_not_found(self.db.get_ser(&to_key(BLOCK_PREFIX, h)), || { + format!("Block with hash: {}", h) + }) } /// Does the block exist? pub fn block_exists(&self, h: &Hash) -> Result { - self.db.exists(&to_key(BLOCK_PREFIX, &mut h.to_vec())) + self.db.exists(&to_key(BLOCK_PREFIX, h)) } /// Save the block to the db. /// Note: the block header is not saved to the db here, assumes this has already been done. pub fn save_block(&self, b: &Block) -> Result<(), Error> { - self.db - .put_ser(&to_key(BLOCK_PREFIX, &mut b.hash().to_vec())[..], b)?; + self.db.put_ser(&to_key(BLOCK_PREFIX, b.hash())[..], b)?; Ok(()) } /// We maintain a "spent" index for each full block to allow the output_pos /// to be easily reverted during rewind. pub fn save_spent_index(&self, h: &Hash, spent: &Vec) -> Result<(), Error> { - self.db - .put_ser(&to_key(BLOCK_SPENT_PREFIX, &mut h.to_vec())[..], spent)?; + self.db.put_ser(&to_key(BLOCK_SPENT_PREFIX, h)[..], spent)?; Ok(()) } /// Migrate a block stored in the db by serializing it using the provided protocol version. /// Block may have been read using a previous protocol version but we do not actually care. pub fn migrate_block(&self, b: &Block, version: ProtocolVersion) -> Result<(), Error> { - self.db.put_ser_with_version( - &to_key(BLOCK_PREFIX, &mut b.hash().to_vec())[..], - b, - version, - )?; + self.db + .put_ser_with_version(&to_key(BLOCK_PREFIX, &mut b.hash())[..], b, version)?; Ok(()) } @@ -238,8 +227,7 @@ impl<'a> Batch<'a> { /// Delete a full block. Does not delete any record associated with a block /// header. pub fn delete_block(&self, bh: &Hash) -> Result<(), Error> { - self.db - .delete(&to_key(BLOCK_PREFIX, &mut bh.to_vec())[..])?; + self.db.delete(&to_key(BLOCK_PREFIX, bh)[..])?; // Best effort at deleting associated data for this block. // Not an error if these fail. @@ -257,7 +245,7 @@ impl<'a> Batch<'a> { // Store the header itself indexed by hash. self.db - .put_ser(&to_key(BLOCK_HEADER_PREFIX, &mut hash.to_vec())[..], header)?; + .put_ser(&to_key(BLOCK_HEADER_PREFIX, hash)[..], header)?; Ok(()) } @@ -269,29 +257,26 @@ impl<'a> Batch<'a> { pos: u64, height: u64, ) -> Result<(), Error> { - self.db.put_ser( - &to_key(OUTPUT_POS_PREFIX, &mut commit.as_ref().to_vec())[..], - &(pos, height), - ) + self.db + .put_ser(&to_key(OUTPUT_POS_PREFIX, commit)[..], &(pos, height)) } /// Delete the output_pos index entry for a spent output. pub fn delete_output_pos_height(&self, commit: &Commitment) -> Result<(), Error> { - self.db - .delete(&to_key(OUTPUT_POS_PREFIX, &mut commit.as_ref().to_vec())) + self.db.delete(&to_key(OUTPUT_POS_PREFIX, commit)) } /// When using the output_pos iterator we have access to the index keys but not the /// original commitment that the key is constructed from. So we need a way of comparing /// a key with another commitment without reconstructing the commitment from the key bytes. pub fn is_match_output_pos_key(&self, key: &[u8], commit: &Commitment) -> bool { - let commit_key = to_key(OUTPUT_POS_PREFIX, &mut commit.as_ref().to_vec()); + let commit_key = to_key(OUTPUT_POS_PREFIX, commit); commit_key == key } /// Iterator over the output_pos index. pub fn output_pos_iter(&self) -> Result, Error> { - let key = to_key(OUTPUT_POS_PREFIX, &mut "".to_string().into_bytes()); + let key = to_key(OUTPUT_POS_PREFIX, ""); self.db.iter(&key) } @@ -308,8 +293,7 @@ impl<'a> Batch<'a> { /// Get output_pos and block height from index. pub fn get_output_pos_height(&self, commit: &Commitment) -> Result, Error> { - self.db - .get_ser(&to_key(OUTPUT_POS_PREFIX, &mut commit.as_ref().to_vec())) + self.db.get_ser(&to_key(OUTPUT_POS_PREFIX, commit)) } /// Get the previous header. @@ -319,41 +303,34 @@ impl<'a> Batch<'a> { /// Get block header. pub fn get_block_header(&self, h: &Hash) -> Result { - option_to_not_found( - self.db - .get_ser(&to_key(BLOCK_HEADER_PREFIX, &mut h.to_vec())), - || format!("BLOCK HEADER: {}", h), - ) + option_to_not_found(self.db.get_ser(&to_key(BLOCK_HEADER_PREFIX, h)), || { + format!("BLOCK HEADER: {}", h) + }) } /// Delete the block spent index. fn delete_spent_index(&self, bh: &Hash) -> Result<(), Error> { // Clean up the legacy input bitmap as well. - let _ = self - .db - .delete(&to_key(BLOCK_INPUT_BITMAP_PREFIX, &mut bh.to_vec())); + let _ = self.db.delete(&to_key(BLOCK_INPUT_BITMAP_PREFIX, bh)); - self.db - .delete(&to_key(BLOCK_SPENT_PREFIX, &mut bh.to_vec())) + self.db.delete(&to_key(BLOCK_SPENT_PREFIX, bh)) } /// Save block_sums for the block. pub fn save_block_sums(&self, h: &Hash, sums: BlockSums) -> Result<(), Error> { - self.db - .put_ser(&to_key(BLOCK_SUMS_PREFIX, &mut h.to_vec())[..], &sums) + self.db.put_ser(&to_key(BLOCK_SUMS_PREFIX, h)[..], &sums) } /// Get block_sums for the block. 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), - ) + option_to_not_found(self.db.get_ser(&to_key(BLOCK_SUMS_PREFIX, h)), || { + format!("Block sums for block: {}", h) + }) } /// Delete the block_sums for the block. fn delete_block_sums(&self, bh: &Hash) -> Result<(), Error> { - self.db.delete(&to_key(BLOCK_SUMS_PREFIX, &mut bh.to_vec())) + self.db.delete(&to_key(BLOCK_SUMS_PREFIX, bh)) } /// Get the block input bitmap based on our spent index. @@ -371,10 +348,7 @@ impl<'a> Batch<'a> { } fn get_legacy_input_bitmap(&self, bh: &Hash) -> Result { - if let Ok(Some(bytes)) = self - .db - .get(&to_key(BLOCK_INPUT_BITMAP_PREFIX, &mut bh.to_vec())) - { + if let Ok(Some(bytes)) = self.db.get(&to_key(BLOCK_INPUT_BITMAP_PREFIX, bh)) { Ok(Bitmap::deserialize(&bytes)) } else { Err(Error::NotFoundErr("legacy block input bitmap".to_string())) @@ -384,11 +358,9 @@ impl<'a> Batch<'a> { /// Get the "spent index" from the db for the specified block. /// If we need to rewind a block then we use this to "unspend" the spent outputs. pub fn get_spent_index(&self, bh: &Hash) -> Result, Error> { - option_to_not_found( - self.db - .get_ser(&to_key(BLOCK_SPENT_PREFIX, &mut bh.to_vec())), - || format!("spent index: {}", bh), - ) + option_to_not_found(self.db.get_ser(&to_key(BLOCK_SPENT_PREFIX, bh)), || { + format!("spent index: {}", bh) + }) } /// Commits this batch. If it's a child batch, it will be merged with the @@ -407,7 +379,7 @@ impl<'a> Batch<'a> { /// An iterator to all block in db pub fn blocks_iter(&self) -> Result, Error> { - let key = to_key(BLOCK_PREFIX, &mut "".to_string().into_bytes()); + let key = to_key(BLOCK_PREFIX, ""); self.db.iter(&key) } } diff --git a/p2p/src/store.rs b/p2p/src/store.rs index baba718c9..9dae5387f 100644 --- a/p2p/src/store.rs +++ b/p2p/src/store.rs @@ -154,7 +154,7 @@ impl PeerStore { ) -> Result, Error> { let mut peers = self .db - .iter::(&to_key(PEER_PREFIX, &mut "".to_string().into_bytes()))? + .iter::(&to_key(PEER_PREFIX, ""))? .map(|(_, v)| v) .filter(|p| p.flags == state && p.capabilities.contains(cap)) .collect::>(); @@ -165,7 +165,7 @@ impl PeerStore { /// List all known peers /// Used for /v1/peers/all api endpoint pub fn all_peers(&self) -> Result, Error> { - let key = to_key(PEER_PREFIX, &mut "".to_string().into_bytes()); + let key = to_key(PEER_PREFIX, ""); Ok(self .db .iter::(&key)? @@ -221,5 +221,5 @@ impl PeerStore { // Ignore the port unless ip is loopback address. fn peer_key(peer_addr: PeerAddr) -> Vec { - to_key(PEER_PREFIX, &mut peer_addr.as_key().into_bytes()) + to_key(PEER_PREFIX, &peer_addr.as_key()) } diff --git a/store/src/lib.rs b/store/src/lib.rs index 2378dbbdc..e9e7a17b1 100644 --- a/store/src/lib.rs +++ b/store/src/lib.rs @@ -44,20 +44,22 @@ use byteorder::{BigEndian, WriteBytesExt}; pub use crate::lmdb::*; /// Build a db key from a prefix and a byte vector identifier. -pub fn to_key(prefix: u8, k: &mut Vec) -> Vec { +pub fn to_key>(prefix: u8, k: K) -> Vec { + let k = k.as_ref(); let mut res = Vec::with_capacity(k.len() + 2); res.push(prefix); res.push(SEP); - res.append(k); + res.extend_from_slice(k); res } /// Build a db key from a prefix and a byte vector identifier and numeric identifier -pub fn to_key_u64(prefix: u8, k: &mut Vec, val: u64) -> Vec { +pub fn to_key_u64>(prefix: u8, k: K, val: u64) -> Vec { + let k = k.as_ref(); let mut res = Vec::with_capacity(k.len() + 10); res.push(prefix); res.push(SEP); - res.append(k); + res.extend_from_slice(k); res.write_u64::(val).unwrap(); res } diff --git a/store/tests/lmdb.rs b/store/tests/lmdb.rs index a8d1e397c..6e63dce7d 100644 --- a/store/tests/lmdb.rs +++ b/store/tests/lmdb.rs @@ -75,9 +75,9 @@ fn lmdb_allocate() -> Result<(), store::Error> { for i in 0..WRITE_CHUNK_SIZE * 2 { println!("Allocating chunk: {}", i); let chunk = PhatChunkStruct::new(); - let mut key_val = format!("phat_chunk_set_1_{}", i).as_bytes().to_vec(); + let key_val = format!("phat_chunk_set_1_{}", i); let batch = store.batch()?; - let key = store::to_key(b'P', &mut key_val); + let key = store::to_key(b'P', &key_val); batch.put_ser(&key, &chunk)?; batch.commit()?; } @@ -91,9 +91,9 @@ fn lmdb_allocate() -> Result<(), store::Error> { for i in 0..WRITE_CHUNK_SIZE * 2 { println!("Allocating chunk: {}", i); let chunk = PhatChunkStruct::new(); - let mut key_val = format!("phat_chunk_set_2_{}", i).as_bytes().to_vec(); + let key_val = format!("phat_chunk_set_2_{}", i); let batch = store.batch()?; - let key = store::to_key(b'P', &mut key_val); + let key = store::to_key(b'P', &key_val); batch.put_ser(&key, &chunk)?; batch.commit()?; }