Skip to content

Commit

Permalink
Avoid copying data from Python heap in Encoder.write method
Browse files Browse the repository at this point in the history
  • Loading branch information
althonos committed Apr 10, 2024
1 parent 81dd7f4 commit 62aa921
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 4 deletions.
47 changes: 44 additions & 3 deletions nafcodec-py/nafcodec/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,10 +524,51 @@ impl Encoder {

pub fn write<'py>(mut slf: PyRefMut<'py, Self>, record: &'py Record) -> PyResult<()> {
let py = slf.py();

// This macro allows borrowing a field from the Python record and get
// a `Cow<'py, str>` instead of copying the data.
//
// The problem here is that the borrow need to live long enough
// for all fields to be read and the resulting record written to the
// encoder. However, because every field is optional, the borrows
// would occur in `if let` blocks:
//
// ```
// if let Some(x) = record.id {
// id = Some(x.bind(py).as_borrowed().to_str());
// }
// ```
//
// To avoid this, we store the borrowed reference in an external
// variable that lives longer than the `if let` scope.
//
macro_rules! borrow_field {
($field:ident) => {
#[allow(unused_assignments)]
let mut borrowed = None;
let mut $field = None;
if let Some(x) = record.$field.as_ref() {
let s = x.bind(py);
let b = s.as_borrowed();
borrowed = Some(b);
$field = borrowed.as_ref().map(|b| b.to_cow()).transpose()?;
}
};
}

if let Some(encoder) = slf.encoder.as_mut() {
encoder
.push(&record.try_into()?)
.map_err(|err| convert_error(py, err, None))
borrow_field!(id);
borrow_field!(comment);
borrow_field!(sequence);
borrow_field!(quality);
let r = nafcodec::Record {
id,
comment,
sequence,
quality,
length: record.length.clone(),
};
encoder.push(&r).map_err(|err| convert_error(py, err, None))
} else {
Err(PyRuntimeError::new_err("operation on closed encoder."))
}
Expand Down
3 changes: 2 additions & 1 deletion nafcodec/src/encoder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ fn write_length<W: Write>(mut l: u64, mut w: W) -> Result<(), IoError> {
/// For instance, to write a nucleotide archive containing only the sequence
/// and identifier of each record:
/// ```rust
/// let encoder = nafcodec::EncoderBuilder::new(nafcodec::SequenceType::Dna)
/// # use nafcodec::*;
/// let encoder = EncoderBuilder::new(SequenceType::Dna)
/// .id(true)
/// .sequence(true)
/// .with_memory()
Expand Down

0 comments on commit 62aa921

Please sign in to comment.