Skip to content

Commit 9f1a70a

Browse files
Minor code fixes (#2252)
- `ScoreBreakdown` map was always getting created, and not being reused. When recycling the `DocumentMatch`, clear out the score breakdown map and reuse it when needed. - Fix dropped error when creating pre-filter searcher. - Close KNN and Pre-filter Searchers when done. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent ce09d87 commit 9f1a70a

File tree

5 files changed

+38
-13
lines changed

5 files changed

+38
-13
lines changed

search/query/boolean.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package query
1616

1717
import (
18-
"bytes"
1918
"context"
2019
"encoding/json"
2120
"fmt"
@@ -203,15 +202,15 @@ func (q *BooleanQuery) Searcher(ctx context.Context, i index.IndexReader, m mapp
203202
return false
204203
}
205204
// Compare document IDs
206-
cmp := bytes.Compare(refDoc.IndexInternalID, d.IndexInternalID)
205+
cmp := refDoc.IndexInternalID.Compare(d.IndexInternalID)
207206
if cmp < 0 {
208207
// filterSearcher is behind the current document, Advance() it
209208
refDoc, err = filterSearcher.Advance(sctx, d.IndexInternalID)
210209
if err != nil || refDoc == nil {
211210
return false
212211
}
213212
// After advance, check if they're now equal
214-
return bytes.Equal(refDoc.IndexInternalID, d.IndexInternalID)
213+
cmp = refDoc.IndexInternalID.Compare(d.IndexInternalID)
215214
}
216215
// cmp >= 0: either equal (match) or filterSearcher is ahead (no match)
217216
return cmp == 0

search/scorer/scorer_disjunction.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,10 @@ func (s *DisjunctionQueryScorer) Score(ctx *search.SearchContext, constituents [
8888
func (s *DisjunctionQueryScorer) ScoreAndExplBreakdown(ctx *search.SearchContext, constituents []*search.DocumentMatch,
8989
matchingIdxs []int, originalPositions []int, countTotal int) *search.DocumentMatch {
9090

91-
scoreBreakdown := make(map[int]float64)
91+
rv := constituents[0]
92+
if rv.ScoreBreakdown == nil {
93+
rv.ScoreBreakdown = make(map[int]float64, len(constituents))
94+
}
9295
var childrenExplanations []*search.Explanation
9396
if s.options.Explain {
9497
// since we want to notify which expl belongs to which matched searcher within the disjunction searcher
@@ -104,7 +107,7 @@ func (s *DisjunctionQueryScorer) ScoreAndExplBreakdown(ctx *search.SearchContext
104107
// scorer used in disjunction heap searcher
105108
index = matchingIdxs[i]
106109
}
107-
scoreBreakdown[index] = docMatch.Score
110+
rv.ScoreBreakdown[index] = docMatch.Score
108111
if s.options.Explain {
109112
childrenExplanations[index] = docMatch.Expl
110113
}
@@ -113,9 +116,6 @@ func (s *DisjunctionQueryScorer) ScoreAndExplBreakdown(ctx *search.SearchContext
113116
if s.options.Explain {
114117
explBreakdown = &search.Explanation{Children: childrenExplanations}
115118
}
116-
117-
rv := constituents[0]
118-
rv.ScoreBreakdown = scoreBreakdown
119119
rv.Expl = explBreakdown
120120
rv.FieldTermLocations = search.MergeFieldTermLocations(
121121
rv.FieldTermLocations, constituents[1:])

search/search.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,20 +207,29 @@ func (dm *DocumentMatch) Reset() *DocumentMatch {
207207
indexInternalID := dm.IndexInternalID
208208
// remember the []interface{} used for sort
209209
sort := dm.Sort
210+
// remember the []string used for decoded sort
211+
decodedSort := dm.DecodedSort
210212
// remember the FieldTermLocations backing array
211213
ftls := dm.FieldTermLocations
212214
for i := range ftls { // recycle the ArrayPositions of each location
213215
ftls[i].Location.ArrayPositions = ftls[i].Location.ArrayPositions[:0]
214216
}
217+
// remember the score breakdown map
218+
scoreBreakdown := dm.ScoreBreakdown
219+
// clear out the score breakdown map
220+
clear(scoreBreakdown)
215221
// idiom to copy over from empty DocumentMatch (0 allocations)
216222
*dm = DocumentMatch{}
217223
// reuse the []byte already allocated (and reset len to 0)
218224
dm.IndexInternalID = indexInternalID[:0]
219225
// reuse the []interface{} already allocated (and reset len to 0)
220226
dm.Sort = sort[:0]
221-
dm.DecodedSort = dm.DecodedSort[:0]
227+
// reuse the []string already allocated (and reset len to 0)
228+
dm.DecodedSort = decodedSort[:0]
222229
// reuse the FieldTermLocations already allocated (and reset len to 0)
223230
dm.FieldTermLocations = ftls[:0]
231+
// reuse the score breakdown map already allocated (after clearing it)
232+
dm.ScoreBreakdown = scoreBreakdown
224233
return dm
225234
}
226235

search/searcher/search_knn.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func (s *KNNSearcher) VectorOptimize(ctx context.Context, octx index.VectorOptim
8484

8585
func (s *KNNSearcher) Advance(ctx *search.SearchContext, ID index.IndexInternalID) (
8686
*search.DocumentMatch, error) {
87-
knnMatch, err := s.vectorReader.Next(s.vd.Reset())
87+
knnMatch, err := s.vectorReader.Advance(ID, s.vd.Reset())
8888
if err != nil {
8989
return nil, err
9090
}

search_knn.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ func addSortAndFieldsToKNNHits(req *SearchRequest, knnHits []*search.DocumentMat
381381
return nil
382382
}
383383

384-
func (i *indexImpl) runKnnCollector(ctx context.Context, req *SearchRequest, reader index.IndexReader, preSearch bool) ([]*search.DocumentMatch, error) {
384+
func (i *indexImpl) runKnnCollector(ctx context.Context, req *SearchRequest, reader index.IndexReader, preSearch bool) (knnHits []*search.DocumentMatch, err error) {
385385
// Maps the index of a KNN query in the request to its pre-filter result:
386386
// - If the KNN query is **not filtered**, the value will be `nil`.
387387
// - If the KNN query **is filtered**, the value will be an eligible document selector
@@ -401,21 +401,33 @@ func (i *indexImpl) runKnnCollector(ctx context.Context, req *SearchRequest, rea
401401
continue
402402
}
403403
// Applies to all supported types of queries.
404-
filterSearcher, _ := filterQ.Searcher(ctx, reader, i.m, search.SearcherOptions{
404+
filterSearcher, err := filterQ.Searcher(ctx, reader, i.m, search.SearcherOptions{
405405
Score: "none", // just want eligible hits --> don't compute scores if not needed
406406
})
407+
if err != nil {
408+
return nil, err
409+
}
407410
// Using the index doc count to determine collector size since we do not
408411
// have an estimate of the number of eligible docs in the index yet.
409412
indexDocCount, err := i.DocCount()
410413
if err != nil {
414+
// close the searcher before returning
415+
filterSearcher.Close()
411416
return nil, err
412417
}
413418
filterColl := collector.NewEligibleCollector(int(indexDocCount))
414419
err = filterColl.Collect(ctx, filterSearcher, reader)
415420
if err != nil {
421+
// close the searcher before returning
422+
filterSearcher.Close()
416423
return nil, err
417424
}
418425
knnFilterResults[idx] = filterColl.EligibleSelector()
426+
// Close the filter searcher, as we are done with it.
427+
err = filterSearcher.Close()
428+
if err != nil {
429+
return nil, err
430+
}
419431
}
420432

421433
// Add the filter hits when creating the kNN query
@@ -429,12 +441,17 @@ func (i *indexImpl) runKnnCollector(ctx context.Context, req *SearchRequest, rea
429441
if err != nil {
430442
return nil, err
431443
}
444+
defer func() {
445+
if serr := knnSearcher.Close(); err == nil && serr != nil {
446+
err = serr
447+
}
448+
}()
432449
knnCollector := collector.NewKNNCollector(kArray, sumOfK)
433450
err = knnCollector.Collect(ctx, knnSearcher, reader)
434451
if err != nil {
435452
return nil, err
436453
}
437-
knnHits := knnCollector.Results()
454+
knnHits = knnCollector.Results()
438455
if !preSearch {
439456
knnHits = finalizeKNNResults(req, knnHits)
440457
}

0 commit comments

Comments
 (0)