What is the problem the feature request solves?
SpillWriter.getMemoryUsage() walks every entry in allocatedPages to sum the current page bytes. This is normally cheap because shuffle pages are large, but it is called from JVM shuffle spill accounting and from the bypass-shuffle "largest writer first" selection path. A writer with many small pages can therefore pay repeated O(page count) sampling costs.
This is follow-up performance work for finding #12 in #5212. The correctness fix should remain narrowly scoped; this issue tracks whether exact O(1) page-byte accounting is worthwhile.
Using allocatedPages.size() * configuredPageSize would not be exact:
CometBoundedShuffleMemoryAllocator allocates max(pageSize, required), so an oversized record can create a larger page.
CometUnifiedShuffleMemoryAllocator can receive a smaller partial grant that still satisfies required.
- Allocator totals cannot be used generically because allocator scope differs and the bounded allocator is shared by multiple writers.
Describe the potential solution
Maintain exact writer-local allocated page bytes in SpillWriter:
- Add each successfully allocated block's actual
MemoryBlock.size() at both allocation sites.
- Subtract the actual byte count returned by
allocator.free(block) for each freed block.
- Return the tracked value from
getMemoryUsage() in O(1).
- Continue adding the pointer-array usage separately in
SpillSorter.getMemoryUsage().
The value needs cross-thread visibility because CometDiskBlockWriter.ArrowIPCWriter memory can be read while selecting another writer to spill. Avoid making the base allocation/free methods uniformly synchronized without first checking the existing currentWriters and per-writer lock ordering.
Additional context
Before implementation, establish that this improves end-to-end behavior rather than only a scan microbenchmark:
- Cover normal, oversized, and partial page allocations with exact byte assertions.
- Cover free/reset, repeated or idempotent frees, and partial-failure accounting.
- Exercise cross-thread
ArrowIPCWriter reads.
- Run warmed repeated A/B measurements for 1, 16, 256, and 1,024 pages, plus tiny and representative end-to-end shuffle spills.
- Keep the change only if the end-to-end gain is measurable and spill bytes, output, and correctness remain unchanged.
What is the problem the feature request solves?
SpillWriter.getMemoryUsage()walks every entry inallocatedPagesto sum the current page bytes. This is normally cheap because shuffle pages are large, but it is called from JVM shuffle spill accounting and from the bypass-shuffle "largest writer first" selection path. A writer with many small pages can therefore pay repeated O(page count) sampling costs.This is follow-up performance work for finding #12 in #5212. The correctness fix should remain narrowly scoped; this issue tracks whether exact O(1) page-byte accounting is worthwhile.
Using
allocatedPages.size() * configuredPageSizewould not be exact:CometBoundedShuffleMemoryAllocatorallocatesmax(pageSize, required), so an oversized record can create a larger page.CometUnifiedShuffleMemoryAllocatorcan receive a smaller partial grant that still satisfiesrequired.Describe the potential solution
Maintain exact writer-local allocated page bytes in
SpillWriter:MemoryBlock.size()at both allocation sites.allocator.free(block)for each freed block.getMemoryUsage()in O(1).SpillSorter.getMemoryUsage().The value needs cross-thread visibility because
CometDiskBlockWriter.ArrowIPCWritermemory can be read while selecting another writer to spill. Avoid making the base allocation/free methods uniformly synchronized without first checking the existingcurrentWritersand per-writer lock ordering.Additional context
Before implementation, establish that this improves end-to-end behavior rather than only a scan microbenchmark:
ArrowIPCWriterreads.