zjw1111 commented on code in PR #472:
URL: https://github.com/apache/paimon-cpp/pull/472#discussion_r4398346835
##########
src/paimon/core/deletionvectors/apply_deletion_vector_batch_reader.h:
##########
@@ -101,9 +100,12 @@ class ApplyDeletionVectorBatchReader : public
FileBatchReader {
}
private:
- Result<RoaringBitmap32> Filter(int32_t batch_size) const {
+ Result<RoaringBitmap32> Filter(const RoaringBitmap32& selection) const {
RoaringBitmap32 is_valid;
- for (int32_t i = 0; i < batch_size; ++i) {
+ // Unselected rows may lie outside the group's anchor range. Probe
only rows that the
+ // format/index selection actually returns, just as
ApplyDeletionVectorReader does.
+ for (auto iter = selection.Begin(); iter != selection.End(); ++iter) {
Review Comment:
**Repeated heap allocation in the DV filtering loop.** Both Paimon
`RoaringBitmap32/64` iterator wrappers own a heap-allocated CRoaring iterator.
Each evaluation of `selection.End()` here constructs and destroys one of these
objects, adding an allocation/free pair per selected row, plus the final
condition check. This puts allocator traffic on the DV batch-read hot path.
The same wrapper behavior also affects existing callers such as
`BitmapDeletionVector::ForEachDeletedPosition()` and
`BitmapGlobalIndexResult::Iterator::HasNext()`, so the underlying cost is
shared across bitmap traversal paths.
--
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]
