tustvold commented on code in PR #7015:
URL: https://github.com/apache/arrow-rs/pull/7015#discussion_r1986249663
##########
arrow-json/src/writer/encoder.rs:
##########
@@ -196,9 +236,18 @@ impl Encoder for StructArrayEncoder<'_> {
let mut is_first = true;
// Nulls can only be dropped in explicit mode
let drop_nulls = (self.struct_mode == StructMode::ObjectOnly) &&
!self.explicit_nulls;
- for field_encoder in &mut self.encoders {
- let is_null = is_some_and(field_encoder.nulls.as_ref(), |n|
n.is_null(idx));
- if drop_nulls && is_null {
+
+ // Collect all the field nulls buffers up front to avoid dynamic
dispatch in the loop
+ // This creates a temporary Vec, but avoids repeated virtual calls
which should be a net win
Review Comment:
We're still doing this dynamic dispatch to get null information on every
value write... Unless we can show with benchmarks it doesn't regress
performance I'm lukewarm on this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
