Skip to content

Commit

Permalink
chore: deprecate default bitwidths in the KAMT/AMT
Browse files Browse the repository at this point in the history
1. We don't intentionally use these anywhere.
2. We _do_ unintentionally use these where we
shouldn't (filecoin-project/builtin-actors#1346).

fixes #1828
  • Loading branch information
Stebalien committed Aug 2, 2023
1 parent 0f247eb commit f42072d
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 15 deletions.
14 changes: 7 additions & 7 deletions ipld/hamt/benches/hamt_benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand All @@ -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();
Expand All @@ -67,15 +67,15 @@ 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();
}
let cid = a.flush().unwrap();

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();
}
Expand All @@ -85,15 +85,15 @@ 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();
}
let cid = a.flush().unwrap();

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();
})
});
Expand Down
2 changes: 2 additions & 0 deletions ipld/hamt/src/hamt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand All @@ -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, Error> {
Self::load_with_config(cid, store, Config::default())
}
Expand Down
2 changes: 2 additions & 0 deletions ipld/hamt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
22 changes: 14 additions & 8 deletions ipld/kamt/benches/kamt_benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)))
Expand All @@ -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();
Expand All @@ -85,15 +91,15 @@ 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();
}
let cid = a.flush().unwrap();

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();
}
Expand All @@ -103,15 +109,15 @@ 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();
}
let cid = a.flush().unwrap();

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();
})
});
Expand Down
2 changes: 2 additions & 0 deletions ipld/kamt/src/kamt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand All @@ -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, Error> {
Self::load_with_config(cid, store, Config::default())
}
Expand Down
2 changes: 2 additions & 0 deletions ipld/kamt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit f42072d

Please sign in to comment.