-
Notifications
You must be signed in to change notification settings - Fork 979
perf: cache packed proof nonces for cheaper header/block hash #3899
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -330,12 +330,23 @@ impl ProofOfWork { | |
| /// (i+1) * edge_bits - 1, padding it with up to 7 0-bits to a multiple of 8 bits, | ||
| /// writing as a little endian byte array, and hashing with blake2b using 256 bit digest. | ||
|
|
||
| #[derive(Clone, PartialOrd, PartialEq, Serialize)] | ||
| #[derive(Clone, Serialize)] | ||
| pub struct Proof { | ||
| /// Power of 2 used for the size of the cuckoo graph | ||
| pub edge_bits: u8, | ||
| /// The nonces | ||
| pub nonces: Vec<u64>, | ||
| /// Cached packed nonces used for Hash (and full) serialization. | ||
| /// | ||
| /// Populated when the proof is deserialized from the on-wire / on-disk bit | ||
| /// packing, so subsequent `hash()` / `write()` calls do not re-run the | ||
| /// relatively expensive bit-packing step. Not populated for locally | ||
| /// constructed proofs (mining / tests) unless filled explicitly. | ||
| /// | ||
| /// **Important:** if you mutate `edge_bits` or `nonces` after construction, | ||
| /// call [`Proof::clear_packed_cache`] so a stale cache cannot poison hashes. | ||
| #[serde(skip)] | ||
| packed_nonces: Option<Vec<u8>>, | ||
| } | ||
|
|
||
| impl DefaultHashable for Proof {} | ||
|
|
@@ -353,15 +364,36 @@ impl fmt::Debug for Proof { | |
| } | ||
| } | ||
|
|
||
| impl PartialEq for Proof { | ||
| fn eq(&self, other: &Proof) -> bool { | ||
| self.edge_bits == other.edge_bits && self.nonces == other.nonces | ||
| } | ||
| } | ||
|
|
||
| impl Eq for Proof {} | ||
|
|
||
| impl PartialOrd for Proof { | ||
| fn partial_cmp(&self, other: &Proof) -> Option<std::cmp::Ordering> { | ||
| Some(self.cmp(other)) | ||
| } | ||
| } | ||
|
|
||
| impl Ord for Proof { | ||
| fn cmp(&self, other: &Proof) -> std::cmp::Ordering { | ||
| self.edge_bits | ||
| .cmp(&other.edge_bits) | ||
| .then_with(|| self.nonces.cmp(&other.nonces)) | ||
| } | ||
| } | ||
|
|
||
| impl Proof { | ||
| /// Builds a proof with provided nonces at default edge_bits | ||
| pub fn new(mut in_nonces: Vec<u64>) -> Proof { | ||
| in_nonces.sort_unstable(); | ||
| Proof { | ||
| edge_bits: global::min_edge_bits(), | ||
| nonces: in_nonces, | ||
| packed_nonces: None, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -370,6 +402,7 @@ impl Proof { | |
| Proof { | ||
| edge_bits: global::min_edge_bits(), | ||
| nonces: vec![0; proof_size], | ||
| packed_nonces: None, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -394,6 +427,7 @@ impl Proof { | |
| Proof { | ||
| edge_bits: global::min_edge_bits(), | ||
| nonces: v, | ||
| packed_nonces: None, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -402,8 +436,27 @@ impl Proof { | |
| self.nonces.len() | ||
| } | ||
|
|
||
| /// Pack the nonces of the proof to their exact bit size as described above | ||
| /// Drop any cached packed nonces. Call this after mutating `edge_bits` or `nonces`. | ||
| pub fn clear_packed_cache(&mut self) { | ||
| self.packed_nonces = None; | ||
| } | ||
|
|
||
| /// Whether this proof currently holds a packed-nonces cache. | ||
| #[cfg(test)] | ||
| pub fn has_packed_cache(&self) -> bool { | ||
| self.packed_nonces.is_some() | ||
| } | ||
|
|
||
| /// Pack the nonces of the proof to their exact bit size as described above. | ||
| /// Uses the deserialized cache when available. | ||
| pub fn pack_nonces(&self) -> Vec<u8> { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add before/after numbers for repeated header hashing? The cache keeps and clones an extra allocation per proof, so it would be good to confirm the net win. |
||
| if let Some(ref packed) = self.packed_nonces { | ||
| return packed.clone(); | ||
| } | ||
| self.compute_packed_nonces() | ||
| } | ||
|
|
||
| fn compute_packed_nonces(&self) -> Vec<u8> { | ||
| let mut compressed = vec![0u8; Proof::pack_len(self.edge_bits)]; | ||
| pack_bits( | ||
| self.edge_bits, | ||
|
|
@@ -510,11 +563,17 @@ impl Readable for Proof { | |
| if read_number(&bits, end_of_data, bytes_len * 8 - end_of_data) != 0 { | ||
| return Err(ser::Error::CorruptedData); | ||
| } | ||
| Ok(Proof { edge_bits, nonces }) | ||
| // Cache the on-wire packed form so later hash()/write() skip re-packing. | ||
| Ok(Proof { | ||
| edge_bits, | ||
| nonces, | ||
| packed_nonces: Some(bits), | ||
| }) | ||
| } else { | ||
| Ok(Proof { | ||
| edge_bits, | ||
| nonces: vec![], | ||
| packed_nonces: None, | ||
| }) | ||
| } | ||
| } | ||
|
|
@@ -525,7 +584,12 @@ impl Writeable for Proof { | |
| if writer.serialization_mode() != ser::SerializationMode::Hash { | ||
| writer.write_u8(self.edge_bits)?; | ||
| } | ||
| writer.write_fixed_bytes(&self.pack_nonces()) | ||
| // Prefer the deserialized packed form (hash-hot path); otherwise pack on demand. | ||
| if let Some(ref packed) = self.packed_nonces { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we use the cache only for hash serialization and always repack full wire/disk writes from the fields? That matches #3372 and prevents stale cached bytes from being emitted. |
||
| writer.write_fixed_bytes(packed) | ||
| } else { | ||
| writer.write_fixed_bytes(&self.compute_packed_nonces()) | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -542,6 +606,7 @@ mod tests { | |
| for edge_bits in 10..63 { | ||
| let mut proof = Proof::new(gen_proof(edge_bits as u32)); | ||
| proof.edge_bits = edge_bits; | ||
| proof.clear_packed_cache(); | ||
| let mut buf = Cursor::new(Vec::new()); | ||
| let mut w = BinWriter::new(&mut buf, ProtocolVersion::local()); | ||
| if let Err(e) = proof.write(&mut w) { | ||
|
|
@@ -555,11 +620,43 @@ mod tests { | |
| ); | ||
| match Proof::read(&mut r) { | ||
| Err(e) => panic!("failed to read proof: {:?}", e), | ||
| Ok(p) => assert_eq!(p, proof), | ||
| Ok(p) => { | ||
| assert_eq!(p, proof); | ||
| assert!( | ||
| p.has_packed_cache(), | ||
| "deserialized proof should cache packed nonces" | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn packed_cache_matches_recomputed_and_hash() { | ||
| global::set_local_chain_type(global::ChainTypes::Mainnet); | ||
| let mut proof = Proof::new(gen_proof(29)); | ||
| proof.edge_bits = 29; | ||
| let hash_before = proof.hash(); | ||
| let packed = proof.compute_packed_nonces(); | ||
|
|
||
| // Simulate a deserialized proof: same nonces, cache filled. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this use a real serialize/deserialize round trip instead of setting the private cache directly? That would cover the actual read => cache => hash/write path. |
||
| let cached = Proof { | ||
| edge_bits: proof.edge_bits, | ||
| nonces: proof.nonces.clone(), | ||
| packed_nonces: Some(packed.clone()), | ||
| }; | ||
| assert!(cached.has_packed_cache()); | ||
| assert_eq!(cached.pack_nonces(), packed); | ||
| assert_eq!(cached.hash(), hash_before); | ||
|
|
||
| // Mutating nonces without clearing would be wrong; clear keeps hash correct. | ||
| let mut mutated = cached.clone(); | ||
| mutated.nonces[0] = mutated.nonces[0].wrapping_add(1); | ||
| mutated.clear_packed_cache(); | ||
| assert!(!mutated.has_packed_cache()); | ||
| assert_ne!(mutated.hash(), hash_before); | ||
| } | ||
|
|
||
| fn gen_proof(bits: u32) -> Vec<u64> { | ||
| let mut rng = rand::thread_rng(); | ||
| let mut v = Vec::with_capacity(42); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cache depends on edge_bits and nonces, but both are still publicly mutable. A missed clear_packed_cache() makes verification and hashing use different proof data. Could we make mutation invalidate the cache automatically in this PR?