andygrove opened a new issue, #6720: URL: https://github.com/apache/datafusion-comet/issues/6720
### What is the problem the feature request solves? `RowArrowReader` and `SparkColumnarArrowReader`, which `CometSparkToColumnarExec` uses for row and columnar input, call `ArrowWriter.create` for every Arrow batch. That calls `allocateNew` on each vector in the reader's `VectorSchemaRoot`, which releases the previous batch's buffers and allocates new ones. For a fixed-width vector, Arrow 18.3's `BaseFixedWidthVector.allocateNew(int)` also zeroes the validity buffer and the whole data buffer. The zeroed data is then overwritten: the columnar path's bulk copy writes every slot, and the row path writes every non-null one. While working on #6566, I measured this allocating and zeroing at about half of what the fixed-width columnar path still costs once the bulk copies are in. The fresh buffers are deliberate. As `RowArrowReader`'s doc says, a batch exported to native over the C Data Interface keeps its buffers until native releases it, so writing the next batch into them in place would corrupt a batch native may still hold. ### Describe the potential solution This needs a measurement on a real query first. In #6566's end-to-end numbers, the Parquet → TPC-H Q1-style query spent 80 ms of 548 ms in `conversionTime`. Options, which can be combined: - Reuse a vector's buffers for the next batch when nothing else references them. The exporter retains each buffer and releases it when native calls the batch's release callback, so the reference counts show whether native still holds the batch. On the JVM path, `CometArrowStream.readerBatchIter` hands out the reader's own vectors, so a batch is already invalid once the next one loads. A consumer can also close the vectors, as `ColumnarBatchArrowReader` does with each source batch, so allocating fresh buffers has to stay as the fallback. - Skip zeroing the data buffers. Validity and offset buffers still need it, because some writers rely on it: on the row path, the array and map writers write nothing for a null row. Those buffers are small next to the data. Arrow's fixed-width `allocateNew` always zeroes, so this means allocating the buffers directly and loading them into the vector. This also applies to `CometArrowConverters`, which the cache serializer uses. It gives each batch its own root, so reuse doesn't apply there. Without zeroing, a null row's data slot holds whatever was there before. The columnar bulk copy already leaves the value under a null unspecified. ### Additional context Part of #6565. `CometStringWriterSuite` (from #6281) checks that C Data consumers keep their string batches after the source is reused and the reader is closed. Reusing buffers needs that kind of test for every vector type. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
