From 73fab82cbf78cffd11b2415355368c794db04ebe Mon Sep 17 00:00:00 2001 From: arvidn Date: Fri, 20 Dec 2024 10:48:29 +0100 Subject: [PATCH 1/4] optimize ReadCacheLookup::find_paths() --- benches/serialize.rs | 1 - src/serde/read_cache_lookup.rs | 66 ++++++++++++++++++++++++++-------- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/benches/serialize.rs b/benches/serialize.rs index 8d6db2e2..de6e2a67 100644 --- a/benches/serialize.rs +++ b/benches/serialize.rs @@ -15,7 +15,6 @@ fn serialize_benchmark(c: &mut Criterion) { let block1: &[u8] = include_bytes!("1.generator"); let mut group = c.benchmark_group("serialize"); - group.sample_size(10); for (block, name) in [(&block0, "0"), (&block1, "1")] { let mut a = Allocator::new(); diff --git a/src/serde/read_cache_lookup.rs b/src/serde/read_cache_lookup.rs index d01e2b1d..89930a24 100644 --- a/src/serde/read_cache_lookup.rs +++ b/src/serde/read_cache_lookup.rs @@ -16,9 +16,35 @@ /// /// All hashes correspond to sha256 tree hashes. use std::collections::{HashMap, HashSet}; +use std::hash::{BuildHasher, Hasher}; use super::bytes32::{hash_blob, hash_blobs, Bytes32}; +#[derive(Default, Clone, Copy)] +pub struct IdentityHash(u64); + +impl Hasher for IdentityHash { + fn finish(&self) -> u64 { + self.0 + } + + fn write(&mut self, bytes: &[u8]) { + self.0 = u64::from_le_bytes(bytes[0..8].try_into().expect("expected 32 byte hashes")); + } + + fn write_u64(&mut self, _i: u64) { + panic!("This hasher only takes bytes"); + } +} + +impl BuildHasher for IdentityHash { + type Hasher = Self; + + fn build_hasher(&self) -> Self::Hasher { + *self + } +} + #[derive(Debug)] pub struct ReadCacheLookup { root_hash: Bytes32, @@ -28,10 +54,10 @@ pub struct ReadCacheLookup { /// the tree hashes of the contents on the left and right read_stack: Vec<(Bytes32, Bytes32)>, - count: HashMap, + count: HashMap, /// a mapping of tree hashes to `(parent, is_right)` tuples - parent_lookup: HashMap>, + parent_lookup: HashMap, IdentityHash>, } impl Default for ReadCacheLookup { @@ -43,10 +69,12 @@ impl Default for ReadCacheLookup { impl ReadCacheLookup { pub fn new() -> Self { let root_hash = hash_blob(&[1]); - let read_stack = vec![]; - let mut count = HashMap::default(); + let read_stack = Vec::with_capacity(1000); + // all keys in count and parent_lookup are tree-hashes. There's no need + // to hash them again for the hash map + let mut count = HashMap::with_hasher(IdentityHash::default()); count.insert(root_hash, 1); - let parent_lookup = HashMap::default(); + let parent_lookup = HashMap::with_hasher(IdentityHash::default()); Self { root_hash, read_stack, @@ -121,18 +149,28 @@ impl ReadCacheLookup { /// return the list of minimal-length paths to the given hash which will serialize to no larger /// than the given size (or an empty list if no such path exists) pub fn find_paths(&self, id: &Bytes32, serialized_length: u64) -> Vec> { - let mut seen_ids = HashSet::<&Bytes32>::default(); - let mut possible_responses = vec![]; - if serialized_length < 3 { - return possible_responses; + // this function is not cheap. only keep going if there's potential to + // save enough bytes + if serialized_length < 4 { + return vec![]; } - assert!(serialized_length > 2); + + let mut possible_responses = Vec::with_capacity(50); + + // all the values we put in this hash set are themselves sha256 hashes. + // There's no point in hashing the hashes + let mut seen_ids = HashSet::<&Bytes32, IdentityHash>::with_capacity_and_hasher( + 1000, + IdentityHash::default(), + ); + let max_bytes_for_path_encoding = serialized_length - 2; // 1 byte for 0xfe, 1 min byte for savings let max_path_length: usize = (max_bytes_for_path_encoding.saturating_mul(8) - 1) .try_into() .unwrap_or(usize::MAX); seen_ids.insert(id); - let mut partial_paths = vec![(*id, vec![])]; + let mut partial_paths = Vec::with_capacity(500); + partial_paths.push((*id, vec![])); while !partial_paths.is_empty() { let mut new_partial_paths = vec![]; @@ -147,11 +185,11 @@ impl ReadCacheLookup { for (parent, direction) in items.iter() { if *(self.count.get(parent).unwrap_or(&0)) > 0 && !seen_ids.contains(parent) { - let mut new_path = path.clone(); - new_path.push(*direction); - if new_path.len() > max_path_length { + if path.len() + 1 > max_path_length { return possible_responses; } + let mut new_path = path.clone(); + new_path.push(*direction); new_partial_paths.push((*parent, new_path)); } seen_ids.insert(parent); From 00f4c029974cc0476066559e05fdc1172859dcc0 Mon Sep 17 00:00:00 2001 From: arvidn Date: Fri, 20 Dec 2024 11:02:55 +0100 Subject: [PATCH 2/4] use bitset instead of vector to record paths --- Cargo.lock | 40 +++++++++++++++++++++++++++ Cargo.toml | 2 ++ src/serde/read_cache_lookup.rs | 49 +++++++++++++++++++--------------- 3 files changed, 69 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 687d9699..81599226 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -102,6 +102,18 @@ version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b048fb63fd8b5923fc5aa7b340d8e156aec7ec02f0c78fa8a6ddc2613f6f71de" +[[package]] +name = "bitvec" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1bc2832c24239b0141d5674bb9174f9d68a8b5b3f2753311927c172ca46f7e9c" +dependencies = [ + "funty", + "radium", + "tap", + "wyz", +] + [[package]] name = "block-buffer" version = "0.10.4" @@ -320,6 +332,7 @@ dependencies = [ name = "clvmr" version = "0.10.0" dependencies = [ + "bitvec", "chia-bls", "chia-sha2", "criterion", @@ -566,6 +579,12 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" +[[package]] +name = "funty" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6d5a32815ae3f33302d95fdcb2ce17862f8c65363dcfd29360480ba1001fc9c" + [[package]] name = "futures" version = "0.3.30" @@ -1218,6 +1237,12 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "radium" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc33ff2d4973d518d823d61aa239014831e521c75da58e3df4840d3f47749d09" + [[package]] name = "rand" version = "0.8.5" @@ -1520,6 +1545,12 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "tap" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" + [[package]] name = "target-lexicon" version = "0.12.15" @@ -1839,6 +1870,15 @@ dependencies = [ "memchr", ] +[[package]] +name = "wyz" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05f360fc0b24296329c78fda852a1e9ae82de9cf7b27dae4b7f62f118f77b9ed" +dependencies = [ + "tap", +] + [[package]] name = "zeroize" version = "1.8.1" diff --git a/Cargo.toml b/Cargo.toml index 46e4f2ea..a095ed01 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -61,6 +61,7 @@ serde = "1.0.214" serde_json = "1.0.133" clap = "4.5.20" rand_chacha = "0.3.1" +bitvec = "1.0.1" [dependencies] lazy_static = { workspace = true } @@ -70,6 +71,7 @@ num-integer = { workspace = true } chia-bls = { workspace = true } chia-sha2 = { workspace = true } hex-literal = { workspace = true } +bitvec = { workspace = true } # for secp sigs k256 = { version = "0.13.4", features = ["ecdsa"] } p256 = { version = "0.13.2", features = ["ecdsa"] } diff --git a/src/serde/read_cache_lookup.rs b/src/serde/read_cache_lookup.rs index 89930a24..93da29c7 100644 --- a/src/serde/read_cache_lookup.rs +++ b/src/serde/read_cache_lookup.rs @@ -1,3 +1,5 @@ +use bitvec::prelude::*; +use bitvec::vec::BitVec; /// When deserializing a clvm object, a stack of deserialized child objects /// is created, which can be used with back-references. A `ReadCacheLookup` keeps /// track of the state of this stack and all child objects under each root @@ -57,7 +59,7 @@ pub struct ReadCacheLookup { count: HashMap, /// a mapping of tree hashes to `(parent, is_right)` tuples - parent_lookup: HashMap, IdentityHash>, + parent_lookup: HashMap, IdentityHash>, } impl Default for ReadCacheLookup { @@ -95,13 +97,13 @@ impl ReadCacheLookup { *self.count.entry(id).or_insert(0) += 1; *self.count.entry(new_root_hash).or_insert(0) += 1; - let new_parent_to_old_root = (new_root_hash, 0); + let new_parent_to_old_root = (new_root_hash, false); self.parent_lookup .entry(id) .or_default() .push(new_parent_to_old_root); - let new_parent_to_id = (new_root_hash, 1); + let new_parent_to_id = (new_root_hash, true); self.parent_lookup .entry(self.root_hash) .or_default() @@ -136,12 +138,12 @@ impl ReadCacheLookup { self.parent_lookup .entry(left.0) .or_default() - .push((new_root_hash, 0)); + .push((new_root_hash, false)); self.parent_lookup .entry(right.0) .or_default() - .push((new_root_hash, 1)); + .push((new_root_hash, true)); self.push(new_root_hash); } @@ -170,7 +172,7 @@ impl ReadCacheLookup { .unwrap_or(usize::MAX); seen_ids.insert(id); let mut partial_paths = Vec::with_capacity(500); - partial_paths.push((*id, vec![])); + partial_paths.push((*id, BitVec::with_capacity(100))); while !partial_paths.is_empty() { let mut new_partial_paths = vec![]; @@ -223,13 +225,13 @@ impl ReadCacheLookup { /// If `A` => `v` then `[A] + [0]` => `v * 2` and `[A] + [1]` => `v * 2 + 1` /// Then the integer is turned into the minimal-length array of `u8` representing /// that value as an unsigned integer. -fn reversed_path_to_vec_u8(path: &[u8]) -> Vec { +fn reversed_path_to_vec_u8(path: &BitSlice) -> Vec { let byte_count = (path.len() + 1 + 7) >> 3; let mut v = vec![0; byte_count]; let mut index = byte_count - 1; let mut mask: u8 = 1; for p in path.iter().rev() { - if *p != 0 { + if p != false { v[index] |= mask; } mask = { @@ -251,30 +253,33 @@ mod tests { #[test] fn test_path_to_vec_u8() { - assert_eq!(reversed_path_to_vec_u8(&[]), vec!(0b1)); - assert_eq!(reversed_path_to_vec_u8(&[0]), vec!(0b10)); - assert_eq!(reversed_path_to_vec_u8(&[1]), vec!(0b11)); - assert_eq!(reversed_path_to_vec_u8(&[0, 0]), vec!(0b100)); - assert_eq!(reversed_path_to_vec_u8(&[0, 1]), vec!(0b101)); - assert_eq!(reversed_path_to_vec_u8(&[1, 0]), vec!(0b110)); - assert_eq!(reversed_path_to_vec_u8(&[1, 1]), vec!(0b111)); - assert_eq!(reversed_path_to_vec_u8(&[1, 1, 1]), vec!(0b1111)); - assert_eq!(reversed_path_to_vec_u8(&[0, 1, 1, 1]), vec!(0b10111)); - assert_eq!(reversed_path_to_vec_u8(&[1, 0, 1, 1, 1]), vec!(0b110111)); + assert_eq!(reversed_path_to_vec_u8(bits![]), vec!(0b1)); + assert_eq!(reversed_path_to_vec_u8(bits![0]), vec!(0b10)); + assert_eq!(reversed_path_to_vec_u8(bits![1]), vec!(0b11)); + assert_eq!(reversed_path_to_vec_u8(bits![0, 0]), vec!(0b100)); + assert_eq!(reversed_path_to_vec_u8(bits![0, 1]), vec!(0b101)); + assert_eq!(reversed_path_to_vec_u8(bits![1, 0]), vec!(0b110)); + assert_eq!(reversed_path_to_vec_u8(bits![1, 1]), vec!(0b111)); + assert_eq!(reversed_path_to_vec_u8(bits![1, 1, 1]), vec!(0b1111)); + assert_eq!(reversed_path_to_vec_u8(bits![0, 1, 1, 1]), vec!(0b10111)); assert_eq!( - reversed_path_to_vec_u8(&[1, 1, 0, 1, 1, 1]), + reversed_path_to_vec_u8(bits![1, 0, 1, 1, 1]), + vec!(0b110111) + ); + assert_eq!( + reversed_path_to_vec_u8(bits![1, 1, 0, 1, 1, 1]), vec!(0b1110111) ); assert_eq!( - reversed_path_to_vec_u8(&[0, 1, 1, 0, 1, 1, 1]), + reversed_path_to_vec_u8(bits![0, 1, 1, 0, 1, 1, 1]), vec!(0b10110111) ); assert_eq!( - reversed_path_to_vec_u8(&[0, 0, 1, 1, 0, 1, 1, 1]), + reversed_path_to_vec_u8(bits![0, 0, 1, 1, 0, 1, 1, 1]), vec!(0b1, 0b00110111) ); assert_eq!( - reversed_path_to_vec_u8(&[1, 0, 0, 1, 1, 0, 1, 1, 1]), + reversed_path_to_vec_u8(bits![1, 0, 0, 1, 1, 0, 1, 1, 1]), vec!(0b11, 0b00110111) ); } From cc9e03c6d23609ca0b59eb9dec090291dfadb352 Mon Sep 17 00:00:00 2001 From: arvidn Date: Mon, 30 Dec 2024 12:09:46 +0100 Subject: [PATCH 3/4] fix custom hasher to also use a salt --- Cargo.lock | 1 + Cargo.toml | 1 + src/serde/read_cache_lookup.rs | 41 +++++++++++++++++++++++----------- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 81599226..dad6da1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -344,6 +344,7 @@ dependencies = [ "num-integer", "num-traits", "p256", + "rand", "rstest", "sha3", ] diff --git a/Cargo.toml b/Cargo.toml index a095ed01..a026c650 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -77,6 +77,7 @@ k256 = { version = "0.13.4", features = ["ecdsa"] } p256 = { version = "0.13.2", features = ["ecdsa"] } # for keccak256 sha3 = "0.10.8" +rand = { workspace = true } [dev-dependencies] rstest = { workspace = true } diff --git a/src/serde/read_cache_lookup.rs b/src/serde/read_cache_lookup.rs index 93da29c7..711092fd 100644 --- a/src/serde/read_cache_lookup.rs +++ b/src/serde/read_cache_lookup.rs @@ -1,5 +1,6 @@ use bitvec::prelude::*; use bitvec::vec::BitVec; +use rand::Rng; /// When deserializing a clvm object, a stack of deserialized child objects /// is created, which can be used with back-references. A `ReadCacheLookup` keeps /// track of the state of this stack and all child objects under each root @@ -23,7 +24,13 @@ use std::hash::{BuildHasher, Hasher}; use super::bytes32::{hash_blob, hash_blobs, Bytes32}; #[derive(Default, Clone, Copy)] -pub struct IdentityHash(u64); +pub struct IdentityHash(u64, u64); + +impl IdentityHash { + fn new(salt: u64) -> Self { + Self(0, salt) + } +} impl Hasher for IdentityHash { fn finish(&self) -> u64 { @@ -31,7 +38,8 @@ impl Hasher for IdentityHash { } fn write(&mut self, bytes: &[u8]) { - self.0 = u64::from_le_bytes(bytes[0..8].try_into().expect("expected 32 byte hashes")); + self.0 = + u64::from_le_bytes(bytes[0..8].try_into().expect("expected 32 byte hashes")) ^ self.1; } fn write_u64(&mut self, _i: u64) { @@ -39,11 +47,20 @@ impl Hasher for IdentityHash { } } -impl BuildHasher for IdentityHash { - type Hasher = Self; +pub struct RandomState(u64); + +impl RandomState { + fn new() -> Self { + let mut rng = rand::thread_rng(); + Self(rng.gen()) + } +} + +impl BuildHasher for RandomState { + type Hasher = IdentityHash; fn build_hasher(&self) -> Self::Hasher { - *self + IdentityHash::new(self.0) } } @@ -56,10 +73,10 @@ pub struct ReadCacheLookup { /// the tree hashes of the contents on the left and right read_stack: Vec<(Bytes32, Bytes32)>, - count: HashMap, + count: HashMap, /// a mapping of tree hashes to `(parent, is_right)` tuples - parent_lookup: HashMap, IdentityHash>, + parent_lookup: HashMap, RandomState>, } impl Default for ReadCacheLookup { @@ -74,9 +91,9 @@ impl ReadCacheLookup { let read_stack = Vec::with_capacity(1000); // all keys in count and parent_lookup are tree-hashes. There's no need // to hash them again for the hash map - let mut count = HashMap::with_hasher(IdentityHash::default()); + let mut count = HashMap::with_hasher(RandomState::new()); count.insert(root_hash, 1); - let parent_lookup = HashMap::with_hasher(IdentityHash::default()); + let parent_lookup = HashMap::with_hasher(RandomState::new()); Self { root_hash, read_stack, @@ -161,10 +178,8 @@ impl ReadCacheLookup { // all the values we put in this hash set are themselves sha256 hashes. // There's no point in hashing the hashes - let mut seen_ids = HashSet::<&Bytes32, IdentityHash>::with_capacity_and_hasher( - 1000, - IdentityHash::default(), - ); + let mut seen_ids = + HashSet::<&Bytes32, RandomState>::with_capacity_and_hasher(1000, RandomState::new()); let max_bytes_for_path_encoding = serialized_length - 2; // 1 byte for 0xfe, 1 min byte for savings let max_path_length: usize = (max_bytes_for_path_encoding.saturating_mul(8) - 1) From 32bed638800f5bedc19829a84c7d932b8b3294c4 Mon Sep 17 00:00:00 2001 From: arvidn Date: Mon, 30 Dec 2024 12:38:39 +0100 Subject: [PATCH 4/4] move IdentityHasher into its own file --- src/serde/identity_hash.rs | 43 +++++++++++++++++++++++++++ src/serde/mod.rs | 2 ++ src/serde/read_cache_lookup.rs | 54 +++++----------------------------- 3 files changed, 52 insertions(+), 47 deletions(-) create mode 100644 src/serde/identity_hash.rs diff --git a/src/serde/identity_hash.rs b/src/serde/identity_hash.rs new file mode 100644 index 00000000..1a1396cb --- /dev/null +++ b/src/serde/identity_hash.rs @@ -0,0 +1,43 @@ +use rand::Rng; +use std::hash::{BuildHasher, Hasher}; + +#[derive(Default, Clone, Copy)] +pub struct IdentityHash(u64, u64); + +impl IdentityHash { + fn new(salt: u64) -> Self { + Self(0, salt) + } +} + +impl Hasher for IdentityHash { + fn finish(&self) -> u64 { + self.0 + } + + fn write(&mut self, bytes: &[u8]) { + self.0 = + u64::from_le_bytes(bytes[0..8].try_into().expect("expected 32 byte hashes")) ^ self.1; + } + + fn write_u64(&mut self, _i: u64) { + panic!("This hasher only takes bytes"); + } +} + +pub struct RandomState(u64); + +impl Default for RandomState { + fn default() -> Self { + let mut rng = rand::thread_rng(); + Self(rng.gen()) + } +} + +impl BuildHasher for RandomState { + type Hasher = IdentityHash; + + fn build_hasher(&self) -> Self::Hasher { + IdentityHash::new(self.0) + } +} diff --git a/src/serde/mod.rs b/src/serde/mod.rs index f7ebe823..6d90d102 100644 --- a/src/serde/mod.rs +++ b/src/serde/mod.rs @@ -3,6 +3,7 @@ mod de; mod de_br; mod de_tree; mod errors; +mod identity_hash; mod incremental; mod object_cache; mod parse_atom; @@ -19,6 +20,7 @@ mod test; pub use de::node_from_bytes; pub use de_br::{node_from_bytes_backrefs, node_from_bytes_backrefs_record}; pub use de_tree::{parse_triples, ParsedTriple}; +pub use identity_hash::RandomState; pub use incremental::{Serializer, UndoState}; pub use object_cache::{serialized_length, treehash, ObjectCache}; pub use ser::{node_to_bytes, node_to_bytes_limit}; diff --git a/src/serde/read_cache_lookup.rs b/src/serde/read_cache_lookup.rs index 711092fd..87df599c 100644 --- a/src/serde/read_cache_lookup.rs +++ b/src/serde/read_cache_lookup.rs @@ -1,6 +1,6 @@ +use crate::serde::RandomState; use bitvec::prelude::*; use bitvec::vec::BitVec; -use rand::Rng; /// When deserializing a clvm object, a stack of deserialized child objects /// is created, which can be used with back-references. A `ReadCacheLookup` keeps /// track of the state of this stack and all child objects under each root @@ -19,51 +19,9 @@ use rand::Rng; /// /// All hashes correspond to sha256 tree hashes. use std::collections::{HashMap, HashSet}; -use std::hash::{BuildHasher, Hasher}; use super::bytes32::{hash_blob, hash_blobs, Bytes32}; -#[derive(Default, Clone, Copy)] -pub struct IdentityHash(u64, u64); - -impl IdentityHash { - fn new(salt: u64) -> Self { - Self(0, salt) - } -} - -impl Hasher for IdentityHash { - fn finish(&self) -> u64 { - self.0 - } - - fn write(&mut self, bytes: &[u8]) { - self.0 = - u64::from_le_bytes(bytes[0..8].try_into().expect("expected 32 byte hashes")) ^ self.1; - } - - fn write_u64(&mut self, _i: u64) { - panic!("This hasher only takes bytes"); - } -} - -pub struct RandomState(u64); - -impl RandomState { - fn new() -> Self { - let mut rng = rand::thread_rng(); - Self(rng.gen()) - } -} - -impl BuildHasher for RandomState { - type Hasher = IdentityHash; - - fn build_hasher(&self) -> Self::Hasher { - IdentityHash::new(self.0) - } -} - #[derive(Debug)] pub struct ReadCacheLookup { root_hash: Bytes32, @@ -91,9 +49,9 @@ impl ReadCacheLookup { let read_stack = Vec::with_capacity(1000); // all keys in count and parent_lookup are tree-hashes. There's no need // to hash them again for the hash map - let mut count = HashMap::with_hasher(RandomState::new()); + let mut count = HashMap::with_hasher(RandomState::default()); count.insert(root_hash, 1); - let parent_lookup = HashMap::with_hasher(RandomState::new()); + let parent_lookup = HashMap::with_hasher(RandomState::default()); Self { root_hash, read_stack, @@ -178,8 +136,10 @@ impl ReadCacheLookup { // all the values we put in this hash set are themselves sha256 hashes. // There's no point in hashing the hashes - let mut seen_ids = - HashSet::<&Bytes32, RandomState>::with_capacity_and_hasher(1000, RandomState::new()); + let mut seen_ids = HashSet::<&Bytes32, RandomState>::with_capacity_and_hasher( + 1000, + RandomState::default(), + ); let max_bytes_for_path_encoding = serialized_length - 2; // 1 byte for 0xfe, 1 min byte for savings let max_path_length: usize = (max_bytes_for_path_encoding.saturating_mul(8) - 1)