-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[Groupby Query Metrics] Add merge buffer tracking #18731
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
21004b4
c935ea6
c781910
7063d09
19f6bc3
0fcb6a0
25f10d2
b6ad3c2
28719eb
59fe03c
e6020a6
507eecd
9623e3a
ae40900
400d0f4
8f7b218
df3bf70
ac71a63
003da9c
e416867
cd38a05
a26c40a
9725532
1455712
5db69c5
9ce074a
4f4a10a
e92357d
d55e402
34eb62d
988de09
ce05900
32c1ed1
08c235a
dd0267b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,8 @@ public class ByteBufferIntList | |
| private final int maxElements; | ||
| private int numElements; | ||
|
|
||
| private int maxMergeBufferUsedBytes; | ||
|
|
||
| public ByteBufferIntList( | ||
| ByteBuffer buffer, | ||
| int maxElements | ||
|
|
@@ -38,6 +40,7 @@ public ByteBufferIntList( | |
| this.buffer = buffer; | ||
| this.maxElements = maxElements; | ||
| this.numElements = 0; | ||
| this.maxMergeBufferUsedBytes = 0; | ||
|
|
||
| if (buffer.capacity() < (maxElements * Integer.BYTES)) { | ||
| throw new IAE( | ||
|
|
@@ -55,6 +58,7 @@ public void add(int val) | |
| } | ||
| buffer.putInt(numElements * Integer.BYTES, val); | ||
| numElements++; | ||
| maxMergeBufferUsedBytes = Math.max(maxMergeBufferUsedBytes, numElements * Integer.BYTES); | ||
| } | ||
|
|
||
| public void set(int index, int val) | ||
|
|
@@ -71,4 +75,9 @@ public void reset() | |
| { | ||
| numElements = 0; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Here I am intending to record the maximum usage for the entire |
||
| } | ||
|
|
||
| public int getMaxMergeBufferUsedBytes() | ||
| { | ||
| return maxMergeBufferUsedBytes; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a similar call to
updateMaxMergeBufferUsedBytes()fromadjustTableWhenFull()as well?The
LimitedBufferHashGrouperdoes that tooThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since
updateMaxMergeBufferUsedBytesis calculated bysize * bucketSizeWithHash, I added this function whereversizeorbucketSizeWithHashis changed.ByteBufferHashTable does not change both values in
adjustTableWhenFull, so I did not add that. Granted, from a code reader's perspective, the places where the max merge buffer usage is updated seems quite random...I have added a Javadoc to the related function.