Skip to content

Commit 18cf20e

Browse files
committed
perf(py): decode subscriber samples without copying the payload
The Python callback path went through `RawBytesCdrSerdes::deserialize`, whose `Output` is an owned `RawBytesMessage` carrying no lifetime, so the whole payload was `to_vec()`d before the closure ran -- one full copy per message, scaling with payload size, discarded as soon as msgspec had decoded out of it. Adds `ZSubBuilder::build_with_sample_callback`, which hands the callback the `Sample` so it can borrow the payload and decode straight out of the network buffer, and switches `hiroz-py` to it. Everything else about the subscriber is unchanged: same encoding validation, same dispatch rules, same liveliness and graph registration. Split out of #250. It shared a call site with that PR's re-entrancy fix, which is proximity, not a reason to review them together.
1 parent 08405bc commit 18cf20e

2 files changed

Lines changed: 76 additions & 2 deletions

File tree

crates/hiroz-py/src/node.rs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use hiroz::node::ZNode;
1616
use pyo3::prelude::*;
1717
use std::any::Any;
1818
use std::sync::Arc;
19+
use zenoh_buffers::buffer::SplitBuffer;
1920

2021
/// Try to extract type info from a message class.
2122
///
@@ -233,9 +234,25 @@ impl PyZNode {
233234
// matching rmw_zenoh_cpp's NodeData::subs_ pattern. The caller does not
234235
// need to assign the returned PyZSubscriber to keep the subscription active.
235236
let type_name = msg_type_str.clone();
237+
// Sample-level callback, not `build_with_callback`. The typed form
238+
// would route through `RawBytesCdrSerdes::deserialize`, whose
239+
// `Output` is an owned `RawBytesMessage` and so must `to_vec()` the
240+
// whole payload before this closure runs — a full copy per message,
241+
// scaling with payload size, immediately discarded once msgspec has
242+
// decoded it. Taking the `Sample` lets the decode read straight out
243+
// of the network buffer, and matches what the polling `recv()` path
244+
// in `pubsub.rs` already does.
236245
let zsub = sub_builder
237-
.build_with_callback(move |raw_msg: RawBytesMessage| {
238-
let payload = raw_msg.0;
246+
.build_with_sample_callback(move |sample| {
247+
// Same zero-copy setup as `PyZSubscriber::recv`: the ZBuf is
248+
// cheap Arc clones, and publishing it as the deserializer's
249+
// source lets `bytes`-typed fields become sub-ZSlices of the
250+
// received buffer instead of copies.
251+
let payload_zbuf: zenoh_buffers::ZBuf = sample.payload().clone().into();
252+
hiroz_cdr::ZBUF_DESER_SOURCE.with(|cell| {
253+
*cell.borrow_mut() = Some(payload_zbuf.clone());
254+
});
255+
let payload = payload_zbuf.contiguous();
239256
Python::with_gil(|py| {
240257
match hiroz_msgs::deserialize_from_cdr(&type_name, py, &payload) {
241258
Ok(obj) => {
@@ -248,6 +265,9 @@ impl PyZNode {
248265
}
249266
}
250267
});
268+
hiroz_cdr::ZBUF_DESER_SOURCE.with(|cell| {
269+
*cell.borrow_mut() = None;
270+
});
251271
})
252272
.map_err(|e| e.into_pyerr())?;
253273

crates/hiroz/src/pubsub.rs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,60 @@ where
867867
})
868868
}
869869

870+
/// Build a callback subscriber that receives the whole [`Sample`], undecoded.
871+
///
872+
/// [`Self::build_with_callback`] must hand the callback an owned `S::Output`,
873+
/// and [`ZDeserializer::Output`] carries no lifetime — so a serdes that only
874+
/// forwards bytes (a language binding's identity codec, say) has no way to
875+
/// express "borrow the payload", and must copy the entire message before the
876+
/// callback has even seen it. That copy scales with payload size and is pure
877+
/// waste when the consumer immediately re-reads the bytes into its own
878+
/// representation.
879+
///
880+
/// This entry point steps around it: the callback gets the `Sample`, so it
881+
/// can borrow the payload (`sample.payload().to_bytes()` is a `Cow` that
882+
/// borrows whenever the `ZBuf` is contiguous, which the receive path makes it)
883+
/// and decode straight out of the network buffer. It can also reach the
884+
/// sample's attachment, encoding and timestamp, which the decoded form drops.
885+
///
886+
/// Everything else is identical to `build_with_callback` — same encoding
887+
/// validation, same [`CallbackDispatcher`] handling, same liveliness and
888+
/// graph registration. The callback is user code and is dispatched by exactly
889+
/// the same rules.
890+
///
891+
/// # Ownership
892+
///
893+
/// As with `build_with_callback`, the returned [`ZSub`] must be kept alive for
894+
/// the subscription to stay active.
895+
pub fn build_with_sample_callback<F>(self, callback: F) -> Result<ZSub<T, (), S>>
896+
where
897+
F: Fn(Sample) + Send + Sync + 'static,
898+
S: ZDeserializer,
899+
{
900+
let expected_encoding = self.expected_encoding.clone();
901+
let callback = Arc::new(move |sample: Sample| {
902+
if let Some(ref expected) = expected_encoding {
903+
let encoding_str = sample.encoding().to_string();
904+
if let Some(received) =
905+
crate::encoding::Encoding::from_zenoh_encoding(&encoding_str)
906+
{
907+
if &received != expected {
908+
tracing::warn!(
909+
"Encoding mismatch: expected {:?}, received {:?}",
910+
expected,
911+
received
912+
);
913+
}
914+
} else {
915+
tracing::debug!("Unknown encoding format: {}", encoding_str);
916+
}
917+
}
918+
callback(sample);
919+
});
920+
921+
self.build_internal(DataHandler::Callback(callback), None)
922+
}
923+
870924
/// Build a subscriber with a callback that processes deserialized messages directly.
871925
///
872926
/// This method creates a subscriber that invokes the provided callback for each

0 commit comments

Comments
 (0)