Skip to content

Commit c950056

Browse files
capemoxclaude
andcommitted
chore: address PR review comments (comments, disjunction close, benchmarks)
- Restore the concise "clear out the score breakdown map" comment in DocumentMatch.Reset (keep the nil-guard). - Drop the redundant fast-path comment in MergeFieldTermLocations. - Split the combined err/rv check after the unadorned disjunction optimization into separate error and success returns, per review suggestion. - Remove the standalone micro-benchmark files (merge_reset_bench_test.go, disj_unadorned_bench_test.go); the measurements live in the commit messages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ea72b3f commit c950056

5 files changed

Lines changed: 13 additions & 141 deletions

File tree

index/scorch/disj_unadorned_bench_test.go

Lines changed: 0 additions & 96 deletions
This file was deleted.

search/merge_reset_bench_test.go

Lines changed: 0 additions & 31 deletions
This file was deleted.

search/search.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,9 +235,7 @@ func (dm *DocumentMatch) Reset() *DocumentMatch {
235235
}
236236
// remember the score breakdown map
237237
scoreBreakdown := dm.ScoreBreakdown
238-
// clear out the score breakdown map; nil-guard because clear(nil) still
239-
// dispatches through the map runtime (~1ns), and ScoreBreakdown is nil on
240-
// the common path (no KNN, no score-breakdown retrieval)
238+
// clear out the score breakdown map
241239
if scoreBreakdown != nil {
242240
clear(scoreBreakdown)
243241
}

search/searcher/search_disjunction.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,18 @@ func newDisjunctionSearcher(ctx context.Context, indexReader index.IndexReader,
6868
optionsDisjunctionOptimizable(options) {
6969
rv, err := optimizeCompositeSearcher(ctx, "disjunction:unadorned",
7070
indexReader, qsearchers, options)
71-
if err != nil || rv != nil {
72-
if rv != nil {
73-
// Finish() extracted all it needs (bitmaps are cloned/new).
74-
// Close the original sub-searchers so their TFRs are returned
75-
// to the snapshot per-field pool, avoiding re-allocation of
76-
// Dictionary+FST Reader objects on the next query.
77-
for _, s := range qsearchers {
78-
_ = s.Close()
79-
}
71+
if err != nil {
72+
return nil, err
73+
}
74+
if rv != nil {
75+
// Finish() extracted all it needs (bitmaps are cloned/new).
76+
// Close the original sub-searchers so their TFRs are returned
77+
// to the snapshot per-field pool, avoiding re-allocation of
78+
// Dictionary+FST Reader objects on the next query.
79+
for _, s := range qsearchers {
80+
_ = s.Close()
8081
}
81-
return rv, err
82+
return rv, nil
8283
}
8384
}
8485
}

search/util.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func MergeFieldTermLocations(dest []FieldTermLocation, matches []*DocumentMatch)
5555
}
5656
}
5757
if n == len(dest) {
58-
return dest // fast path: no constituent has field term locations to merge
58+
return dest
5959
}
6060
if cap(dest) < n {
6161
dest = append(make([]FieldTermLocation, 0, n), dest...)

0 commit comments

Comments
 (0)