zhuqi-lucas opened a new pull request, #10901:
URL: https://github.com/apache/arrow-rs/pull/10901
## Which issue does this PR close?
- Part of #10774.
## Rationale for this change
While profiling #10774 (`CachedArrayReader::consume_batch` on ClickBench
Q12/Q25 shapes, where the filter column is also the projected column), the
largest avoidable cost turned out not to be the `filter`+`concat` pair itself
but `cleanup_consumed_batches`:
```rust
for batch_id_to_remove in 0..(current_batch_id.val - 1) {
cache.remove(...);
}
```
It rescans batch ids from 0 on every `consume_batch` call, re-removing ids
that earlier calls already removed, and takes the shared cache's **write lock**
each time even when there is nothing left to remove. Over a row group with N
batches this is O(N²) `remove` calls; the consumer-role reader runs it once per
output batch per cached column.
## What changes are included in this PR?
Track the already-cleaned frontier (`cleaned_up_to`) and remove only the new
range `cleaned_up_to..end`. When the frontier has not advanced, return without
touching the lock.
## Are these changes tested?
Covered by the existing `cached_array_reader` unit tests and the parquet
`--lib` suite (1331 passed; the one failure, `test_int96_interop`, is a missing
`parquet-testing` file unrelated to this change).
Local `arrow_reader_clickbench` Q12 runs on my machine showed `async` /
`async_object_store` improving ~1–2% with `sync` neutral, but the machine's
run-to-run drift is of the same order, so I'd rather rely on the benchmark bot
for numbers.
## Are there any user-facing changes?
No. `cleanup_consumed_batches` now takes `&mut self`, but it is a private
method of a `pub(crate)` 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]
