diff --git a/src/bam/buffer.rs b/src/bam/buffer.rs index 6ebcc6202..7f16a6eae 100644 --- a/src/bam/buffer.rs +++ b/src/bam/buffer.rs @@ -157,12 +157,12 @@ impl RecordBuffer { } /// Iterate over records that have been fetched with `fetch`. - pub fn iter(&self) -> vec_deque::Iter> { + pub fn iter(&'_ self) -> vec_deque::Iter<'_, Rc> { self.inner.iter() } /// Iterate over mutable references to records that have been fetched with `fetch`. - pub fn iter_mut(&mut self) -> vec_deque::IterMut> { + pub fn iter_mut(&'_ mut self) -> vec_deque::IterMut<'_, Rc> { self.inner.iter_mut() } diff --git a/src/bam/header.rs b/src/bam/header.rs index 02e594c55..45fa3da18 100644 --- a/src/bam/header.rs +++ b/src/bam/header.rs @@ -106,7 +106,7 @@ impl Header { } /// Returns an iterator of comment lines. - pub fn comments(&self) -> impl Iterator> { + pub fn comments(&'_ self) -> impl Iterator> { self.records.iter().flat_map(|r| { r.split(|x| x == &b'\n') .filter(|x| x.starts_with(b"@CO\t")) diff --git a/src/bam/mod.rs b/src/bam/mod.rs index 9b9d3aa02..f913108fe 100644 --- a/src/bam/mod.rs +++ b/src/bam/mod.rs @@ -21,6 +21,7 @@ use std::path::Path; use std::rc::Rc; use std::slice; use std::str; +use std::sync::Arc; use url::Url; @@ -250,7 +251,7 @@ pub trait Read: Sized { #[derive(Debug)] pub struct Reader { htsfile: *mut htslib::htsFile, - header: Rc, + header: Arc, tpool: Option, } @@ -298,7 +299,7 @@ impl Reader { Ok(Reader { htsfile, - header: Rc::new(HeaderView::new(header)), + header: Arc::new(HeaderView::new(header)), tpool: None, }) } @@ -383,7 +384,7 @@ impl Read for Reader { -2 => Some(Err(Error::BamTruncatedRecord)), -4 => Some(Err(Error::BamInvalidRecord)), _ => { - record.set_header(Rc::clone(&self.header)); + record.set_header(Arc::clone(&self.header)); Some(Ok(())) } @@ -591,8 +592,8 @@ impl<'a, T: AsRef<[u8]>, X: Into, Y: Into> Fro #[derive(Debug)] pub struct IndexedReader { htsfile: *mut htslib::htsFile, - header: Rc, - idx: Rc, + header: Arc, + idx: Arc, itr: Option<*mut htslib::hts_itr_t>, tpool: Option, } @@ -637,8 +638,8 @@ impl IndexedReader { } else { Ok(IndexedReader { htsfile, - header: Rc::new(HeaderView::new(header)), - idx: Rc::new(IndexView::new(idx)), + header: Arc::new(HeaderView::new(header)), + idx: Arc::new(IndexView::new(idx)), itr: None, tpool: None, }) @@ -665,8 +666,8 @@ impl IndexedReader { } else { Ok(IndexedReader { htsfile, - header: Rc::new(HeaderView::new(header)), - idx: Rc::new(IndexView::new(idx)), + header: Arc::new(HeaderView::new(header)), + idx: Arc::new(IndexView::new(idx)), itr: None, tpool: None, }) @@ -908,15 +909,14 @@ impl IndexedReader { #[derive(Debug)] pub struct IndexView { inner: *mut hts_sys::hts_idx_t, - owned: bool, } +unsafe impl Send for IndexView {} +unsafe impl Sync for IndexView {} + impl IndexView { fn new(hts_idx: *mut hts_sys::hts_idx_t) -> Self { - Self { - inner: hts_idx, - owned: true, - } + Self { inner: hts_idx } } #[inline] @@ -960,10 +960,8 @@ impl IndexView { impl Drop for IndexView { fn drop(&mut self) { - if self.owned { - unsafe { - htslib::hts_idx_destroy(self.inner); - } + unsafe { + htslib::hts_idx_destroy(self.inner); } } } @@ -977,7 +975,7 @@ impl Read for IndexedReader { -2 => Some(Err(Error::BamTruncatedRecord)), -4 => Some(Err(Error::BamInvalidRecord)), _ => { - record.set_header(Rc::clone(&self.header)); + record.set_header(Arc::clone(&self.header)); Some(Ok(())) } @@ -1060,7 +1058,7 @@ impl Format { #[derive(Debug)] pub struct Writer { f: *mut htslib::htsFile, - header: Rc, + header: Arc, tpool: Option, } @@ -1134,7 +1132,7 @@ impl Writer { Ok(Writer { f, - header: Rc::new(HeaderView::new(header_record)), + header: Arc::new(HeaderView::new(header_record)), tpool: None, }) } @@ -1364,9 +1362,11 @@ fn itr_next( #[derive(Debug)] pub struct HeaderView { inner: *mut htslib::bam_hdr_t, - owned: bool, } +unsafe impl Send for HeaderView {} +unsafe impl Sync for HeaderView {} + impl HeaderView { /// Create a new HeaderView from a pre-populated Header object pub fn from_header(header: &Header) -> Self { @@ -1399,8 +1399,8 @@ impl HeaderView { } /// Create a new HeaderView from the underlying Htslib type, and own it. - pub fn new(inner: *mut htslib::bam_hdr_t) -> Self { - HeaderView { inner, owned: true } + fn new(inner: *mut htslib::bam_hdr_t) -> Self { + HeaderView { inner } } #[inline] @@ -1480,17 +1480,14 @@ impl Clone for HeaderView { fn clone(&self) -> Self { HeaderView { inner: unsafe { htslib::sam_hdr_dup(self.inner) }, - owned: true, } } } impl Drop for HeaderView { fn drop(&mut self) { - if self.owned { - unsafe { - htslib::sam_hdr_destroy(self.inner); - } + unsafe { + htslib::sam_hdr_destroy(self.inner); } } } diff --git a/src/bam/record.rs b/src/bam/record.rs index f63e87797..16bee70c9 100644 --- a/src/bam/record.rs +++ b/src/bam/record.rs @@ -11,9 +11,9 @@ use std::marker::PhantomData; use std::mem::{size_of, MaybeUninit}; use std::ops; use std::os::raw::c_char; -use std::rc::Rc; use std::slice; use std::str; +use std::sync::Arc; use byteorder::{LittleEndian, ReadBytesExt}; @@ -53,7 +53,7 @@ pub struct Record { pub inner: htslib::bam1_t, own: bool, cigar: Option, - header: Option>, + header: Option>, } unsafe impl Send for Record {} @@ -183,7 +183,7 @@ impl Record { } } - pub fn set_header(&mut self, header: Rc) { + pub fn set_header(&mut self, header: Arc) { self.header = Some(header); } @@ -833,7 +833,7 @@ impl Record { /// /// When an error occurs, the `Err` variant will be returned /// and the iterator will not be able to advance anymore. - pub fn aux_iter(&self) -> AuxIter { + pub fn aux_iter(&'_ self) -> AuxIter<'_> { AuxIter { // In order to get to the aux data section of a `bam::Record` // we need to skip fields in front of it @@ -1118,14 +1118,14 @@ impl Record { /// } /// assert_eq!(mod_count, 14); /// ``` - pub fn basemods_iter(&self) -> Result { + pub fn basemods_iter(&'_ self) -> Result> { BaseModificationsIter::new(self) } /// An iterator that returns all of the modifications for each position as a vector. /// This is useful for the case where multiple possible modifications can be annotated /// at a single position (for example a C could be 5-mC or 5-hmC) - pub fn basemods_position_iter(&self) -> Result { + pub fn basemods_position_iter(&'_ self) -> Result> { BaseModificationsPositionIter::new(self) } @@ -1504,7 +1504,7 @@ where } /// Returns an iterator over the array. - pub fn iter(&self) -> AuxArrayIter { + pub fn iter(&'_ self) -> AuxArrayIter<'_, T> { AuxArrayIter { index: 0, array: self, diff --git a/src/bcf/header.rs b/src/bcf/header.rs index 20594b09d..39702eeed 100644 --- a/src/bcf/header.rs +++ b/src/bcf/header.rs @@ -34,9 +34,9 @@ use std::ffi; use std::os::raw::c_char; -use std::rc::Rc; use std::slice; use std::str; +use std::sync::Arc; use crate::htslib; @@ -65,10 +65,13 @@ custom_derive! { /// A BCF header. #[derive(Debug)] pub struct Header { - pub inner: *mut htslib::bcf_hdr_t, + pub(crate) inner: *mut htslib::bcf_hdr_t, pub subset: Option, } +unsafe impl Send for Header {} +unsafe impl Sync for Header {} + impl Default for Header { fn default() -> Self { Self::new() @@ -266,11 +269,14 @@ pub enum HeaderRecord { #[derive(Debug)] pub struct HeaderView { - pub inner: *mut htslib::bcf_hdr_t, + pub(crate) inner: *mut htslib::bcf_hdr_t, } +unsafe impl Send for HeaderView {} +unsafe impl Sync for HeaderView {} + impl HeaderView { - pub fn new(inner: *mut htslib::bcf_hdr_t) -> Self { + pub(crate) fn new(inner: *mut htslib::bcf_hdr_t) -> Self { HeaderView { inner } } @@ -513,8 +519,15 @@ impl HeaderView { /// Create an empty record using this header view. /// /// The record can be reused multiple times. + /// + /// # Performance + /// + /// This is quite slow and resource-intensive as it clones the actual + /// header, instead of the reference to it. Therefore, whenever possible + /// / feasible you should use [`Record::new`](crate::bcf::Record::new) with + /// a reference to your header. pub fn empty_record(&self) -> crate::bcf::Record { - crate::bcf::Record::new(Rc::new(self.clone())) + crate::bcf::Record::new(Arc::new(self.clone())) } } diff --git a/src/bcf/mod.rs b/src/bcf/mod.rs index a19183073..44b66fc62 100644 --- a/src/bcf/mod.rs +++ b/src/bcf/mod.rs @@ -105,8 +105,8 @@ use std::ffi; use std::path::Path; -use std::rc::Rc; use std::str; +use std::sync::Arc; use url::Url; @@ -159,10 +159,11 @@ pub trait Read: Sized { #[derive(Debug)] pub struct Reader { inner: *mut htslib::htsFile, - header: Rc, + header: Arc, } unsafe impl Send for Reader {} + /// # Safety /// /// Implementation for `Reader::set_threads()` and `Writer::set_threads`. @@ -202,7 +203,7 @@ impl Reader { let header = unsafe { htslib::bcf_hdr_read(htsfile) }; Ok(Reader { inner: htsfile, - header: Rc::new(HeaderView::new(header)), + header: Arc::new(HeaderView::new(header)), }) } } @@ -215,7 +216,7 @@ impl Read for Reader { // Always unpack record. htslib::bcf_unpack(record.inner_mut(), htslib::BCF_UN_ALL as i32); } - record.set_header(Rc::clone(&self.header)); + record.set_header(Arc::clone(&self.header)); Some(Ok(())) } -1 => None, @@ -224,7 +225,7 @@ impl Read for Reader { } fn records(&mut self) -> Records<'_, Self> { - Records { reader: self } + Records::new(self) } fn set_threads(&mut self, n_threads: usize) -> Result<()> { @@ -237,7 +238,7 @@ impl Read for Reader { /// Return empty record. Can be reused multiple times. fn empty_record(&self) -> Record { - self.header.empty_record() + Record::new(self.header.clone()) } } @@ -255,7 +256,7 @@ pub struct IndexedReader { /// The synced VCF/BCF reader to use internally. inner: *mut htslib::bcf_srs_t, /// The header. - header: Rc, + header: Arc, /// The position of the previous fetch, if any. current_region: Option<(u32, u64, Option)>, @@ -298,7 +299,7 @@ impl IndexedReader { } // 0: BCF_SR_REQUIRE_IDX // Attach a file with the path from the arguments. if unsafe { htslib::bcf_sr_add_reader(ser_reader, path.as_ptr()) } >= 0 { - let header = Rc::new(HeaderView::new(unsafe { + let header = Arc::new(HeaderView::new(unsafe { htslib::bcf_hdr_dup((*(*ser_reader).readers.offset(0)).header) })); Ok(IndexedReader { @@ -368,7 +369,7 @@ impl Read for IndexedReader { htslib::bcf_unpack(record.inner_mut(), htslib::BCF_UN_ALL as i32); } - record.set_header(Rc::clone(&self.header)); + record.set_header(Arc::clone(&self.header)); match self.current_region { Some((rid, _start, end)) => { @@ -389,7 +390,7 @@ impl Read for IndexedReader { } fn records(&mut self) -> Records<'_, Self> { - Records { reader: self } + Records::new(self) } fn set_threads(&mut self, n_threads: usize) -> Result<()> { @@ -408,7 +409,7 @@ impl Read for IndexedReader { } fn empty_record(&self) -> Record { - Record::new(Rc::clone(&self.header)) + Record::new(self.header.clone()) } } @@ -451,8 +452,8 @@ pub mod synced { /// Internal handle for the synced reader. inner: *mut crate::htslib::bcf_srs_t, - /// RC's of `HeaderView`s of the readers. - headers: Vec>, + /// Arcs of `HeaderView`s of the readers. + headers: Vec>, /// The position of the previous fetch, if any. current_region: Option<(u32, u64, u64)>, @@ -503,7 +504,7 @@ pub mod synced { } let i = (self.reader_count() - 1) as isize; - let header = Rc::new(HeaderView::new(unsafe { + let header = Arc::new(HeaderView::new(unsafe { crate::htslib::bcf_hdr_dup((*(*self.inner).readers.offset(i)).header) })); self.headers.push(header); @@ -642,7 +643,7 @@ pub enum Format { #[derive(Debug)] pub struct Writer { inner: *mut htslib::htsFile, - header: Rc, + header: Arc, subset: Option, } @@ -710,7 +711,7 @@ impl Writer { unsafe { htslib::bcf_hdr_write(htsfile, header.inner) }; Ok(Writer { inner: htsfile, - header: Rc::new(HeaderView::new(unsafe { + header: Arc::new(HeaderView::new(unsafe { htslib::bcf_hdr_dup(header.inner) })), subset: header.subset.clone(), @@ -726,7 +727,7 @@ impl Writer { /// /// This record can then be reused multiple times. pub fn empty_record(&self) -> Record { - record::Record::new(Rc::clone(&self.header)) + Record::new(self.header.clone()) } /// Translate record to header of this writer. @@ -738,7 +739,7 @@ impl Writer { unsafe { htslib::bcf_translate(self.header.inner, record.header().inner, record.inner); } - record.set_header(Rc::clone(&self.header)); + record.set_header(Arc::clone(&self.header)); } /// Subset samples of record to match header of this writer. @@ -796,6 +797,12 @@ pub struct Records<'a, R: Read> { reader: &'a mut R, } +impl<'a, R: Read> Records<'a, R> { + pub fn new(reader: &'a mut R) -> Self { + Self { reader } + } +} + impl Iterator for Records<'_, R> { type Item = Result; diff --git a/src/bcf/record.rs b/src/bcf/record.rs index 8cd1edd4b..2ebc87b97 100644 --- a/src/bcf/record.rs +++ b/src/bcf/record.rs @@ -9,9 +9,9 @@ use std::marker::PhantomData; use std::ops::Deref; use std::os::raw::c_char; use std::ptr; -use std::rc::Rc; use std::slice; use std::str; +use std::sync::Arc; use std::{ffi, iter}; use bio_types::genome; @@ -175,12 +175,12 @@ impl<'a, T: 'a + fmt::Debug + fmt::Display, B: Borrow + 'a> fmt::Display #[derive(Debug)] pub struct Record { pub inner: *mut htslib::bcf1_t, - header: Rc, + header: Arc, } impl Record { /// Construct record with reference to header `HeaderView`, for create-internal use. - pub(crate) fn new(header: Rc) -> Self { + pub fn new(header: Arc) -> Self { let inner = unsafe { let inner = htslib::bcf_init(); // Always unpack record. @@ -201,7 +201,7 @@ impl Record { } /// Set the record header. - pub(crate) fn set_header(&mut self, header: Rc) { + pub(crate) fn set_header(&mut self, header: Arc) { self.header = header; } diff --git a/src/faidx/mod.rs b/src/faidx/mod.rs index 1de779bb2..93fd06b62 100644 --- a/src/faidx/mod.rs +++ b/src/faidx/mod.rs @@ -195,6 +195,8 @@ impl Drop for Reader { } } +unsafe impl Send for Reader {} + #[cfg(test)] mod tests { use super::*;