diff --git a/benchmarks/results.yml b/benchmarks/results.yml index ab66c5d7..7559d85f 100644 --- a/benchmarks/results.yml +++ b/benchmarks/results.yml @@ -80,12 +80,12 @@ btreemap_get_blob_8_1024_v2: stable_memory_size: 139 btreemap_get_blob_8_u64: measurements: - instructions: 426482546 + instructions: 426482525 node_load_v1: 294602061 stable_memory_size: 7 btreemap_get_blob_8_u64_v2: measurements: - instructions: 523481237 + instructions: 523481258 node_load_v2: 383093469 stable_memory_size: 5 btreemap_get_u64_blob_8: @@ -110,141 +110,141 @@ btreemap_get_u64_u64_v2: stable_memory_size: 7 btreemap_insert_10mib_values: measurements: - instructions: 153745217 + instructions: 153646946 node_load_v2: 10377468 - node_save_v2: 121431267 + node_save_v2: 121462145 stable_memory_size: 33 btreemap_insert_blob_128_1024: measurements: - instructions: 1729768012 + instructions: 1728428025 node_load_v1: 818923030 node_save_v1: 424336331 stable_memory_size: 261 btreemap_insert_blob_128_1024_v2: measurements: - instructions: 1885312232 + instructions: 1884192245 node_load_v2: 916598797 node_save_v2: 470597559 stable_memory_size: 196 btreemap_insert_blob_16_1024: measurements: - instructions: 1176903332 + instructions: 1175563345 node_load_v1: 362914338 node_save_v1: 403232566 stable_memory_size: 216 btreemap_insert_blob_16_1024_v2: measurements: - instructions: 1295167866 + instructions: 1294047879 node_load_v2: 440404989 node_save_v2: 448435449 stable_memory_size: 162 btreemap_insert_blob_256_1024: measurements: - instructions: 2255269553 + instructions: 2253929566 node_load_v1: 1272224795 node_save_v1: 429936500 stable_memory_size: 293 btreemap_insert_blob_256_1024_v2: measurements: - instructions: 2415876387 + instructions: 2414756400 node_load_v2: 1368714490 node_save_v2: 476169063 stable_memory_size: 220 btreemap_insert_blob_32_1024: measurements: - instructions: 1224078359 + instructions: 1222738372 node_load_v1: 390780802 node_save_v1: 415591575 stable_memory_size: 231 btreemap_insert_blob_32_1024_v2: measurements: - instructions: 1345546619 + instructions: 1344426632 node_load_v2: 477582317 node_save_v2: 461048387 stable_memory_size: 174 btreemap_insert_blob_4_1024: measurements: - instructions: 938459480 + instructions: 937079494 node_load_v1: 246219371 node_save_v1: 369888833 stable_memory_size: 124 btreemap_insert_blob_4_1024_v2: measurements: - instructions: 1032956555 + instructions: 1031796569 node_load_v2: 302721338 node_save_v2: 410342602 stable_memory_size: 93 btreemap_insert_blob_512_1024: measurements: - instructions: 3306266369 + instructions: 3304926382 node_load_v1: 2162164471 node_save_v1: 447224687 stable_memory_size: 352 btreemap_insert_blob_512_1024_v2: measurements: - instructions: 3438950584 + instructions: 3437830597 node_load_v2: 2249806809 node_save_v2: 493373365 stable_memory_size: 264 btreemap_insert_blob_64_1024: measurements: - instructions: 1463634810 + instructions: 1462294823 node_load_v1: 595998298 node_save_v1: 418506217 stable_memory_size: 246 btreemap_insert_blob_64_1024_v2: measurements: - instructions: 1606159890 + instructions: 1605039903 node_load_v2: 692239035 node_save_v2: 464486004 stable_memory_size: 184 btreemap_insert_blob_8_1024: measurements: - instructions: 1084454578 + instructions: 1083074595 node_load_v1: 286595797 node_save_v1: 394091363 stable_memory_size: 184 btreemap_insert_blob_8_1024_v2: measurements: - instructions: 1183662633 + instructions: 1182502650 node_load_v2: 344998681 node_save_v2: 437846838 stable_memory_size: 139 btreemap_insert_blob_8_u64: measurements: - instructions: 684209106 - node_load_v1: 277937358 - node_save_v1: 189165744 + instructions: 678857141 + node_load_v1: 279373875 + node_save_v1: 186568536 stable_memory_size: 7 btreemap_insert_blob_8_u64_v2: measurements: - instructions: 810706169 - node_load_v2: 358652286 - node_save_v2: 236812122 + instructions: 794004499 + node_load_v2: 347638079 + node_save_v2: 235014746 stable_memory_size: 5 btreemap_insert_u64_blob_8: measurements: - instructions: 749642151 - node_load_v1: 276866073 - node_save_v1: 255566331 + instructions: 742981445 + node_load_v1: 276551505 + node_save_v1: 256838739 stable_memory_size: 8 btreemap_insert_u64_blob_8_v2: measurements: - instructions: 821061744 - node_load_v2: 320684381 - node_save_v2: 287579894 + instructions: 814978537 + node_load_v2: 320369042 + node_save_v2: 289170404 stable_memory_size: 6 btreemap_insert_u64_u64: measurements: - instructions: 771295973 - node_load_v1: 274520203 - node_save_v1: 266429035 + instructions: 758994729 + node_load_v1: 271496229 + node_save_v1: 266589945 stable_memory_size: 8 btreemap_insert_u64_u64_v2: measurements: - instructions: 851451139 - node_load_v2: 324640651 - node_save_v2: 299725270 + instructions: 836845668 + node_load_v2: 320572126 + node_save_v2: 298663826 stable_memory_size: 7 btreemap_remove_blob_128_1024: measurements: @@ -344,7 +344,7 @@ btreemap_remove_blob_8_1024_v2: stable_memory_size: 139 btreemap_remove_blob_8_u64: measurements: - instructions: 903169486 + instructions: 903169465 node_load_v1: 318747944 node_save_v1: 301798231 stable_memory_size: 7 @@ -356,27 +356,27 @@ btreemap_remove_blob_8_u64_v2: stable_memory_size: 5 btreemap_remove_u64_blob_8: measurements: - instructions: 1072102455 + instructions: 1076955435 node_load_v1: 313757215 - node_save_v1: 457512144 + node_save_v1: 459748260 stable_memory_size: 8 btreemap_remove_u64_blob_8_v2: measurements: - instructions: 1189636395 + instructions: 1195048404 node_load_v2: 366667638 - node_save_v2: 518185171 + node_save_v2: 520980316 stable_memory_size: 6 btreemap_remove_u64_u64: measurements: - instructions: 1109353494 + instructions: 1114260609 node_load_v1: 314148033 - node_save_v1: 482917665 + node_save_v1: 485183169 stable_memory_size: 8 btreemap_remove_u64_u64_v2: measurements: - instructions: 1243004765 + instructions: 1248478277 node_load_v2: 374499836 - node_save_v2: 551307909 + node_save_v2: 554139789 stable_memory_size: 7 memory_manager_baseline: measurements: diff --git a/src/btreemap.rs b/src/btreemap.rs index 21d416f6..da68057d 100644 --- a/src/btreemap.rs +++ b/src/btreemap.rs @@ -300,34 +300,7 @@ where /// key.to_bytes().len() <= max_size(Key) /// value.to_bytes().len() <= max_size(Value) pub fn insert(&mut self, key: K, value: V) -> Option { - let key_bytes = key.to_bytes(); - let value_bytes = value.to_bytes(); - - match self.version { - Version::V1(DerivedPageSize { - max_key_size, - max_value_size, - }) => { - assert!( - key_bytes.len() <= max_key_size as usize, - "Key is too large. Expected <= {} bytes, found {} bytes", - max_key_size, - key_bytes.len() - ); - - assert!( - value_bytes.len() <= max_value_size as usize, - "Value is too large. Expected <= {} bytes, found {} bytes", - max_value_size, - value_bytes.len() - ); - } - Version::V2 { .. } => { - // Nothing to assert. - } - } - - let value = value_bytes.to_vec(); + let value = value.to_bytes_checked().to_vec(); let root = if self.root_addr == NULL { // No root present. Allocate one. @@ -2582,56 +2555,107 @@ mod test { }); } + // A buggy implementation of storable where the max size is smaller than the serialized size. + #[derive(Clone, Ord, PartialOrd, Eq, PartialEq)] + struct BuggyStruct; + impl crate::Storable for BuggyStruct { + fn to_bytes(&self) -> Cow<[u8]> { + Cow::Borrowed(&[1, 2, 3, 4]) + } + + fn from_bytes(_: Cow<[u8]>) -> Self { + unimplemented!(); + } + + const BOUND: StorableBound = StorableBound::Bounded { + max_size: 1, + is_fixed_size: false, + }; + } + #[test] - #[should_panic(expected = "Key is too large. Expected <= 0 bytes, found 4 bytes")] - fn panics_if_key_is_too_large() { + #[should_panic(expected = "max_key_size must be <= 0")] + fn panics_if_max_key_grows() { + let mem = make_memory(); + let btree: BTreeMap<(), (), _> = BTreeMap::init(mem); + btree.save(); + #[derive(Clone, Ord, PartialOrd, Eq, PartialEq)] struct K; impl crate::Storable for K { fn to_bytes(&self) -> Cow<[u8]> { - Cow::Borrowed(&[1, 2, 3, 4]) + Cow::Borrowed(&[1]) } fn from_bytes(_: Cow<[u8]>) -> Self { unimplemented!(); } - // A buggy implementation where the max_size is smaller than what Storable::to_bytes() - // returns. const BOUND: StorableBound = StorableBound::Bounded { - max_size: 0, + max_size: 1, is_fixed_size: false, }; } - let mut btree: BTreeMap = BTreeMap::init(make_memory()); - btree.insert(K, ()); + // Reload the BTree but with a key that has a larger max_size. Should panic. + let _: BTreeMap = BTreeMap::init(btree.into_memory()); } #[test] - #[should_panic(expected = "Value is too large. Expected <= 0 bytes, found 4 bytes")] - fn panics_if_value_is_too_large() { + #[should_panic(expected = "expected an element with length <= 1 bytes, but found 4")] + fn v1_panics_if_key_is_bigger_than_max_size() { + let mut btree = BTreeMap::init(make_memory()); + btree.insert(BuggyStruct, ()); + } + + #[test] + #[should_panic(expected = "expected an element with length <= 1 bytes, but found 4")] + fn v2_panics_if_key_is_bigger_than_max_size() { + let mut btree = BTreeMap::init_v2(make_memory()); + btree.insert(BuggyStruct, ()); + } + + #[test] + #[should_panic(expected = "max_value_size must be <= 0")] + fn v1_panics_if_value_is_too_large() { + // Initialize a btreemap where the key and value both have a max size of zero. + let mem = make_memory(); + let btree: BTreeMap<(), (), _> = BTreeMap::init(mem); + btree.save(); + #[derive(Clone, Ord, PartialOrd, Eq, PartialEq)] struct V; impl crate::Storable for V { fn to_bytes(&self) -> Cow<[u8]> { - Cow::Borrowed(&[1, 2, 3, 4]) + Cow::Borrowed(&[1]) } fn from_bytes(_: Cow<[u8]>) -> Self { unimplemented!(); } - // A buggy implementation where the max_size is smaller than what Storable::to_bytes() - // returns. const BOUND: StorableBound = StorableBound::Bounded { - max_size: 0, + max_size: 1, is_fixed_size: false, }; } - let mut btree: BTreeMap<(), V, _> = BTreeMap::init(make_memory()); - btree.insert((), V); + // Reload the BTree but with a value that has a larger max_size. Should panic. + let _: BTreeMap<(), V, _> = BTreeMap::init(btree.into_memory()); + } + + #[test] + #[should_panic(expected = "expected an element with length <= 1 bytes, but found 4")] + fn v1_panics_if_value_is_bigger_than_max_size() { + let mut btree = BTreeMap::init(make_memory()); + btree.insert((), BuggyStruct); + } + + #[test] + #[should_panic(expected = "expected an element with length <= 1 bytes, but found 4")] + fn v2_panics_if_value_is_bigger_than_max_size() { + let mut btree = BTreeMap::init(make_memory()); + btree.insert((), BuggyStruct); } // To generate the memory dump file for the current version: diff --git a/src/btreemap/node/v1.rs b/src/btreemap/node/v1.rs index 5e12c1bd..373e5b49 100644 --- a/src/btreemap/node/v1.rs +++ b/src/btreemap/node/v1.rs @@ -166,7 +166,7 @@ impl Node { // Write the entries. for (idx, key) in self.keys.iter().enumerate() { // Write the size of the key. - let key_bytes = key.to_bytes(); + let key_bytes = key.to_bytes_checked(); write_u32(memory, self.address + offset, key_bytes.len() as u32); offset += U32_SIZE; diff --git a/src/btreemap/node/v2.rs b/src/btreemap/node/v2.rs index 96bb85f3..3b9a337f 100644 --- a/src/btreemap/node/v2.rs +++ b/src/btreemap/node/v2.rs @@ -248,7 +248,7 @@ impl Node { // Write the keys. for key in self.keys.iter() { - let key_bytes = key.to_bytes(); + let key_bytes = key.to_bytes_checked(); // Write the size of the key if it isn't fixed in size. if !is_fixed_size::() { diff --git a/src/storable.rs b/src/storable.rs index ccb4a0b0..12459e3b 100644 --- a/src/storable.rs +++ b/src/storable.rs @@ -18,6 +18,35 @@ pub trait Storable { /// The size bounds of the type. const BOUND: Bound; + + /// Like `to_bytes`, but includes additional checks to ensure the element's serialized bytes + /// are within the element's bounds. + fn to_bytes_checked(&self) -> Cow<[u8]> { + let bytes = self.to_bytes(); + if let Bound::Bounded { + max_size, + is_fixed_size, + } = Self::BOUND + { + if is_fixed_size { + assert_eq!( + bytes.len(), + max_size as usize, + "expected a fixed-size element with length {} bytes, but found {} bytes", + max_size, + bytes.len() + ); + } else { + assert!( + bytes.len() <= max_size as usize, + "expected an element with length <= {} bytes, but found {} bytes", + max_size, + bytes.len() + ); + } + } + bytes + } } /// States whether the type's size is bounded or unbounded. diff --git a/src/storable/tests.rs b/src/storable/tests.rs index b4ff618a..584af041 100644 --- a/src/storable/tests.rs +++ b/src/storable/tests.rs @@ -36,3 +36,107 @@ proptest! { prop_assert_eq!(v, Storable::from_bytes(v.to_bytes())); } } + +#[test] +#[should_panic(expected = "expected an element with length <= 1 bytes, but found 4")] +fn to_bytes_checked_element_too_long_panics() { + struct X; + impl Storable for X { + fn to_bytes(&self) -> Cow<[u8]> { + Cow::Borrowed(&[1, 2, 3, 4]) + } + + fn from_bytes(_: Cow<[u8]>) -> Self { + unimplemented!(); + } + + const BOUND: Bound = Bound::Bounded { + max_size: 1, + is_fixed_size: false, + }; + } + + X.to_bytes_checked(); +} + +#[test] +fn to_bytes_checked_unbounded_element_no_panic() { + struct X; + impl Storable for X { + fn to_bytes(&self) -> Cow<[u8]> { + Cow::Borrowed(&[1, 2, 3, 4]) + } + + fn from_bytes(_: Cow<[u8]>) -> Self { + unimplemented!(); + } + + const BOUND: Bound = Bound::Unbounded; + } + + assert_eq!(X.to_bytes_checked(), Cow::Borrowed(&[1, 2, 3, 4])); +} + +#[test] +fn to_bytes_checked_element_correct_size_no_panic() { + struct X; + impl Storable for X { + fn to_bytes(&self) -> Cow<[u8]> { + Cow::Borrowed(&[1, 2, 3, 4]) + } + + fn from_bytes(_: Cow<[u8]>) -> Self { + unimplemented!(); + } + + const BOUND: Bound = Bound::Bounded { + max_size: 4, + is_fixed_size: false, + }; + } + + assert_eq!(X.to_bytes_checked(), Cow::Borrowed(&[1, 2, 3, 4])); +} + +#[test] +#[should_panic(expected = "expected a fixed-size element with length 5 bytes, but found 4")] +fn to_bytes_checked_fixed_element_wrong_size_panics() { + struct X; + impl Storable for X { + fn to_bytes(&self) -> Cow<[u8]> { + Cow::Borrowed(&[1, 2, 3, 4]) + } + + fn from_bytes(_: Cow<[u8]>) -> Self { + unimplemented!(); + } + + const BOUND: Bound = Bound::Bounded { + max_size: 5, + is_fixed_size: true, + }; + } + + X.to_bytes_checked(); +} + +#[test] +fn to_bytes_checked_fixed_element_correct_size_no_panic() { + struct X; + impl Storable for X { + fn to_bytes(&self) -> Cow<[u8]> { + Cow::Borrowed(&[1, 2, 3, 4, 5]) + } + + fn from_bytes(_: Cow<[u8]>) -> Self { + unimplemented!(); + } + + const BOUND: Bound = Bound::Bounded { + max_size: 5, + is_fixed_size: true, + }; + } + + assert_eq!(X.to_bytes_checked(), Cow::Borrowed(&[1, 2, 3, 4, 5])); +}