tanmay442 commented on PR #26536:
URL: https://github.com/apache/datafusion/pull/26536#issuecomment-6311265607
A follow-up, because the numbers moved under me.
The version you measured was replaced. The tournament path now collects its
winners from the list of groups the batch touched, the same list the
pre-ordered path has used since #25050, so the per-batch sweep is gone rather
than made cheaper. The head of this pull request is that version as of the time
of writing.
What I can measure locally, on an x86 machine, base and change built from
the same benchmark file and compared by criterion:
| case | base | change | delta |
|---|---|---|---|
| `first_value update_bench_many_groups_distinct_batches groups=1000000,
touched=64` | 50.76 ms | 4.26 ms | -91.6% |
| `first_value update_bench_many_groups_distinct_batches groups=1000000,
touched=8192` | 89.49 ms | 38.52 ms | -57.0% |
| `last_value update_bench_many_groups_distinct_batches groups=1000000,
touched=64` | 51.45 ms | 4.29 ms | -91.7% |
| `last_value update_bench_many_groups_distinct_batches groups=1000000,
touched=8192` | 96.10 ms | 38.62 ms | -59.9% |
| `first_value update_bench_many_groups_dense groups=100000` | 403.75 ms |
402.81 ms | -0.2%, not significant |
| `last_value update_bench_many_groups_dense groups=100000` | 406.86 ms |
400.85 ms | -1.5% |
The `distinct_batches` cases are the ones to read: each batch owns a
disjoint range of groups, so every batch writes the state of every winner,
which is the work a query does. The dense case is the other end, where every
batch touches every group, and the change is flat there.
What I cannot do from here is the run that would attribute your eight rows,
because that needs a baseline that has the benchmark in it, and the benchmark
is in a separate pull request. A bench-only null run would settle it: the same
library, with the benchmark file as the only difference, so anything that moves
on those eight rows is the harness and not the code.
```yaml
run benchmark first_last
baseline:
ref: f9bc5a3fc60cc7f4319cd2c156cb3d024a8c1b21
changed:
ref: perf/first-last-bench-only
env:
BENCH_FILTER: '^last_value update_bench (list|map|struct)'
```
If that shows a spread of the same size, those eight rows are the harness.
If it shows them tight, the change does cost something in that shape, and I
will profile the two binaries and report back with data.
--
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]
