From f42072de19862fd54c44b9ae9c2391632d6d9de3 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Wed, 2 Aug 2023 16:52:26 -0700 Subject: [PATCH] chore: deprecate default bitwidths in the KAMT/AMT 1. We don't intentionally use these anywhere. 2. We _do_ unintentionally use these where we shouldn't (https://github.com/filecoin-project/builtin-actors/issues/1346). fixes https://github.com/filecoin-project/ref-fvm/issues/1828 --- ipld/hamt/benches/hamt_benchmark.rs | 14 +++++++------- ipld/hamt/src/hamt.rs | 2 ++ ipld/hamt/src/lib.rs | 2 ++ ipld/kamt/benches/kamt_benchmark.rs | 22 ++++++++++++++-------- ipld/kamt/src/kamt.rs | 2 ++ ipld/kamt/src/lib.rs | 2 ++ 6 files changed, 29 insertions(+), 15 deletions(-) diff --git a/ipld/hamt/benches/hamt_benchmark.rs b/ipld/hamt/benches/hamt_benchmark.rs index 3c63b1b1a..a0092ed8d 100644 --- a/ipld/hamt/benches/hamt_benchmark.rs +++ b/ipld/hamt/benches/hamt_benchmark.rs @@ -38,7 +38,7 @@ fn insert(c: &mut Criterion) { c.bench_function("HAMT bulk insert (no flush)", |b| { b.iter(|| { let db = fvm_ipld_blockstore::MemoryBlockstore::default(); - let mut a = Hamt::<_, _>::new(&db); + let mut a = Hamt::<_, _>::new_with_bit_width(&db, 5); for i in 0..black_box(ITEM_COUNT) { a.set(black_box(vec![i; 20].into()), black_box(BenchData::new(i))) @@ -52,11 +52,11 @@ fn insert_load_flush(c: &mut Criterion) { c.bench_function("HAMT bulk insert with flushing and loading", |b| { b.iter(|| { let db = fvm_ipld_blockstore::MemoryBlockstore::default(); - let mut empt = Hamt::<_, ()>::new(&db); + let mut empt = Hamt::<_, ()>::new_with_bit_width(&db, 5); let mut cid = empt.flush().unwrap(); for i in 0..black_box(ITEM_COUNT) { - let mut a = Hamt::<_, _>::load(&cid, &db).unwrap(); + let mut a = Hamt::<_, _>::load_with_bit_width(&cid, &db, 5).unwrap(); a.set(black_box(vec![i; 20].into()), black_box(BenchData::new(i))) .unwrap(); cid = a.flush().unwrap(); @@ -67,7 +67,7 @@ fn insert_load_flush(c: &mut Criterion) { fn delete(c: &mut Criterion) { let db = fvm_ipld_blockstore::MemoryBlockstore::default(); - let mut a = Hamt::<_, _>::new(&db); + let mut a = Hamt::<_, _>::new_with_bit_width(&db, 5); for i in 0..black_box(ITEM_COUNT) { a.set(vec![i; 20].into(), BenchData::new(i)).unwrap(); } @@ -75,7 +75,7 @@ fn delete(c: &mut Criterion) { c.bench_function("HAMT deleting all nodes", |b| { b.iter(|| { - let mut a = Hamt::<_, BenchData>::load(&cid, &db).unwrap(); + let mut a = Hamt::<_, BenchData>::load_with_bit_width(&cid, &db, 5).unwrap(); for i in 0..black_box(ITEM_COUNT) { a.delete(black_box([i; 20].as_ref())).unwrap(); } @@ -85,7 +85,7 @@ fn delete(c: &mut Criterion) { fn for_each(c: &mut Criterion) { let db = fvm_ipld_blockstore::MemoryBlockstore::default(); - let mut a = Hamt::<_, _>::new(&db); + let mut a = Hamt::<_, _>::new_with_bit_width(&db, 5); for i in 0..black_box(ITEM_COUNT) { a.set(vec![i; 20].into(), BenchData::new(i)).unwrap(); } @@ -93,7 +93,7 @@ fn for_each(c: &mut Criterion) { c.bench_function("HAMT for_each function", |b| { b.iter(|| { - let a = Hamt::<_, _>::load(&cid, &db).unwrap(); + let a = Hamt::<_, _>::load_with_bit_width(&cid, &db, 5).unwrap(); black_box(a).for_each(|_k, _v: &BenchData| Ok(())).unwrap(); }) }); diff --git a/ipld/hamt/src/hamt.rs b/ipld/hamt/src/hamt.rs index 8fbc5a586..5fe69b681 100644 --- a/ipld/hamt/src/hamt.rs +++ b/ipld/hamt/src/hamt.rs @@ -80,6 +80,7 @@ where Ver: Version, H: HashAlgorithm, { + #[deprecated = "specify a bit-width explicitly"] pub fn new(store: BS) -> Self { Self::new_with_config(store, Config::default()) } @@ -106,6 +107,7 @@ where } /// Lazily instantiate a hamt from this root Cid. + #[deprecated = "specify a bit-width explicitly"] pub fn load(cid: &Cid, store: BS) -> Result { Self::load_with_config(cid, store, Config::default()) } diff --git a/ipld/hamt/src/lib.rs b/ipld/hamt/src/lib.rs index 2bb6567c5..36d839979 100644 --- a/ipld/hamt/src/lib.rs +++ b/ipld/hamt/src/lib.rs @@ -28,6 +28,7 @@ pub use self::hash::*; pub use self::hash_algorithm::*; /// Default bit width for indexing a hash at each depth level +#[deprecated] const DEFAULT_BIT_WIDTH: u32 = 8; /// Configuration options for a HAMT instance. @@ -63,6 +64,7 @@ pub struct Config { impl Default for Config { fn default() -> Self { Self { + #[allow(deprecated)] bit_width: DEFAULT_BIT_WIDTH, min_data_depth: 0, max_array_width: 3, diff --git a/ipld/kamt/benches/kamt_benchmark.rs b/ipld/kamt/benches/kamt_benchmark.rs index 2628c8f9e..d52c870f2 100644 --- a/ipld/kamt/benches/kamt_benchmark.rs +++ b/ipld/kamt/benches/kamt_benchmark.rs @@ -9,10 +9,16 @@ use std::borrow::Cow; use criterion::{black_box, criterion_group, criterion_main, Criterion}; use fvm_ipld_blockstore::MemoryBlockstore; use fvm_ipld_encoding::tuple::*; -use fvm_ipld_kamt::{AsHashedKey, HashedKey, Kamt}; +use fvm_ipld_kamt::{AsHashedKey, Config, HashedKey, Kamt}; const ITEM_COUNT: u8 = 40; +const TEST_CONFIG: Config = Config { + bit_width: 5, + min_data_depth: 0, + max_array_width: 3, +}; + // Struct to simulate a reasonable amount of data per value into the amt #[derive(Clone, Serialize_tuple, Deserialize_tuple, PartialEq)] struct BenchData { @@ -56,7 +62,7 @@ fn insert(c: &mut Criterion) { c.bench_function("KAMT bulk insert (no flush)", |b| { b.iter(|| { let db = fvm_ipld_blockstore::MemoryBlockstore::default(); - let mut a = BKamt::new(&db); + let mut a = BKamt::new_with_config(&db, Config::default()); for i in 0..black_box(ITEM_COUNT) { a.set(black_box(vec![i; 20]), black_box(BenchData::new(i))) @@ -70,11 +76,11 @@ fn insert_load_flush(c: &mut Criterion) { c.bench_function("KAMT bulk insert with flushing and loading", |b| { b.iter(|| { let db = fvm_ipld_blockstore::MemoryBlockstore::default(); - let mut empt = BKamt::new(&db); + let mut empt = BKamt::new_with_config(&db, TEST_CONFIG); let mut cid = empt.flush().unwrap(); for i in 0..black_box(ITEM_COUNT) { - let mut a = BKamt::load(&cid, &db).unwrap(); + let mut a = BKamt::load_with_config(&cid, &db, TEST_CONFIG).unwrap(); a.set(black_box(vec![i; 20]), black_box(BenchData::new(i))) .unwrap(); cid = a.flush().unwrap(); @@ -85,7 +91,7 @@ fn insert_load_flush(c: &mut Criterion) { fn delete(c: &mut Criterion) { let db = fvm_ipld_blockstore::MemoryBlockstore::default(); - let mut a = BKamt::new(&db); + let mut a = BKamt::new_with_config(&db, TEST_CONFIG); for i in 0..black_box(ITEM_COUNT) { a.set(vec![i; 20], BenchData::new(i)).unwrap(); } @@ -93,7 +99,7 @@ fn delete(c: &mut Criterion) { c.bench_function("KAMT deleting all nodes", |b| { b.iter(|| { - let mut a = BKamt::load(&cid, &db).unwrap(); + let mut a = BKamt::load_with_config(&cid, &db, TEST_CONFIG).unwrap(); for i in 0..black_box(ITEM_COUNT) { a.delete(black_box(vec![i; 20].as_ref())).unwrap(); } @@ -103,7 +109,7 @@ fn delete(c: &mut Criterion) { fn for_each(c: &mut Criterion) { let db = fvm_ipld_blockstore::MemoryBlockstore::default(); - let mut a = BKamt::new(&db); + let mut a = BKamt::new_with_config(&db, TEST_CONFIG); for i in 0..black_box(ITEM_COUNT) { a.set(vec![i; 20], BenchData::new(i)).unwrap(); } @@ -111,7 +117,7 @@ fn for_each(c: &mut Criterion) { c.bench_function("KAMT for_each function", |b| { b.iter(|| { - let a = BKamt::load(&cid, &db).unwrap(); + let a = BKamt::load_with_config(&cid, &db, TEST_CONFIG).unwrap(); black_box(a).for_each(|_k, _v: &BenchData| Ok(())).unwrap(); }) }); diff --git a/ipld/kamt/src/kamt.rs b/ipld/kamt/src/kamt.rs index dd28b90f0..64ff7c13c 100644 --- a/ipld/kamt/src/kamt.rs +++ b/ipld/kamt/src/kamt.rs @@ -67,6 +67,7 @@ where V: Serialize + DeserializeOwned, BS: Blockstore, { + #[deprecated = "specify config with an explicit bit-width"] pub fn new(store: BS) -> Self { Self::new_with_config(store, Config::default()) } @@ -81,6 +82,7 @@ where } /// Lazily instantiate a Kamt from this root Cid. + #[deprecated = "specify config with an explicit bit-width"] pub fn load(cid: &Cid, store: BS) -> Result { Self::load_with_config(cid, store, Config::default()) } diff --git a/ipld/kamt/src/lib.rs b/ipld/kamt/src/lib.rs index e4acdc359..c9fb0e626 100644 --- a/ipld/kamt/src/lib.rs +++ b/ipld/kamt/src/lib.rs @@ -31,6 +31,7 @@ pub use self::error::Error; pub use self::kamt::Kamt; /// Default bit width for indexing a hash at each depth level +#[deprecated] const DEFAULT_BIT_WIDTH: u32 = 8; /// Configuration options for a KAMT instance. @@ -63,6 +64,7 @@ pub struct Config { impl Default for Config { fn default() -> Self { Self { + #[allow(deprecated)] bit_width: DEFAULT_BIT_WIDTH, min_data_depth: 0, max_array_width: 3,