dongjoon-hyun commented on PR #58844: URL: https://github.com/apache/spark/pull/58844#issuecomment-5732488275
Thank you for addressing the retained-capacity concern, @david-mollitor-db. The shrink logic in `69bf503` looks correct to me: `append` is only reachable from `processNext()`, and `processNext()` is only invoked on an empty buffer, so `currentRows.size()` right after it is indeed the batch high-water mark. Two remaining items: 1. Could you update the PR description to match the latest commit message? It doesn't mention the `SHRINK_BUFFER_THRESHOLD` logic yet, and the merge script uses the PR description as the commit message. 2. +1 for @cloud-fan's suggestion to add a focused test that crosses the threshold (1025 rows -> fresh queue on the next refill, 1024 rows -> same queue). FYI, for a sustained fan-out above the threshold, the deque is re-grown on every batch (~13-17 bytes/row in a local microbenchmark vs. 24 bytes/row for `LinkedList`), so it is still not a regression compared to the original code. -- 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]
