From 8bad93c8c9d65c71d897218fb853510fb7fc0e53 Mon Sep 17 00:00:00 2001 From: Antioch Peverell Date: Sun, 2 Dec 2018 19:59:24 +0000 Subject: [PATCH] Reduce amount of clone() going on when applying data to MMRs (#2046) * less cloning when pushing to PMMR * pass a ref to data when applying to the MMR and appending to the backend * cleanup * rustfmt * fixup tests --- chain/src/txhashset/txhashset.rs | 12 +++++----- core/src/core/block.rs | 2 +- core/src/core/pmmr/backend.rs | 2 +- core/src/core/pmmr/pmmr.rs | 2 +- core/src/core/transaction.rs | 18 +++++++++++---- core/src/ser.rs | 6 ++--- core/tests/merkle_proof.rs | 20 ++++++++--------- core/tests/pmmr.rs | 38 ++++++++++++++++---------------- core/tests/vec_backend/mod.rs | 8 +++---- store/src/pmmr.rs | 2 +- store/tests/pmmr.rs | 6 ++--- 11 files changed, 62 insertions(+), 54 deletions(-) diff --git a/chain/src/txhashset/txhashset.rs b/chain/src/txhashset/txhashset.rs index babe19f21..d7114fb5d 100644 --- a/chain/src/txhashset/txhashset.rs +++ b/chain/src/txhashset/txhashset.rs @@ -661,9 +661,7 @@ impl<'a> HeaderExtension<'a> { /// This may be either the header MMR or the sync MMR depending on the /// extension. pub fn apply_header(&mut self, header: &BlockHeader) -> Result { - self.pmmr - .push(header.clone()) - .map_err(&ErrorKind::TxHashSetErr)?; + self.pmmr.push(header).map_err(&ErrorKind::TxHashSetErr)?; self.header = header.clone(); Ok(self.root()) } @@ -967,13 +965,13 @@ impl<'a> Extension<'a> { // push the new output to the MMR. let output_pos = self .output_pmmr - .push(out.clone()) + .push(out) .map_err(&ErrorKind::TxHashSetErr)?; // push the rangeproof to the MMR. let rproof_pos = self .rproof_pmmr - .push(out.proof) + .push(&out.proof) .map_err(&ErrorKind::TxHashSetErr)?; // The output and rproof MMRs should be exactly the same size @@ -996,14 +994,14 @@ impl<'a> Extension<'a> { /// Push kernel onto MMR (hash and data files). fn apply_kernel(&mut self, kernel: &TxKernel) -> Result<(), Error> { self.kernel_pmmr - .push(kernel.clone()) + .push(kernel) .map_err(&ErrorKind::TxHashSetErr)?; Ok(()) } fn apply_header(&mut self, header: &BlockHeader) -> Result<(), Error> { self.header_pmmr - .push(header.clone()) + .push(header) .map_err(&ErrorKind::TxHashSetErr)?; Ok(()) } diff --git a/core/src/core/block.rs b/core/src/core/block.rs index 609770dd7..4f2bfc0a5 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -162,7 +162,7 @@ impl Default for BlockHeader { impl PMMRable for BlockHeader { type E = Hash; - fn as_elmt(self) -> Self::E { + fn as_elmt(&self) -> Self::E { self.hash() } } diff --git a/core/src/core/pmmr/backend.rs b/core/src/core/pmmr/backend.rs index 8946168cd..f127cc72a 100644 --- a/core/src/core/pmmr/backend.rs +++ b/core/src/core/pmmr/backend.rs @@ -27,7 +27,7 @@ pub trait Backend { /// associated data element to flatfile storage (for leaf nodes only). The /// position of the first element of the Vec in the MMR is provided to /// help the implementation. - fn append(&mut self, data: T, hashes: Vec) -> Result<(), String>; + fn append(&mut self, data: &T, hashes: Vec) -> Result<(), String>; /// Rewind the backend state to a previous position, as if all append /// operations after that had been canceled. Expects a position in the PMMR diff --git a/core/src/core/pmmr/pmmr.rs b/core/src/core/pmmr/pmmr.rs index 8ef9ec687..ca5c704c1 100644 --- a/core/src/core/pmmr/pmmr.rs +++ b/core/src/core/pmmr/pmmr.rs @@ -171,7 +171,7 @@ where /// Push a new element into the MMR. Computes new related peaks at /// the same time if applicable. - pub fn push(&mut self, elmt: T) -> Result { + pub fn push(&mut self, elmt: &T) -> Result { let elmt_pos = self.last_pos + 1; let mut current_hash = elmt.hash_with_index(elmt_pos - 1); diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index ab510990f..4b1e61efc 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -192,8 +192,8 @@ impl Readable for TxKernel { impl PMMRable for TxKernel { type E = TxKernelEntry; - fn as_elmt(self) -> Self::E { - self.into() + fn as_elmt(&self) -> TxKernelEntry { + TxKernelEntry::from_kernel(self) } } @@ -260,6 +260,9 @@ impl TxKernel { } /// Wrapper around a tx kernel used when maintaining them in the MMR. +/// These will be useful once we implement relative lockheights via relative kernels +/// as a kernel may have an optional rel_kernel but we will not want to store these +/// directly in the kernel MMR. #[derive(Serialize, Deserialize, Debug, Clone)] pub struct TxKernelEntry { /// The underlying tx kernel. @@ -305,6 +308,13 @@ impl TxKernelEntry { pub fn verify(&self) -> Result<(), Error> { self.kernel.verify() } + + /// Build a new tx kernel entry from a kernel. + pub fn from_kernel(kernel: &TxKernel) -> TxKernelEntry { + TxKernelEntry { + kernel: kernel.clone(), + } + } } impl From for TxKernelEntry { @@ -1151,8 +1161,8 @@ impl Readable for Output { impl PMMRable for Output { type E = OutputIdentifier; - fn as_elmt(self) -> Self::E { - self.into() + fn as_elmt(&self) -> OutputIdentifier { + OutputIdentifier::from_output(self) } } diff --git a/core/src/ser.rs b/core/src/ser.rs index ac7ed599d..72a4fb62f 100644 --- a/core/src/ser.rs +++ b/core/src/ser.rs @@ -515,8 +515,8 @@ impl FixedLength for RangeProof { impl PMMRable for RangeProof { type E = Self; - fn as_elmt(self) -> Self::E { - self + fn as_elmt(&self) -> Self::E { + self.clone() } } @@ -695,7 +695,7 @@ pub trait PMMRable: Writeable + Clone + Debug { type E: FixedLength + Readable + Writeable; /// Convert the pmmrable into the element to be stored in the MMR data file. - fn as_elmt(self) -> Self::E; + fn as_elmt(&self) -> Self::E; } /// Generic trait to ensure PMMR elements can be hashed with an index diff --git a/core/tests/merkle_proof.rs b/core/tests/merkle_proof.rs index 0c9810d70..af269e5f3 100644 --- a/core/tests/merkle_proof.rs +++ b/core/tests/merkle_proof.rs @@ -36,7 +36,7 @@ fn merkle_proof_ser_deser() { let mut ba = VecBackend::new(); let mut pmmr = PMMR::new(&mut ba); for x in 0..15 { - pmmr.push(TestElem([0, 0, 0, x])).unwrap(); + pmmr.push(&TestElem([0, 0, 0, x])).unwrap(); } let proof = pmmr.merkle_proof(9).unwrap(); @@ -51,8 +51,8 @@ fn merkle_proof_ser_deser() { fn pmmr_merkle_proof_prune_and_rewind() { let mut ba = VecBackend::new(); let mut pmmr = PMMR::new(&mut ba); - pmmr.push(TestElem([0, 0, 0, 1])).unwrap(); - pmmr.push(TestElem([0, 0, 0, 2])).unwrap(); + pmmr.push(&TestElem([0, 0, 0, 1])).unwrap(); + pmmr.push(&TestElem([0, 0, 0, 2])).unwrap(); let proof = pmmr.merkle_proof(2).unwrap(); // now prune an element and check we can still generate @@ -79,7 +79,7 @@ fn pmmr_merkle_proof() { let mut ba = VecBackend::new(); let mut pmmr = PMMR::new(&mut ba); - pmmr.push(elems[0]).unwrap(); + pmmr.push(&elems[0]).unwrap(); let pos_0 = elems[0].hash_with_index(0); assert_eq!(pmmr.get_hash(1).unwrap(), pos_0); @@ -87,7 +87,7 @@ fn pmmr_merkle_proof() { assert_eq!(proof.path, vec![]); assert!(proof.verify(pmmr.root(), &elems[0], 1).is_ok()); - pmmr.push(elems[1]).unwrap(); + pmmr.push(&elems[1]).unwrap(); let pos_1 = elems[1].hash_with_index(1); assert_eq!(pmmr.get_hash(2).unwrap(), pos_1); let pos_2 = (pos_0, pos_1).hash_with_index(2); @@ -106,7 +106,7 @@ fn pmmr_merkle_proof() { assert!(proof.verify(pmmr.root(), &elems[1], 2).is_ok()); // three leaves, two peaks (one also the right-most leaf) - pmmr.push(elems[2]).unwrap(); + pmmr.push(&elems[2]).unwrap(); let pos_3 = elems[2].hash_with_index(3); assert_eq!(pmmr.get_hash(4).unwrap(), pos_3); @@ -126,7 +126,7 @@ fn pmmr_merkle_proof() { assert!(proof.verify(pmmr.root(), &elems[2], 4).is_ok()); // 7 leaves, 3 peaks, 11 pos in total - pmmr.push(elems[3]).unwrap(); + pmmr.push(&elems[3]).unwrap(); let pos_4 = elems[3].hash_with_index(4); assert_eq!(pmmr.get_hash(5).unwrap(), pos_4); let pos_5 = (pos_3, pos_4).hash_with_index(5); @@ -134,18 +134,18 @@ fn pmmr_merkle_proof() { let pos_6 = (pos_2, pos_5).hash_with_index(6); assert_eq!(pmmr.get_hash(7).unwrap(), pos_6); - pmmr.push(elems[4]).unwrap(); + pmmr.push(&elems[4]).unwrap(); let pos_7 = elems[4].hash_with_index(7); assert_eq!(pmmr.get_hash(8).unwrap(), pos_7); - pmmr.push(elems[5]).unwrap(); + pmmr.push(&elems[5]).unwrap(); let pos_8 = elems[5].hash_with_index(8); assert_eq!(pmmr.get_hash(9).unwrap(), pos_8); let pos_9 = (pos_7, pos_8).hash_with_index(9); assert_eq!(pmmr.get_hash(10).unwrap(), pos_9); - pmmr.push(elems[6]).unwrap(); + pmmr.push(&elems[6]).unwrap(); let pos_10 = elems[6].hash_with_index(10); assert_eq!(pmmr.get_hash(11).unwrap(), pos_10); diff --git a/core/tests/pmmr.rs b/core/tests/pmmr.rs index 9fa1b936f..431675dd8 100644 --- a/core/tests/pmmr.rs +++ b/core/tests/pmmr.rs @@ -281,7 +281,7 @@ fn pmmr_push_root() { let mut pmmr = PMMR::new(&mut ba); // one element - pmmr.push(elems[0]).unwrap(); + pmmr.push(&elems[0]).unwrap(); pmmr.dump(false); let pos_0 = elems[0].hash_with_index(0); assert_eq!(pmmr.peaks(), vec![pos_0]); @@ -289,7 +289,7 @@ fn pmmr_push_root() { assert_eq!(pmmr.unpruned_size(), 1); // two elements - pmmr.push(elems[1]).unwrap(); + pmmr.push(&elems[1]).unwrap(); pmmr.dump(false); let pos_1 = elems[1].hash_with_index(1); let pos_2 = (pos_0, pos_1).hash_with_index(2); @@ -298,7 +298,7 @@ fn pmmr_push_root() { assert_eq!(pmmr.unpruned_size(), 3); // three elements - pmmr.push(elems[2]).unwrap(); + pmmr.push(&elems[2]).unwrap(); pmmr.dump(false); let pos_3 = elems[2].hash_with_index(3); assert_eq!(pmmr.peaks(), vec![pos_2, pos_3]); @@ -306,7 +306,7 @@ fn pmmr_push_root() { assert_eq!(pmmr.unpruned_size(), 4); // four elements - pmmr.push(elems[3]).unwrap(); + pmmr.push(&elems[3]).unwrap(); pmmr.dump(false); let pos_4 = elems[3].hash_with_index(4); let pos_5 = (pos_3, pos_4).hash_with_index(5); @@ -316,7 +316,7 @@ fn pmmr_push_root() { assert_eq!(pmmr.unpruned_size(), 7); // five elements - pmmr.push(elems[4]).unwrap(); + pmmr.push(&elems[4]).unwrap(); pmmr.dump(false); let pos_7 = elems[4].hash_with_index(7); assert_eq!(pmmr.peaks(), vec![pos_6, pos_7]); @@ -324,7 +324,7 @@ fn pmmr_push_root() { assert_eq!(pmmr.unpruned_size(), 8); // six elements - pmmr.push(elems[5]).unwrap(); + pmmr.push(&elems[5]).unwrap(); let pos_8 = elems[5].hash_with_index(8); let pos_9 = (pos_7, pos_8).hash_with_index(9); assert_eq!(pmmr.peaks(), vec![pos_6, pos_9]); @@ -332,7 +332,7 @@ fn pmmr_push_root() { assert_eq!(pmmr.unpruned_size(), 10); // seven elements - pmmr.push(elems[6]).unwrap(); + pmmr.push(&elems[6]).unwrap(); let pos_10 = elems[6].hash_with_index(10); assert_eq!(pmmr.peaks(), vec![pos_6, pos_9, pos_10]); assert_eq!( @@ -343,7 +343,7 @@ fn pmmr_push_root() { // 001001200100123 // eight elements - pmmr.push(elems[7]).unwrap(); + pmmr.push(&elems[7]).unwrap(); let pos_11 = elems[7].hash_with_index(11); let pos_12 = (pos_10, pos_11).hash_with_index(12); let pos_13 = (pos_9, pos_12).hash_with_index(13); @@ -353,7 +353,7 @@ fn pmmr_push_root() { assert_eq!(pmmr.unpruned_size(), 15); // nine elements - pmmr.push(elems[8]).unwrap(); + pmmr.push(&elems[8]).unwrap(); let pos_15 = elems[8].hash_with_index(15); assert_eq!(pmmr.peaks(), vec![pos_14, pos_15]); assert_eq!(pmmr.root(), (pos_14, pos_15).hash_with_index(16)); @@ -381,29 +381,29 @@ fn pmmr_get_last_n_insertions() { let res = pmmr.get_last_n_insertions(19); assert!(res.len() == 0); - pmmr.push(elems[0]).unwrap(); + pmmr.push(&elems[0]).unwrap(); let res = pmmr.get_last_n_insertions(19); assert!(res.len() == 1); - pmmr.push(elems[1]).unwrap(); + pmmr.push(&elems[1]).unwrap(); let res = pmmr.get_last_n_insertions(12); assert!(res.len() == 2); - pmmr.push(elems[2]).unwrap(); + pmmr.push(&elems[2]).unwrap(); let res = pmmr.get_last_n_insertions(2); assert!(res.len() == 2); - pmmr.push(elems[3]).unwrap(); + pmmr.push(&elems[3]).unwrap(); let res = pmmr.get_last_n_insertions(19); assert!(res.len() == 4); - pmmr.push(elems[5]).unwrap(); - pmmr.push(elems[6]).unwrap(); - pmmr.push(elems[7]).unwrap(); - pmmr.push(elems[8]).unwrap(); + pmmr.push(&elems[5]).unwrap(); + pmmr.push(&elems[6]).unwrap(); + pmmr.push(&elems[7]).unwrap(); + pmmr.push(&elems[8]).unwrap(); let res = pmmr.get_last_n_insertions(7); assert!(res.len() == 7); @@ -430,7 +430,7 @@ fn pmmr_prune() { { let mut pmmr = PMMR::new(&mut ba); for elem in &elems[..] { - pmmr.push(*elem).unwrap(); + pmmr.push(elem).unwrap(); } orig_root = pmmr.root(); sz = pmmr.unpruned_size(); @@ -523,7 +523,7 @@ fn check_elements_from_insertion_index() { let mut ba = VecBackend::new(); let mut pmmr = PMMR::new(&mut ba); for x in 1..1000 { - pmmr.push(TestElem([0, 0, 0, x])).unwrap(); + pmmr.push(&TestElem([0, 0, 0, x])).unwrap(); } // Normal case let res = pmmr.elements_from_insertion_index(1, 100); diff --git a/core/tests/vec_backend/mod.rs b/core/tests/vec_backend/mod.rs index 9c22c6896..433fa6144 100644 --- a/core/tests/vec_backend/mod.rs +++ b/core/tests/vec_backend/mod.rs @@ -32,8 +32,8 @@ impl FixedLength for TestElem { impl PMMRable for TestElem { type E = Self; - fn as_elmt(self) -> Self::E { - self + fn as_elmt(&self) -> Self::E { + self.clone() } } @@ -69,8 +69,8 @@ pub struct VecBackend { } impl Backend for VecBackend { - fn append(&mut self, data: T, hashes: Vec) -> Result<(), String> { - self.data.push(data); + fn append(&mut self, data: &T, hashes: Vec) -> Result<(), String> { + self.data.push(data.clone()); self.hashes.append(&mut hashes.clone()); Ok(()) } diff --git a/store/src/pmmr.rs b/store/src/pmmr.rs index 6f17ee5ab..ea090b45d 100644 --- a/store/src/pmmr.rs +++ b/store/src/pmmr.rs @@ -63,7 +63,7 @@ impl Backend for PMMRBackend { /// Append the provided data and hashes to the backend storage. /// Add the new leaf pos to our leaf_set if this is a prunable MMR. #[allow(unused_variables)] - fn append(&mut self, data: T, hashes: Vec) -> Result<(), String> { + fn append(&mut self, data: &T, hashes: Vec) -> Result<(), String> { if self.prunable { let shift = self.prune_list.get_total_shift(); let position = self.hash_file.size_unsync() + shift + 1; diff --git a/store/tests/pmmr.rs b/store/tests/pmmr.rs index 2d7043b6f..700a96c0b 100644 --- a/store/tests/pmmr.rs +++ b/store/tests/pmmr.rs @@ -746,7 +746,7 @@ fn teardown(data_dir: String) { fn load(pos: u64, elems: &[TestElem], backend: &mut store::pmmr::PMMRBackend) -> u64 { let mut pmmr = PMMR::at(backend, pos); for elem in elems { - pmmr.push(elem.clone()).unwrap(); + pmmr.push(elem).unwrap(); } pmmr.unpruned_size() } @@ -761,8 +761,8 @@ impl FixedLength for TestElem { impl PMMRable for TestElem { type E = Self; - fn as_elmt(self) -> Self::E { - self + fn as_elmt(&self) -> Self::E { + self.clone() } }