zhuqi-lucas commented on PR #16196:
URL: https://github.com/apache/datafusion/pull/16196#issuecomment-2922689219
Thank you @pepijnve , i may got it now, it means:
# Per-Batch Overhead Comparison
## 1. Counter + compare
```rust
this.batches_processed += 1; // INC or ADD reg,1
if this.batches_processed == N { // CMP reg,imm + conditional jump
this.batches_processed = 0;
// yield…
}
```
- **INC/ADD** is a single register operation (~1 cycle).
- **CMP + branch** is another ~1–2 cycles (almost always predicted “no”).
- **Total**: ~2–3 cycles every batch.
## 2. AtomicBoolean.load() + compare
```rust
if ctx.is_cancelled() { // AtomicBoolean::load + CMP + branch
// wake+Pending…
}
```
- **Atomic load (Acquire)** on x86 is basically an L1 cache read: ~1–4
cycles (rarely up to ~10–20 cycles if cooling in from L2/L3).
- **CMP + branch**: ~1–2 cycles.
- **Total**: ~2–6 cycles every batch.
## Conclusion
Both patterns sit in the **same ballpark (a handful of CPU cycles)** on the
hot path. The far heavier cost is the occasional `wake_by_ref()` + context
switch, not these tiny checks.
Replacing the “count every N batches” with an `AtomicBool` check per batch
won’t introduce any meaningful overhead—and you get the benefit of a single,
clear “is_cancelled” check instead of arbitrary counters.
But, it seems we also need to add the is_cancelled logic to all streaming
exec? How can we just make it unified instead of inserting to all stream exec?
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
