perf: Eliminate several copies from C to rust (#293)

* metadata decoding no longer makes an extra
  copy
* ancestral/derived states of sites/mutations
  no longer make extra copies.

BREAKING CHANGE: return types of
SiteTable::ancestral_state and
MutationTable:derived_state now return
slices instead of vectors.

Author: molpopgen

src/_macros.rs

@@ -173,8 +173,9 @@ macro_rules! build_tskit_type {
 }
 
 macro_rules! metadata_to_vector {
-    ($table: expr, $row: expr) => {
-        $crate::metadata::char_column_to_vector(
+    ($outer: ident, $table: expr, $row: expr) => {
+        $crate::metadata::char_column_to_slice(
+            $outer,
             $table.metadata,
             $table.metadata_offset,
             $row,
@@ -196,8 +197,10 @@ macro_rules! decode_metadata_row {
 }
 
 macro_rules! table_row_decode_metadata {
-    ($table: ident, $pos: ident) => {
-        metadata_to_vector!($table, $pos).unwrap().map(|x| x)
+    ($owner: ident, $table: ident, $pos: ident) => {
+        metadata_to_vector!($owner, $table, $pos)
+            .unwrap()
+            .map(|x| x)
     };
 }
 

src/edge_table.rs

@@ -39,7 +39,7 @@ fn make_edge_table_row(table: &EdgeTable, pos: tsk_id_t) -> Option<EdgeTableRow>
             right: table.right(pos).unwrap(),
             parent: table.parent(pos).unwrap(),
             child: table.child(pos).unwrap(),
-            metadata: table_row_decode_metadata!(table_ref, pos),
+            metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
         };
         Some(rv)
     } else {
@@ -141,7 +141,7 @@ impl<'a> EdgeTable<'a> {
         row: EdgeId,
     ) -> Result<Option<T>, TskitError> {
         let table_ref = self.table_;
-        let buffer = metadata_to_vector!(table_ref, row.0)?;
+        let buffer = metadata_to_vector!(self, table_ref, row.0)?;
         decode_metadata_row!(T, buffer)
     }
 

src/individual_table.rs

@@ -63,7 +63,7 @@ fn make_individual_table_row(table: &IndividualTable, pos: tsk_id_t) -> Option<I
             flags: table.flags(pos).unwrap(),
             location: table.location(pos).unwrap().map(|s| s.to_vec()),
             parents: table.parents(pos).unwrap().map(|s| s.to_vec()),
-            metadata: table_row_decode_metadata!(table_ref, pos),
+            metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
         };
         Some(rv)
     } else {
@@ -248,7 +248,7 @@ impl<'a> IndividualTable<'a> {
         row: IndividualId,
     ) -> Result<Option<T>, TskitError> {
         let table_ref = self.table_;
-        let buffer = metadata_to_vector!(table_ref, row.0)?;
+        let buffer = metadata_to_vector!(self, table_ref, row.0)?;
         decode_metadata_row!(T, buffer)
     }
 

src/metadata.rs

@@ -250,13 +250,14 @@ pub enum MetadataError {
     },
 }
 
-pub(crate) fn char_column_to_vector(
+pub(crate) fn char_column_to_slice<T: Sized>(
+    _lifetime: &T,
     column: *const libc::c_char,
     column_offset: *const tsk_size_t,
     row: tsk_id_t,
     num_rows: tsk_size_t,
     column_length: tsk_size_t,
-) -> Result<Option<Vec<u8>>, crate::TskitError> {
+) -> Result<Option<&[u8]>, crate::TskitError> {
     let row = match tsk_size_t::try_from(row) {
         Ok(r) => r,
         Err(_) => return Err(TskitError::IndexError),
@@ -288,23 +289,27 @@ pub(crate) fn char_column_to_vector(
     if column_length == 0 {
         return Ok(None);
     }
-    let mut buffer = vec![];
-    for i in start..stop {
-        match isize::try_from(i) {
-            // NOTE: cast_sign_loss pedantic lint is a false +ve here.
-            // The metadata live as C strings on the tskit-C side, so
-            // the integer cast exists as part of the round trip.
-            #[allow(clippy::cast_sign_loss)]
-            Ok(o) => buffer.push(unsafe { *column.offset(o) } as u8),
-            Err(_) => {
-                return Err(TskitError::RangeError(format!(
-                    "cauld not convert value {} to isize",
-                    stringify!(i)
-                )))
-            }
-        };
-    }
-    Ok(Some(buffer))
+    let istart = match isize::try_from(start) {
+        Ok(v) => v,
+        Err(_) => {
+            return Err(TskitError::RangeError(format!(
+                "cauld not convert value {} to isize",
+                stringify!(i)
+            )));
+        }
+    };
+    let ustop = match usize::try_from(stop) {
+        Ok(v) => v,
+        Err(_) => {
+            return Err(TskitError::RangeError(format!(
+                "cauld not convert value {} to usize",
+                stringify!(i)
+            )));
+        }
+    };
+    Ok(Some(unsafe {
+        std::slice::from_raw_parts(column.offset(istart) as *const u8, ustop - istart as usize)
+    }))
 }
 
 #[cfg(test)]

src/migration_table.rs

@@ -47,7 +47,7 @@ fn make_migration_table_row(table: &MigrationTable, pos: tsk_id_t) -> Option<Mig
             source: table.source(pos).unwrap(),
             dest: table.dest(pos).unwrap(),
             time: table.time(pos).unwrap(),
-            metadata: table_row_decode_metadata!(table_ref, pos),
+            metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
         })
     } else {
         None
@@ -186,7 +186,7 @@ impl<'a> MigrationTable<'a> {
         row: MigrationId,
     ) -> Result<Option<T>, TskitError> {
         let table_ref = self.table_;
-        let buffer = metadata_to_vector!(table_ref, row.0)?;
+        let buffer = metadata_to_vector!(self, table_ref, row.0)?;
         decode_metadata_row!(T, buffer)
     }
 

src/mutation_table.rs

@@ -42,8 +42,8 @@ fn make_mutation_table_row(table: &MutationTable, pos: tsk_id_t) -> Option<Mutat
             node: table.node(pos).unwrap(),
             parent: table.parent(pos).unwrap(),
             time: table.time(pos).unwrap(),
-            derived_state: table.derived_state(pos).unwrap(),
-            metadata: table_row_decode_metadata!(table_ref, pos),
+            derived_state: table.derived_state(pos).unwrap().map(|s| s.to_vec()),
+            metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
         };
         Some(rv)
     } else {
@@ -155,8 +155,9 @@ impl<'a> MutationTable<'a> {
     pub fn derived_state<M: Into<MutationId>>(
         &'a self,
         row: M,
-    ) -> Result<Option<Vec<u8>>, TskitError> {
-        metadata::char_column_to_vector(
+    ) -> Result<Option<&[u8]>, TskitError> {
+        metadata::char_column_to_slice(
+            self,
             self.table_.derived_state,
             self.table_.derived_state_offset,
             row.into().0,
@@ -170,7 +171,7 @@ impl<'a> MutationTable<'a> {
         row: MutationId,
     ) -> Result<Option<T>, TskitError> {
         let table_ref = self.table_;
-        let buffer = metadata_to_vector!(table_ref, row.0)?;
+        let buffer = metadata_to_vector!(self, table_ref, row.0)?;
         decode_metadata_row!(T, buffer)
     }
 

src/node_table.rs

@@ -41,7 +41,7 @@ fn make_node_table_row(table: &NodeTable, pos: tsk_id_t) -> Option<NodeTableRow>
             flags: table.flags(pos).unwrap(),
             population: table.population(pos).unwrap(),
             individual: table.individual(pos).unwrap(),
-            metadata: table_row_decode_metadata!(table_ref, pos),
+            metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
         })
     } else {
         None
@@ -189,7 +189,7 @@ impl<'a> NodeTable<'a> {
         row: NodeId,
     ) -> Result<Option<T>, TskitError> {
         let table_ref = self.table_;
-        let buffer = metadata_to_vector!(table_ref, row.0)?;
+        let buffer = metadata_to_vector!(self, table_ref, row.0)?;
         decode_metadata_row!(T, buffer)
     }
 

src/population_table.rs

@@ -28,7 +28,7 @@ fn make_population_table_row(table: &PopulationTable, pos: tsk_id_t) -> Option<P
         let table_ref = table.table_;
         let rv = PopulationTableRow {
             id: pos.into(),
-            metadata: table_row_decode_metadata!(table_ref, pos),
+            metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
         };
         Some(rv)
     } else {
@@ -86,7 +86,7 @@ impl<'a> PopulationTable<'a> {
         row: PopulationId,
     ) -> Result<Option<T>, TskitError> {
         let table_ref = self.table_;
-        let buffer = metadata_to_vector!(table_ref, row.0)?;
+        let buffer = metadata_to_vector!(self, table_ref, row.0)?;
         decode_metadata_row!(T, buffer)
     }
 

src/site_table.rs

@@ -34,8 +34,8 @@ fn make_site_table_row(table: &SiteTable, pos: tsk_id_t) -> Option<SiteTableRow>
         let rv = SiteTableRow {
             id: pos.into(),
             position: table.position(pos).unwrap(),
-            ancestral_state: table.ancestral_state(pos).unwrap(),
-            metadata: table_row_decode_metadata!(table_ref, pos),
+            ancestral_state: table.ancestral_state(pos).unwrap().map(|s| s.to_vec()),
+            metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
         };
         Some(rv)
     } else {
@@ -108,11 +108,9 @@ impl<'a> SiteTable<'a> {
     ///
     /// Will return [``IndexError``](crate::TskitError::IndexError)
     /// if ``row`` is out of range.
-    pub fn ancestral_state<S: Into<SiteId>>(
-        &'a self,
-        row: S,
-    ) -> Result<Option<Vec<u8>>, TskitError> {
-        crate::metadata::char_column_to_vector(
+    pub fn ancestral_state<S: Into<SiteId>>(&'a self, row: S) -> Result<Option<&[u8]>, TskitError> {
+        crate::metadata::char_column_to_slice(
+            self,
             self.table_.ancestral_state,
             self.table_.ancestral_state_offset,
             row.into().0,
@@ -126,7 +124,7 @@ impl<'a> SiteTable<'a> {
         row: SiteId,
     ) -> Result<Option<T>, TskitError> {
         let table_ref = self.table_;
-        let buffer = metadata_to_vector!(table_ref, row.0)?;
+        let buffer = metadata_to_vector!(self, table_ref, row.0)?;
         decode_metadata_row!(T, buffer)
     }