Skip to content
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

Jump to bottom

Use #updateTop to speed up InternalComposite#reduce #71278

Merged
merged 2 commits into from Apr 8, 2021
Merged

Use #updateTop to speed up InternalComposite#reduce #71278

merged 2 commits into from Apr 8, 2021

Conversation

gf2121
Copy link
Contributor

@gf2121 gf2121 commented Apr 4, 2021 •
edited

The reducing in InternalComposite used the java.util.PriorityQueue and call poll() and add() for each bucket, which means heap is always rebuilt twice. This can be expensive.

We can use org.apache.lucene.util.PriorityQueue and call #updateTop() instead.

@elasticsearchmachine elasticsearchmachine added the external-contributor Pull request authored by a developer outside the Elasticsearch team label Apr 4, 2021
@cla-checker-service
Copy link

cla-checker-service bot commented Apr 4, 2021 •
edited

💚 CLA has been signed

@gf2121 gf2121 force-pushed the speed_up_InternalComposite_reduce branch from b61fe47 to 96fcdba Compare April 4, 2021 18:37
@gf2121
Copy link
Contributor Author

gf2121 commented Apr 7, 2021 •
edited

@jimczi Hi! sorry to disturb you. I looked into the commit history and find that these codes come from #26800. So i think you must be farmiliar with them. Would you please help take a look at this tiny enhancement when you are free? Thanks!

@jimczi
Copy link
Contributor

jimczi commented Apr 8, 2021

@elasticmachine ok to test

@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Apr 8, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

jimczi
jimczi approved these changes Apr 8, 2021
View reviewed changes
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks good to me. Thanks @gf2121 !

@jimczi jimczi merged commit b43f2ab into elastic:master Apr 8, 2021
@gf2121 gf2121 deleted the speed_up_InternalComposite_reduce branch April 9, 2021 03:43
jimczi pushed a commit that referenced this pull request Apr 9, 2021
@gf2121 @jimczi
Use #updateTop to speed up InternalComposite#reduce (#71278)
6c7cf91 
Co-authored-by: guofeng.my <guofeng.my@bytedance.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimczi jimczi jimczi approved these changes

Assignees
No one assigned
Labels
:Analytics/Aggregations Aggregations >enhancement external-contributor Pull request authored by a developer outside the Elasticsearch team Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v7.13.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@gf2121 @jimczi @elasticmachine @jakelandis @elasticsearchmachine