Add associated functions/methods for exposing/loading filter data in BloomFilter + CuckooFilter #125
Conversation
DJAndries
changed the title
There was a problem hiding this comment.
I'm a bit confused by the interfaces: why is there a "create w/ existing" for the bloomfilter and cuckoofilter, but only the cuckoofilter as a "load" method?
I suggest we streamline the interface a bit and add a dump(&self) -> Vec<u8> method to both which includes the full state (for the cuckoofilter, this also includes n_elements) and one to restore it (maybe load(&mut self, data: &[u8])). This way we could later lift this up into a trait and have this working for multiple data structures.
| Self { | ||
| bs: FixedBitSet::with_capacity_and_blocks(m, bitmap), | ||
| k, | ||
| builder: HashIterBuilder::new(m, k, buildhasher), |
There was a problem hiding this comment.
Does this check that the iterator has the correct number of elements for the given parameters?
| for (i, block) in table_succinct_blocks.into_iter().enumerate() { | ||
| assert!( | ||
| i < filter.table.block_len(), | ||
| "existing input table block length must not exceed filter table block length" | ||
| ); | ||
| filter.table.set_block(i, block); | ||
| } | ||
| filter.n_elements = n_elements; |
There was a problem hiding this comment.
Can this just use filter.load_table(...) instead of a duplicate code block?
| self.clear(); | ||
| for (i, value) in table.into_iter().enumerate() { | ||
| let i = i as u64; | ||
| assert!( |
There was a problem hiding this comment.
There should be a should_panic test that triggers this assertion.
| /// and existing element count. | ||
| pub fn load_table<I: IntoIterator<Item = u64>>(&mut self, table: I, n_elements: usize) { | ||
| self.clear(); | ||
| for (i, value) in table.into_iter().enumerate() { |
There was a problem hiding this comment.
This should probably validate that the iterator has the correct length because this depends on the filter parameters.
No description provided.