Avoid unnecessary getGraph() call #15560
Conversation
| int unfilteredVisit = HnswGraphSearcher.expectedVisitedNodes(knnCollector.k(), graph.size()); | ||
| if (unfilteredVisit >= filteredDocCount || graph.size() == 0) { | ||
| int unfilteredVisit = HnswGraphSearcher.expectedVisitedNodes(knnCollector.k(), graphSize); | ||
| if (unfilteredVisit >= filteredDocCount || graphSize == 0) { |
There was a problem hiding this comment.
What about when the building the graph is delayed? We likely need to validate entry.vectorIndexLength == 0 . right now we use the side-effect of an empty graph as a way to skip hnsw graph search.
There was a problem hiding this comment.
We are already checking that on L338. So in case of delayed graph building graphSize will be 0 because fieldEntry.vectorIndexLength == 0 (same as current) or else the size of graph (when fieldEntry.vectorIndexLength != 0) .
This tries to replicate the getGraph logic but without having to initialize the graph unnecessarily.
There was a problem hiding this comment.
I initially misread that logic. You are correct, shipit!
But, even for smaller things like this, its nice to have a CHANGES entry under "optimizations" that way we have a nice trail even outside of github commits :)
benwtrent
left a comment
benwtrent
left a comment
There was a problem hiding this comment.
It would be nice to have a CHANGES entry under optimizations. But this is good!
|
Thanks @benwtrent ! I added a changes entry. I'll ship it once all checks complete. |
2 participants
Description
We don't need to call
getGraphtwice (which then callsnew OffHeapHnswGraphto initialize graph) where we are only interested to the graph size.