-
Notifications
You must be signed in to change notification settings - Fork 630
[VL] Optimize Delta DV applyDeletionFilter with iterator-based bulk lookup #12395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
b017972
3c7be7a
d5a4489
348484e
b040617
c566737
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,10 +24,14 @@ | |
|
|
||
| #include <benchmark/benchmark.h> | ||
|
|
||
| #include "compute/delta/DeltaDeletionVectorReader.h" | ||
| #include "compute/delta/RoaringBitmapArray.h" | ||
| #include "velox/common/base/Exceptions.h" | ||
| #include "velox/common/memory/Memory.h" | ||
|
|
||
| using gluten::delta::RoaringBitmapArray; | ||
| using gluten::delta::DeltaDeletionVectorReader; | ||
| using namespace facebook::velox; | ||
|
|
||
| namespace { | ||
|
|
||
|
|
@@ -276,4 +280,78 @@ BENCHMARK_CAPTURE( | |
| ->Args({1 << 18, 64}) | ||
| ->Unit(benchmark::kMillisecond); | ||
|
|
||
| // Benchmark for applyDeletionFilter: measures the hot path where a batch of | ||
| // rows is checked against the deletion vector bitmap. | ||
| // deletionPercent: fraction of rows in the total file that are deleted. | ||
| // batchSize: number of rows per batch (typical Velox batch size). | ||
| void BM_ApplyDeletionFilter(benchmark::State& state, double deletionPercent) { | ||
| const auto batchSize = static_cast<uint64_t>(state.range(0)); | ||
| const uint64_t totalFileRows = 1000000; // 1M row file | ||
| const auto numDeleted = | ||
| static_cast<uint64_t>(totalFileRows * deletionPercent / 100.0); | ||
|
|
||
| // Build a DV with deletions spread across the file. | ||
| RoaringBitmapArray bitmap; | ||
| const uint64_t stride = numDeleted > 0 ? totalFileRows / numDeleted : 0; | ||
| for (uint64_t i = 0; i < numDeleted; ++i) { | ||
| bitmap.addSafe(i * stride); | ||
| } | ||
| const auto payload = bitmap.serializeToString(true); | ||
|
|
||
| // Load the DV reader. | ||
| DeltaDeletionVectorReader reader; | ||
| reader.loadSerializedDeletionVector( | ||
| std::string_view(payload.data(), payload.size())); | ||
|
|
||
| // Allocate the output bitmap buffer. | ||
| memory::MemoryManager::testingSetInstance(memory::MemoryManager::Options{}); | ||
| auto pool = memory::memoryManager()->addLeafPool(); | ||
| auto deleteBitmap = AlignedBuffer::allocate<uint64_t>( | ||
| bits::nwords(batchSize), pool.get()); | ||
|
|
||
| // Simulate scanning through the file in batches. | ||
| const uint64_t numBatches = totalFileRows / batchSize; | ||
| uint64_t totalDeletedFound = 0; | ||
|
|
||
| for (auto _ : state) { | ||
| totalDeletedFound = 0; | ||
| for (uint64_t batch = 0; batch < numBatches; ++batch) { | ||
| reader.applyDeletionFilter(batch * batchSize, batchSize, deleteBitmap); | ||
| // Count bits set to prevent dead-code elimination. | ||
| auto* raw = deleteBitmap->as<uint64_t>(); | ||
| for (uint64_t w = 0; w < bits::nwords(batchSize); ++w) { | ||
| totalDeletedFound += __builtin_popcountll(raw[w]); | ||
| } | ||
| } | ||
| benchmark::DoNotOptimize(totalDeletedFound); | ||
| } | ||
|
|
||
| state.SetItemsProcessed(state.iterations() * totalFileRows); | ||
| state.counters["batch_size"] = benchmark::Counter(batchSize); | ||
| state.counters["deletion_pct"] = benchmark::Counter(deletionPercent); | ||
| state.counters["deleted_found"] = benchmark::Counter(totalDeletedFound); | ||
| state.counters["total_batches"] = benchmark::Counter(numBatches); | ||
|
Comment on lines
+308
to
+333
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch on both points. Tail rows: Fixed — Padding bits in popcount: This is actually safe — |
||
| } | ||
|
|
||
| // Sparse deletions (1%) - the common case for MERGE/UPDATE operations. | ||
| BENCHMARK_CAPTURE(BM_ApplyDeletionFilter, Sparse_1pct, 1.0) | ||
| ->Arg(4096) | ||
| ->Unit(benchmark::kMillisecond); | ||
| // Moderate deletions (10%). | ||
| BENCHMARK_CAPTURE(BM_ApplyDeletionFilter, Moderate_10pct, 10.0) | ||
| ->Arg(4096) | ||
| ->Unit(benchmark::kMillisecond); | ||
| // Dense deletions (50%). | ||
| BENCHMARK_CAPTURE(BM_ApplyDeletionFilter, Dense_50pct, 50.0) | ||
| ->Arg(4096) | ||
| ->Unit(benchmark::kMillisecond); | ||
| // Very dense deletions (90%). | ||
| BENCHMARK_CAPTURE(BM_ApplyDeletionFilter, VeryDense_90pct, 90.0) | ||
| ->Arg(4096) | ||
| ->Unit(benchmark::kMillisecond); | ||
| // Sparse with large batch (typical Velox max batch). | ||
| BENCHMARK_CAPTURE(BM_ApplyDeletionFilter, Sparse_1pct_LargeBatch, 1.0) | ||
| ->Arg(10000) | ||
| ->Unit(benchmark::kMillisecond); | ||
|
|
||
| BENCHMARK_MAIN(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,15 +183,29 @@ void DeltaDeletionVectorReader::applyDeletionFilter(uint64_t baseReadOffset, uin | |
| auto* rawBitmap = deleteBitmap->asMutable<uint64_t>(); | ||
| std::memset(rawBitmap, 0, bits::nbytes(size)); | ||
|
|
||
| // Use an iterator-based approach instead of per-row contains() lookups. | ||
| // This is O(deletions_in_range) rather than O(batch_size), which is | ||
| // significantly faster when deletions are sparse relative to batch size. | ||
| const uint64_t rangeEnd = baseReadOffset + size; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed — added saturating arithmetic so const uint64_t rangeEnd = (size <= UINT64_MAX - baseReadOffset)
? baseReadOffset + size : UINT64_MAX;Also added a unit test ( |
||
| auto it = deletionBitmap_->begin(); | ||
| if (!it.move_equalorlarger(baseReadOffset)) { | ||
| // No deleted rows at or after baseReadOffset — nothing to mark. | ||
| deleteBitmap->setSize(0); | ||
| return; | ||
| } | ||
|
|
||
| bool hasDeletedRows = false; | ||
| uint64_t highestDeletedIndex = 0; | ||
| for (uint64_t i = 0; i < size; ++i) { | ||
| const uint64_t absoluteRowPos = baseReadOffset + i; | ||
| if (deletionBitmap_->contains(absoluteRowPos)) { | ||
| bits::setBit(rawBitmap, i); | ||
| hasDeletedRows = true; | ||
| highestDeletedIndex = i; | ||
| while (it != deletionBitmap_->end()) { | ||
| const uint64_t absoluteRowPos = *it; | ||
| if (absoluteRowPos >= rangeEnd) { | ||
| break; | ||
| } | ||
| const uint64_t relativeIndex = absoluteRowPos - baseReadOffset; | ||
| bits::setBit(rawBitmap, relativeIndex); | ||
| hasDeletedRows = true; | ||
| highestDeletedIndex = relativeIndex; | ||
| ++it; | ||
| } | ||
|
|
||
| deleteBitmap->setSize(hasDeletedRows ? bits::nbytes(highestDeletedIndex + 1) : 0); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch — moved
testingSetInstanceout of the benchmark body into a custommain()that initializes once before running benchmarks. This mirrors the pattern inGenericBenchmark.cc:642.