aho135 commented on code in PR #19627:
URL: https://github.com/apache/druid/pull/19627#discussion_r3855816563
##########
processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/ByteBufferHashTable.java:
##########
@@ -442,19 +433,159 @@ public int getGrowthCount()
}
/**
- * To maintain an accurate tracking of the maximum bytes used per query,
this function is to be called immediately
- * whenever either of {@link #size} or {@link #bucketSizeWithHash} is
changed.
+ * Called whenever {@link #size} or {@link #bucketSizeWithHash} changes, to
track {@link #maxMergeBufferUsedBytes} and
+ * the {@link #maxSpillProximity} peak. Proximity is recorded while {@code
size < regrowthThreshold} (the transient hit
+ * at intermediate growth boundaries is skipped, since the table then
grows); at the terminal level, parking at the
+ * threshold is the spill point and pins 1.0. Trim-and-swap tables ({@link
#recordsFillProximity()} == false) skip
+ * proximity entirely — see {@link #findBucketWithAutoGrowth} for their only
spill signal.
*/
protected void updateMaxMergeBufferUsedBytes()
{
maxMergeBufferUsedBytes = Math.max(maxMergeBufferUsedBytes, (long) size *
bucketSizeWithHash);
+ if (!recordsFillProximity()) {
+ return;
+ }
+ final int denominator = getSpillRegrowthThreshold();
+ if (denominator <= 0) {
+ return;
+ }
+ if (size < regrowthThreshold) {
+ // size < regrowthThreshold <= terminal denominator keeps this below
1.0; the clamp is defensive.
+ final double ratio = Math.min(1.0, (double) size / denominator);
+ if (ratio > maxSpillProximity) {
+ maxSpillProximity = ratio;
+ }
+ } else if (isTerminalTableLevel()) {
+ // At the load-factor limit with no room to grow: parking here IS the
spill point.
+ maxSpillProximity = 1.0;
+ }
+ }
+
+ /**
+ * Denominator for {@link #maxSpillProximity}: the {@code regrowthThreshold}
at the terminal growth level, where
+ * {@link #findBucketWithAutoGrowth} can no longer allocate a bucket.
Computed once from fixed geometry and cached.
+ */
+ protected final int getSpillRegrowthThreshold()
+ {
+ if (spillRegrowthThreshold == 0) {
+ spillRegrowthThreshold = computeSpillRegrowthThreshold();
+ }
+ return spillRegrowthThreshold;
+ }
+
+ /**
+ * Replays the arena geometry to the terminal growth level — via the same
{@link #initialTableStart} /
+ * {@link #nextGrowthLevel} primitives {@link #reset()} and {@link
#adjustTableWhenFull()} use — and returns
+ * {@link #maxSizeForBuckets} of its bucket count. Allocates no buffers.
+ *
+ * <p>Valid only for the standard grow-by-doubling layout. Fixed-layout
variants (the alternating limit-pushdown
+ * table) must never reach this — guaranteed by {@link
#recordsFillProximity()} == false — and must override it if
+ * they ever need a spill denominator.
+ */
+ protected int computeSpillRegrowthThreshold()
Review Comment:
Done in d004d4d — both scoped to `private`. Neither is overridden (the
alternating table opts out of proximity via `recordsFillProximity()` rather
than overriding these), and they're only called within `ByteBufferHashTable`.
Also dropped the now-inapplicable "must override it" note from the Javadoc.
--
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]
