-
Notifications
You must be signed in to change notification settings - Fork 697
MB-59633: Improve performance of Geospatial Search #2268
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
base: master
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR optimizes performance for geo-spatial queries by addressing heap allocation bottlenecks. The key objective is to reduce allocations in hot paths during document filtering and geo-spatial calculations.
- Reuses DocumentMatch objects in FilteringSearcher to prevent pool exhaustion
- Extracts DocValues visitor closures out of per-document loops to eliminate repeated closure allocations
- Optimizes slice filtering in snapshot_index.go to avoid unnecessary allocations
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| search/searcher/search_filter.go | Adds DocumentMatchPool.Put() calls to recycle rejected document matches, preventing pool exhaustion |
| search/searcher/search_geoshape.go | Moves dvVisitor closure outside the filter function loop to avoid per-document allocations; adds early exit optimization |
| search/searcher/search_geopolygon.go | Extracts dvVisitor and rayIntersectsSegment closures outside the filter function to eliminate repeated allocations |
| search/searcher/search_geopointdistance.go | Moves dvVisitor closure outside filter function and removes unused field parameter |
| search/searcher/search_geoboundingbox.go | Extracts dvVisitor closure outside filter function and removes unused field parameter |
| search/query/boolean.go | Adds DocumentMatchPool.Put() call to recycle refDoc when behind current document |
| index/scorch/snapshot_index.go | Optimizes slice filtering to use in-place mutation instead of allocating new slices; adds fast path for empty updatedFields |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
DocumentMatchobjects inFilteringSearcherto avoid triggeringDocumentMatchPoolTooSmallwhen documents fail the filter function and are not recycled, which previously caused repeatedDocumentMatchallocations.DocValueVisitorclosure out of the hot path to avoid per-document heap allocations caused by closure escape; the closure is now allocated once and reused, eliminatingruntime.newobjectcalls in the critical loop.DocValuesVisitor.GeoDistanceby avoiding unnecessary string and slice creation and by reusing byte buffers.