Remove methods that allocate, prefer iterators instead

src/core/benches/minhash.rs

@@ -107,5 +107,161 @@ fn intersection(c: &mut Criterion) {
     });
 }
 
-criterion_group!(minhash, intersection);
+fn jaccard(c: &mut Criterion) {
+    let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+    filename.push("../../tests/test-data/gather-abund/genome-s10.fa.gz.sig");
+    let file = File::open(filename).unwrap();
+    let reader = BufReader::new(file);
+    let mut sigs: Vec<Signature> = serde_json::from_reader(reader).expect("Loading error");
+    let mh = if let Sketch::MinHash(mh) = &sigs.swap_remove(0).sketches()[0] {
+        mh.clone()
+    } else {
+        unimplemented!()
+    };
+
+    let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+    filename.push("../../tests/test-data/gather-abund/genome-s11.fa.gz.sig");
+    let file = File::open(filename).unwrap();
+    let reader = BufReader::new(file);
+    let mut sigs: Vec<Signature> = serde_json::from_reader(reader).expect("Loading error");
+    let mh2 = if let Sketch::MinHash(mh) = &sigs.swap_remove(0).sketches()[0] {
+        mh.clone()
+    } else {
+        unimplemented!()
+    };
+
+    let mut group = c.benchmark_group("minhash");
+    group.sample_size(10);
+
+    group.bench_function("jaccard", |b| {
+        b.iter(|| {
+            mh.jaccard(&mh2).unwrap();
+        });
+    });
+
+    let mut mh1 = KmerMinHash::builder()
+        .num(0)
+        .max_hash(1_000_000)
+        .ksize(21)
+        .build();
+    let mut mh2 = KmerMinHash::builder()
+        .num(0)
+        .max_hash(1_000_000)
+        .ksize(21)
+        .build();
+
+    let mut mh1_btree = KmerMinHashBTree::builder()
+        .num(0)
+        .max_hash(1_000_000)
+        .ksize(21)
+        .build();
+    let mut mh2_btree = KmerMinHashBTree::builder()
+        .num(0)
+        .max_hash(1_000_000)
+        .ksize(21)
+        .build();
+
+    for i in 0..=1_000_000 {
+        if i % 2 == 0 {
+            mh1.add_hash(i);
+            mh1_btree.add_hash(i);
+        }
+        if i % 45 == 0 {
+            mh2.add_hash(i);
+            mh2_btree.add_hash(i);
+        }
+    }
+
+    group.bench_function("large jaccard", |b| {
+        b.iter(|| {
+            mh1.jaccard(&mh2).unwrap();
+        });
+    });
+
+    group.bench_function("large jaccard btree", |b| {
+        b.iter(|| {
+            mh1_btree.jaccard(&mh2_btree).unwrap();
+        });
+    });
+}
+
+fn count_common(c: &mut Criterion) {
+    let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+    filename.push("../../tests/test-data/gather-abund/genome-s10.fa.gz.sig");
+    let file = File::open(filename).unwrap();
+    let reader = BufReader::new(file);
+    let mut sigs: Vec<Signature> = serde_json::from_reader(reader).expect("Loading error");
+    let mh = if let Sketch::MinHash(mh) = &sigs.swap_remove(0).sketches()[0] {
+        mh.clone()
+    } else {
+        unimplemented!()
+    };
+
+    let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+    filename.push("../../tests/test-data/gather-abund/genome-s11.fa.gz.sig");
+    let file = File::open(filename).unwrap();
+    let reader = BufReader::new(file);
+    let mut sigs: Vec<Signature> = serde_json::from_reader(reader).expect("Loading error");
+    let mh2 = if let Sketch::MinHash(mh) = &sigs.swap_remove(0).sketches()[0] {
+        mh.clone()
+    } else {
+        unimplemented!()
+    };
+
+    let mut group = c.benchmark_group("minhash");
+    group.sample_size(10);
+
+    group.bench_function("count_common", |b| {
+        b.iter(|| {
+            mh.count_common(&mh2, false).unwrap();
+        });
+    });
+
+    let mut mh1 = KmerMinHash::builder()
+        .num(0)
+        .max_hash(1_000_000)
+        .ksize(21)
+        .build();
+    let mut mh2 = KmerMinHash::builder()
+        .num(0)
+        .max_hash(1_000_000)
+        .ksize(21)
+        .build();
+
+    let mut mh1_btree = KmerMinHashBTree::builder()
+        .num(0)
+        .max_hash(1_000_000)
+        .ksize(21)
+        .build();
+    let mut mh2_btree = KmerMinHashBTree::builder()
+        .num(0)
+        .max_hash(1_000_000)
+        .ksize(21)
+        .build();
+
+    for i in 0..=1_000_000 {
+        if i % 2 == 0 {
+            mh1.add_hash(i);
+            mh1_btree.add_hash(i);
+        }
+        if i % 45 == 0 {
+            mh2.add_hash(i);
+            mh2_btree.add_hash(i);
+        }
+    }
+
+    group.bench_function("large count_common", |b| {
+        b.iter(|| {
+            mh1.count_common(&mh2, false).unwrap();
+        });
+    });
+
+    group.bench_function("large count_common btree", |b| {
+        b.iter(|| {
+            mh1_btree.count_common(&mh2_btree, false).unwrap();
+        });
+    });
+}
+
+criterion_group!(minhash, intersection, jaccard, count_common);
 criterion_main!(minhash);

src/core/src/ffi/minhash.rs

@@ -8,6 +8,7 @@ use crate::ffi::HashFunctions;
 use crate::signature::SeqToHashes;
 use crate::signature::SigsTrait;
 use crate::sketch::minhash::KmerMinHash;
+use crate::HashIntoType;
 
 pub struct SourmashKmerMinHash;
 
@@ -206,7 +207,7 @@ pub unsafe extern "C" fn kmerminhash_remove_many(
 ffi_fn! {
 unsafe fn kmerminhash_get_mins(ptr: *const SourmashKmerMinHash, size: *mut usize) -> Result<*const u64> {
     let mh = SourmashKmerMinHash::as_rust(ptr);
-    let output = mh.mins();
+    let output: Vec<HashIntoType> = mh.iter_mins().copied().collect();
     *size = output.len();
 
     // FIXME: make a SourmashSlice_u64 type?
@@ -434,7 +435,7 @@ unsafe fn kmerminhash_intersection(ptr: *const SourmashKmerMinHash, other: *cons
     let isect = mh.intersection(other_mh)?;
     let mut new_mh = mh.clone();
     new_mh.clear();
-    new_mh.add_many(&isect.0)?;
+    new_mh.add_many(isect.0.iter().copied())?;
 
     Ok(SourmashKmerMinHash::from_rust(new_mh))
 }

src/core/src/ffi/signature.rs

@@ -202,7 +202,7 @@ ffi_fn! {
 unsafe fn signature_get_mhs(ptr: *const SourmashSignature, size: *mut usize) -> Result<*mut *mut SourmashKmerMinHash> {
     let sig = SourmashSignature::as_rust(ptr);
 
-    let output = sig.sketches();
+    let output = sig.iter();
 
     // FIXME: how to fit this into the ForeignObject trait?
     let ptr_sigs: Vec<*mut Signature> = output.into_iter().map(|x| {

src/core/src/from.rs

@@ -16,7 +16,7 @@ impl From<MashSketcher> for KmerMinHash {
 
         let mut new_mh = KmerMinHash::new(
             0,
-            values.get(0).unwrap().kmer.len() as u32,
+            values.first().unwrap().kmer.len() as u32,
             HashFunctions::Murmur64Dna,
             42,
             true,
@@ -65,16 +65,15 @@ mod test {
 
         let b_hashes = b.to_vec();
 
-        let s1: HashSet<_> = a.mins().into_iter().collect();
+        let s1: HashSet<_> = a.iter_mins().copied().collect();
         let s2: HashSet<_> = b_hashes.iter().map(|x| x.hash).collect();
         let i1 = &s1 & &s2;
 
         assert!(i1.len() == a.size());
         assert!(i1.len() == b_hashes.len());
 
         if let Some(abunds) = a.abunds() {
-            let mins = a.mins();
-            let smap: HashMap<_, _> = mins.iter().zip(abunds.iter()).collect();
+            let smap: HashMap<_, _> = a.iter_mins().zip(abunds.iter()).collect();
             println!("{:?}", smap);
             for item in b_hashes.iter() {
                 assert!(smap.contains_key(&{ item.hash }));
@@ -101,19 +100,17 @@ mod test {
 
         let c = KmerMinHash::from(b);
 
-        let s1: HashSet<_> = a.mins().into_iter().collect();
-        let s2: HashSet<_> = c.mins().into_iter().collect();
+        let s1: HashSet<_> = a.iter_mins().collect();
+        let s2: HashSet<_> = c.iter_mins().collect();
         let i1 = &s1 & &s2;
 
-        assert!(i1.len() == a.mins().len());
-        assert!(i1.len() == c.mins().len());
+        assert!(i1.len() == a.iter_mins().count());
+        assert!(i1.len() == c.iter_mins().count());
 
         if let Some(a_abunds) = a.abunds() {
             if let Some(c_abunds) = c.abunds() {
-                let a_mins = a.mins();
-                let a_smap: HashMap<_, _> = a_mins.iter().zip(a_abunds.iter()).collect();
-                let c_mins = c.mins();
-                let c_smap: HashMap<_, _> = c_mins.iter().zip(c_abunds.iter()).collect();
+                let a_smap: HashMap<_, _> = a.iter_mins().zip(a_abunds.iter()).collect();
+                let c_smap: HashMap<_, _> = c.iter_mins().zip(c_abunds.iter()).collect();
                 for item in a_smap.iter() {
                     assert!(c_smap.contains_key(*item.0));
                     assert!(c_smap.get(*item.0).unwrap() == item.1);

src/core/src/index/linear.rs

@@ -25,7 +25,7 @@ pub struct LinearIndex {
 impl LinearIndex {
     pub fn from_collection(collection: CollectionSet) -> Self {
         let sig = collection.sig_for_dataset(0).unwrap();
-        let template = sig.sketches().swap_remove(0);
+        let template = sig.iter().next().unwrap().clone();
         Self {
             collection,
             template,

src/core/src/index/revindex/disk_revindex.rs

@@ -191,15 +191,15 @@ impl RevIndex {
             .collection
             .sig_for_dataset(dataset_id)
             .expect("Couldn't find a compatible Signature");
-        let search_mh = &search_sig.sketches()[0];
+        let search_mh = search_sig.iter().next().unwrap();
 
         let colors = Datasets::new(&[dataset_id]).as_bytes().unwrap();
 
         let cf_hashes = self.db.cf_handle(HASHES).unwrap();
 
-        let hashes = match search_mh {
-            Sketch::MinHash(mh) => mh.mins(),
-            Sketch::LargeMinHash(mh) => mh.mins(),
+        let hashes: Vec<_> = match search_mh {
+            Sketch::MinHash(mh) => mh.iter_mins().copied().collect(),
+            Sketch::LargeMinHash(mh) => mh.iter_mins().copied().collect(),
             _ => unimplemented!(),
         };
 

src/core/src/index/revindex/mem_revindex.rs

@@ -175,7 +175,7 @@ impl RevIndex {
                 }
             }
         } else {
-            let matched = search_mh.mins();
+            let matched: Vec<_> = search_mh.iter_mins().copied().collect();
             let size = matched.len() as u64;
             if !matched.is_empty() || size > threshold as u64 {
                 hash_to_color.add_to(&mut colors, dataset_id, matched);

src/core/src/index/revindex/mod.rs

@@ -19,7 +19,6 @@ use crate::index::{GatherResult, SigCounter};
 use crate::prelude::*;
 use crate::signature::Signature;
 use crate::sketch::minhash::KmerMinHash;
-use crate::sketch::Sketch;
 use crate::HashIntoType;
 use crate::Result;
 
@@ -225,13 +224,7 @@ impl RevIndex {
 pub fn prepare_query(search_sig: Signature, selection: &Selection) -> Option<KmerMinHash> {
     let sig = search_sig.select(selection).ok();
 
-    sig.and_then(|sig| {
-        if let Sketch::MinHash(mh) = sig.sketches().swap_remove(0) {
-            Some(mh)
-        } else {
-            None
-        }
-    })
+    sig.and_then(|sig| sig.minhash().cloned())
 }
 
 #[derive(Debug, Default, Clone)]

src/core/src/signature.rs

@@ -475,10 +475,6 @@ impl Signature {
         self.signatures.len()
     }
 
-    pub fn sketches(&self) -> Vec<Sketch> {
-        self.signatures.clone()
-    }
-
     pub fn reset_sketches(&mut self) {
         self.signatures = vec![];
     }

src/core/src/sketch/hyperloglog/mod.rs

@@ -214,8 +214,8 @@ impl SigsTrait for HyperLogLog {
 
 impl Update<HyperLogLog> for KmerMinHash {
     fn update(&self, other: &mut HyperLogLog) -> Result<(), Error> {
-        for h in self.mins() {
-            other.add_hash(h);
+        for h in self.iter_mins() {
+            other.add_hash(*h);
         }
         Ok(())
     }