peterxcli opened a new pull request, #5463:
URL: https://github.com/apache/datafusion-comet/pull/5463
## Which issue does this PR close?
Part of #5212 (finding #12). This does not close the parent audit epic.
Follow-up performance work for O(1) page-byte accounting is tracked in #5462.
## Rationale for this change
`CometShuffleExternalSorter.spill()` frees the active sorter's data pages
without first updating `peakMemoryUsedBytes`. If an earlier spill has a larger
working set than later spills, the final getter only observes the reset pointer
array and under-reports the actual peak.
On unmodified upstream main, a deterministic two-spill reproducer reached a
known first-spill high-water mark of 1,792 bytes but reported 1,024 bytes.
## What changes are included in this PR?
- Update peak memory immediately before an active spill sorter's pages are
freed.
- Add a focused two-spill regression whose first working set is larger and
which does not poll the peak between spills.
- Assert exact allocator/spill relationships, two spill files, record count,
disk-byte accounting, partition lengths, and cleanup.
## How are these changes tested?
- `make core`
- `./mvnw -q -Prelease test -Dtest=none
-Dsuites="org.apache.spark.sql.comet.CometTaskMetricsSuite JVM shuffle peak
memory includes a larger earlier spill" -Djacoco.skip=true`
- Exactly 1 test expected and run; 1 succeeded.
- `./mvnw -q spotless:check -DskipTests -Dscalastyle.skip=true`
- `git diff --check`
The regression failed against unmodified upstream main with `1024 did not
equal 1792` after its preceding spill-count, record-count, and exact
byte-accounting assertions passed. It passes with this change.
A warmed release-mode A-B-B-A check used 5 warmups and 15 recorded samples
per block for tiny and representative two-spill workloads. Combined medians
were 1.705 ms before versus 1.698 ms after for the tiny workload, and 52.65 ms
before versus 53.90 ms after for the representative workload; both differences
were within measured variability. Spill bytes, record counts, partition
lengths, and spill-file SHA-256 digests were unchanged across all recorded
samples.
--
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]
