-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Remove hppc from nested aggs #86078
Remove hppc from nested aggs #86078
Conversation
The nested and reverse nested aggs use hppc maps and lists for mapping buckets to ordinals. This commit converts these to use HashMaps and ArrayLists. relates elastic#84735
Pinging @elastic/es-analytics-geo (Team:Analytics) |
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.
I left one question. Otherwise LGTM 👍
collectBucket(sub, childDocId, buffer[i]); | ||
} | ||
final int docId = childDocId; | ||
bucketBuffer.stream().mapToLong(Long::longValue).forEach(bucket -> { |
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.
Maybe use a regular for loop here? I notice that we have been avoiding to use steam()
in hot code paths. I think we should do this here too? But Maybe I'm wrong here.
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.
I had done the stream because I was trying to avoid materializing a primitive array. But I realized now I could just iterate over the boxed values and let java do the auto unboxing on the call to collectBucket.
The nested and reverse nested aggs use hppc maps and lists for mapping buckets to
ordinals. This commit converts these to use HashMaps and ArrayLists.
relates #84735