cloud-fan commented on code in PR #59764:
URL: https://github.com/apache/spark/pull/59764#discussion_r4381037332
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/HashAggregateExec.scala:
##########
@@ -680,16 +682,44 @@ case class HashAggregateExec(
outputFunc = generateResultFunction(ctx)
}
- // After the child input is consumed, finish the build: with adaptive
partial aggregation mark
- // that the child is fully consumed (to support re-entry; the map
iterators are set up inside
- // the map-output function), otherwise set up the map iterators for the
output below.
+ // `aggTime` times the build up to the point where pass-through starts,
plus the map iterator
+ // setup (`finishHashMap`, which sorts and merges any spilled data), but
never the pass-through
Review Comment:
Done in 72ae198. The comments now say the setup runs once, before the
aggregate outputs anything: for the first passed-through row, or right after
the build if no row is passed through. That also covers a flip on the last
input row.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/TungstenAggregationIterator.scala:
##########
@@ -551,16 +541,11 @@ class TungstenAggregationIterator(
}
} else {
// Adaptive partial aggregation bypassed partial aggregation: emit the
remaining input
- // rows as single-row partial buffers, after the frozen map output
above.
- if (passThroughDrainStart == -1L) {
- passThroughDrainStart = System.nanoTime()
- }
- val out = nextPassThroughOutput()
- if (!passThroughHasNext) {
- aggTime += NANOSECONDS.toMillis(System.nanoTime() -
passThroughDrainStart)
- passThroughDrainStart = -1L
- }
- out
+ // rows as single-row partial buffers, after the frozen map output
above. The drain is
Review Comment:
You're right, the interpreted drain timer made two calls per task. The
description now attributes the per-row cost to the codegen timer only. For the
interpreted timer it gives the lazy `next()` as the reason: timing from the
first to the last passed-through row also counted the downstream work between
the calls.
##########
docs/sql-performance-tuning.md:
##########
@@ -200,6 +200,9 @@ queue behind it and flushed only after it drains, so a
duplicate of a key alread
merges after that key's accumulated rows, keeping an order-sensitive aggregate
such as
`first`/`last` consistent with a run that never bypasses. The ratio is
evaluated periodically, and
again right before the aggregation map would spill, in which case the spill is
skipped entirely.
+The SQL UI counts the passed-through rows in `number of pass-through output
rows`; they are not
Review Comment:
Done in 72ae198, moved after "...instead of committing the task on a small
sample."
##########
docs/web-ui.md:
##########
@@ -379,7 +379,8 @@ Here is the list of SQL metrics:
<tr><td> <code>sort time</code> </td><td> the time spent on sorting </td><td>
Sort </td></tr>
<tr><td> <code>peak memory</code> </td><td> the peak memory usage in the
operator </td><td> Sort, HashAggregate </td></tr>
<tr><td> <code>spill size</code> </td><td> number of bytes spilled to disk
from memory in the operator </td><td> Sort, HashAggregate </td></tr>
-<tr><td> <code>time in aggregation build</code> </td><td> the time spent on
aggregation </td><td> HashAggregate, ObjectHashAggregate </td></tr>
+<tr><td> <code>time in aggregation build</code> </td><td> the time spent on
aggregation. For a HashAggregate that passes rows through (see adaptive partial
aggregation), it excludes the passed-through rows </td><td> HashAggregate,
ObjectHashAggregate </td></tr>
Review Comment:
Done in 72ae198, both rows now link the section.
--
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]
