From 2e9bebfe6f76c1e9d02a8e9c41071128cb8b380e Mon Sep 17 00:00:00 2001 From: Steve Yen Date: Mon, 8 Jun 2026 22:50:03 -0700 Subject: [PATCH 1/8] =?UTF-8?q?perf:=20=C2=A713=20ternary=20heap=20(3-ary)?= =?UTF-8?q?=20for=20top-N=20collector=20to=20reduce=20siftDown=20depth?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before: container/heap (binary, log₂N comparisons per siftDown) After: inline ternary heap (3 children/node, log₃N comparisons, removed dependency) The top-N collector maintains a min-heap (worst score at root) over the K best results seen so far. Every new candidate triggers a siftDown traversal from the root. A binary heap takes log₂(N) levels per traversal; a ternary heap takes log₃(N): k=10: binary ~3.3 levels → ternary ~2.1 levels (−36%) k=100: binary ~6.6 levels → ternary ~4.2 levels (−37%) k=1000: binary ~10 levels → ternary ~6.3 levels (−37%) Comparing 3 children per step rather than 1 means all three fit in 1-2 cache lines instead of 1 — fewer cache-line fetches per traversal at the cost of one extra comparison per step. Net win: shallower tree beats extra comparison. Implementation: siftUp/siftDown inline methods on collectStoreHeap using child formula 3i+1, 3i+2, 3i+3. Removes the container/heap import entirely. ~67 lines changed; zero interface changes. Measured improvement on M2 Pro (3-term BM25, MAXSCORE path): k=10: 905µs → 892µs (~1.5%) k=100: 922µs → 911µs (~1.2%) k=1000: 1395µs → 1354µs (~3%) Benefit scales with k — meaningful for top-1000, small for top-10. Composes cleanly with §1 WAND: fewer candidates reach the heap, so fewer siftDowns overall; a faster heap multiplies whatever fraction remains. Co-Authored-By: Claude Sonnet 4.6 (cherry picked from commit 2ed83c8b606db4611e52fb91cbc9fc88b32c2a63) --- search/collector/heap.go | 102 +++++++++++------ search/collector/heap_test.go | 202 ++++++++++++++++++++++++++++++++++ 2 files changed, 269 insertions(+), 35 deletions(-) create mode 100644 search/collector/heap_test.go diff --git a/search/collector/heap.go b/search/collector/heap.go index cd662bcf9..5c17a0279 100644 --- a/search/collector/heap.go +++ b/search/collector/heap.go @@ -15,85 +15,117 @@ package collector import ( - "container/heap" - "github.com/blevesearch/bleve/v2/search" ) +// collectStoreHeap is a min-heap of DocumentMatches where the root (heap[0]) +// is always the *worst* result (lowest score / highest sort key). Popping the +// root discards the worst element, so AddNotExceedingSize keeps the best k +// documents at all times. +// +// Implemented as a ternary heap (3 children per node) instead of the standard +// library's binary heap. Height is log₃(n) vs log₂(n), so siftDown is +// shallower and fetches more children per cache line. type collectStoreHeap struct { heap search.DocumentMatchCollection compare collectorCompare } func newStoreHeap(capacity int, compare collectorCompare) *collectStoreHeap { - rv := &collectStoreHeap{ + return &collectStoreHeap{ heap: make(search.DocumentMatchCollection, 0, capacity), compare: compare, } - heap.Init(rv) - return rv } func (c *collectStoreHeap) AddNotExceedingSize(doc *search.DocumentMatch, size int) *search.DocumentMatch { c.add(doc) - if c.Len() > size { + if len(c.heap) > size { return c.removeLast() } return nil } func (c *collectStoreHeap) add(doc *search.DocumentMatch) { - heap.Push(c, doc) + c.heap = append(c.heap, doc) + c.siftUp(len(c.heap) - 1) } func (c *collectStoreHeap) removeLast() *search.DocumentMatch { - return heap.Pop(c).(*search.DocumentMatch) + n := len(c.heap) + c.heap[0], c.heap[n-1] = c.heap[n-1], c.heap[0] + result := c.heap[n-1] + c.heap = c.heap[:n-1] + if len(c.heap) > 0 { + c.siftDown(0) + } + return result +} + +// siftUp restores the heap invariant after appending at index i. +// Moves element at i toward the root while it is less than (worse than) its parent. +func (c *collectStoreHeap) siftUp(i int) { + h := c.heap + for i > 0 { + parent := (i - 1) / 3 + if c.compare(h[i], h[parent]) > 0 { // h[i] is worse → should be closer to root + h[i], h[parent] = h[parent], h[i] + i = parent + } else { + break + } + } +} + +// siftDown restores the heap invariant after replacing the root. +// Moves element at i toward the leaves while any child is worse (less) than it. +func (c *collectStoreHeap) siftDown(i int) { + h := c.heap + n := len(h) + for { + first := 3*i + 1 + if first >= n { + break + } + // Find the worst (least) child among up to three children. + worst := first + if s := first + 1; s < n && c.compare(h[s], h[worst]) > 0 { + worst = s + } + if t := first + 2; t < n && c.compare(h[t], h[worst]) > 0 { + worst = t + } + if c.compare(h[worst], h[i]) > 0 { // worst child is worse than current → swap + h[i], h[worst] = h[worst], h[i] + i = worst + } else { + break + } + } } func (c *collectStoreHeap) Final(skip int, fixup collectorFixup) (search.DocumentMatchCollection, error) { - count := c.Len() + count := len(c.heap) size := count - skip if size <= 0 { return make(search.DocumentMatchCollection, 0), nil } rv := make(search.DocumentMatchCollection, size) for i := size - 1; i >= 0; i-- { - doc := heap.Pop(c).(*search.DocumentMatch) + doc := c.removeLast() rv[i] = doc - err := fixup(doc) - if err != nil { + if err := fixup(doc); err != nil { return nil, err } } return rv, nil } -func (c *collectStoreHeap) Internal() search.DocumentMatchCollection { - return c.heap -} - -// heap interface implementation - func (c *collectStoreHeap) Len() int { return len(c.heap) } -func (c *collectStoreHeap) Less(i, j int) bool { - so := c.compare(c.heap[i], c.heap[j]) - return -so < 0 -} - -func (c *collectStoreHeap) Swap(i, j int) { - c.heap[i], c.heap[j] = c.heap[j], c.heap[i] -} - -func (c *collectStoreHeap) Push(x interface{}) { - c.heap = append(c.heap, x.(*search.DocumentMatch)) -} - -func (c *collectStoreHeap) Pop() interface{} { - var rv *search.DocumentMatch - rv, c.heap = c.heap[len(c.heap)-1], c.heap[:len(c.heap)-1] - return rv +func (c *collectStoreHeap) Internal() search.DocumentMatchCollection { + return c.heap } diff --git a/search/collector/heap_test.go b/search/collector/heap_test.go new file mode 100644 index 000000000..31f1c4742 --- /dev/null +++ b/search/collector/heap_test.go @@ -0,0 +1,202 @@ +// Copyright (c) 2026 Couchbase, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + "testing" + + "github.com/blevesearch/bleve/v2/search" +) + +// scoreDesc is a collectorCompare for score-descending order. +// Returns positive when a.Score < b.Score (a is worse — lower score — and +// should sit closer to the root of the min-heap). +func scoreDesc(a, b *search.DocumentMatch) int { + if a.Score < b.Score { + return 1 + } + if a.Score > b.Score { + return -1 + } + return 0 +} + +func makeScoreDoc(score float64) *search.DocumentMatch { + return &search.DocumentMatch{Score: score} +} + +// checkTernaryInvariant verifies the ternary min-heap property: for every node +// i the parent (at (i-1)/3) must be as bad or worse than the child, i.e. +// compare(parent, child) >= 0. +func checkTernaryInvariant(t *testing.T, h *collectStoreHeap) { + t.Helper() + for i := 1; i < len(h.heap); i++ { + parent := (i - 1) / 3 + if h.compare(h.heap[parent], h.heap[i]) < 0 { + t.Errorf("ternary heap invariant violated at index %d (score=%.2f): parent %d (score=%.2f) is better, should be at least as bad", + i, h.heap[i].Score, parent, h.heap[parent].Score) + } + } +} + +// TestTernaryHeapInvariantAfterInserts inserts scores in several orderings and +// verifies the ternary invariant holds after every insertion. +func TestTernaryHeapInvariantAfterInserts(t *testing.T) { + for _, scores := range [][]float64{ + {5, 3, 8, 1, 7, 2, 9, 4, 6, 10}, // unsorted + {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, // ascending + {10, 9, 8, 7, 6, 5, 4, 3, 2, 1}, // descending + {3, 3, 3, 3}, // all equal + } { + h := newStoreHeap(len(scores), scoreDesc) + for _, s := range scores { + h.add(makeScoreDoc(s)) + checkTernaryInvariant(t, h) + } + // Root must be the minimum score in the heap. + min := scores[0] + for _, s := range scores[1:] { + if s < min { + min = s + } + } + if h.heap[0].Score != min { + t.Errorf("root score %.2f, want min %.2f (scores %v)", h.heap[0].Score, min, scores) + } + } +} + +// TestTernaryHeapRemoveLastAscending verifies that successive removeLast calls +// return elements in ascending score order (worst first = heap-sort property). +func TestTernaryHeapRemoveLastAscending(t *testing.T) { + scores := []float64{5, 3, 8, 1, 7, 2, 9, 4, 6, 10} + h := newStoreHeap(len(scores), scoreDesc) + for _, s := range scores { + h.add(makeScoreDoc(s)) + } + + prev := -1.0 + for len(h.heap) > 0 { + doc := h.removeLast() + if doc.Score < prev { + t.Errorf("removeLast out of order: got %.2f after %.2f", doc.Score, prev) + } + prev = doc.Score + checkTernaryInvariant(t, h) + } +} + +// TestTernaryHeapAddNotExceedingSize checks that size enforcement works: +// adding more than k docs keeps the best k and evicts the rest. +func TestTernaryHeapAddNotExceedingSize(t *testing.T) { + const k = 5 + h := newStoreHeap(k, scoreDesc) + for i := 1; i <= 10; i++ { + evicted := h.AddNotExceedingSize(makeScoreDoc(float64(i)), k) + if i <= k { + if evicted != nil { + t.Errorf("insert %d: expected no eviction, got score %.2f", i, evicted.Score) + } + } else { + if evicted == nil { + t.Errorf("insert %d: expected eviction", i) + } + } + checkTernaryInvariant(t, h) + } + if h.Len() != k { + t.Errorf("heap size %d, want %d", h.Len(), k) + } + // The heap should hold the best k scores (6..10); root is their minimum. + if h.heap[0].Score != 6.0 { + t.Errorf("root score %.2f, want 6.00 (worst of top-%d)", h.heap[0].Score, k) + } +} + +// TestTernaryHeapFinalOrder verifies that Final(0, ...) returns results in +// descending score order (best first). +func TestTernaryHeapFinalOrder(t *testing.T) { + scores := []float64{3, 1, 4, 1, 5, 9, 2, 6, 5, 3} + h := newStoreHeap(len(scores), scoreDesc) + for _, s := range scores { + h.add(makeScoreDoc(s)) + } + + fixup := func(*search.DocumentMatch) error { return nil } + result, err := h.Final(0, fixup) + if err != nil { + t.Fatal(err) + } + if len(result) != len(scores) { + t.Fatalf("len(result)=%d, want %d", len(result), len(scores)) + } + for i := 1; i < len(result); i++ { + if result[i].Score > result[i-1].Score { + t.Errorf("result[%d]=%.2f > result[%d]=%.2f (not sorted descending)", + i, result[i].Score, i-1, result[i-1].Score) + } + } +} + +// TestTernaryHeapFinalWithSkip verifies that Final(skip, ...) skips the skip +// worst results and returns the rest best-first. +func TestTernaryHeapFinalWithSkip(t *testing.T) { + // Heap holds 1..5; skip=2 should return scores [3, 2, 1] (skipping 4 and 5). + scores := []float64{1, 2, 3, 4, 5} + h := newStoreHeap(len(scores), scoreDesc) + for _, s := range scores { + h.add(makeScoreDoc(s)) + } + + fixup := func(*search.DocumentMatch) error { return nil } + result, err := h.Final(2, fixup) + if err != nil { + t.Fatal(err) + } + if len(result) != 3 { + t.Fatalf("len(result)=%d, want 3", len(result)) + } + want := []float64{3, 2, 1} + for i, w := range want { + if result[i].Score != w { + t.Errorf("result[%d]=%.2f, want %.2f", i, result[i].Score, w) + } + } +} + +// TestTernaryHeapLargeN exercises multiple levels of siftDown (n > 3^3 = 27). +func TestTernaryHeapLargeN(t *testing.T) { + const n = 200 + h := newStoreHeap(n, scoreDesc) + for i := n; i >= 1; i-- { // insert descending to stress siftUp + h.add(makeScoreDoc(float64(i))) + checkTernaryInvariant(t, h) + } + + prev := -1.0 + count := 0 + for len(h.heap) > 0 { + doc := h.removeLast() + if doc.Score < prev { + t.Errorf("removeLast out of order at position %d: %.2f after %.2f", count, doc.Score, prev) + } + prev = doc.Score + count++ + checkTernaryInvariant(t, h) + } + if count != n { + t.Errorf("extracted %d elements, want %d", count, n) + } +} From 93ae177f2e04ca174dd3d0d5d71f9cbc0c8e7d6e Mon Sep 17 00:00:00 2001 From: Steve Yen Date: Fri, 19 Jun 2026 21:06:39 -0700 Subject: [PATCH 2/8] perf: sort.Slice in collectStoreHeap Final (replaces heapsort extraction) collectStoreHeap.Final was 11.6% of total CPU when profiling k=1000 BM25 queries. Extracting k docs via repeated removeLast (heapsort) requires O(k log3 k) comparisons with scattered pointer dereferences at each heap level - poor cache behavior. Fix: sort.Slice in-place (pdqsort) then copy heap[skip..skip+size-1] sequentially. Final cum cost: 1.29s -> 0.52s (-60%). Note: the original perf-gar commit (89fa0a17 on perf-gar-v17-only) also specialized the score-descending comparator; that half is already on master via MB-72489 (#2381) as getOptimalCollectorCompare, so this cherry-pick carries only the Final() change. Co-Authored-By: Claude Sonnet 4.6 (cherry picked from commit 89fa0a1797d951e1b472ec4d5b4df913ee88ad8e) --- search/collector/heap.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/search/collector/heap.go b/search/collector/heap.go index 5c17a0279..6a547bb96 100644 --- a/search/collector/heap.go +++ b/search/collector/heap.go @@ -15,6 +15,8 @@ package collector import ( + "sort" + "github.com/blevesearch/bleve/v2/search" ) @@ -111,9 +113,16 @@ func (c *collectStoreHeap) Final(skip int, fixup collectorFixup) (search.Documen if size <= 0 { return make(search.DocumentMatchCollection, 0), nil } + // Sort in-place so heap[0] = best doc (compare < 0 means "better"). + // pdqsort (sort.Slice) has much better cache behavior than repeated + // removeLast (heapsort) because it accesses the array sequentially, + // avoiding the scattered pointer dereferences heapsort pays at each level. + sort.Slice(c.heap, func(i, j int) bool { + return c.compare(c.heap[i], c.heap[j]) < 0 + }) rv := make(search.DocumentMatchCollection, size) - for i := size - 1; i >= 0; i-- { - doc := c.removeLast() + for i := 0; i < size; i++ { + doc := c.heap[skip+i] rv[i] = doc if err := fixup(doc); err != nil { return nil, err From 4c3e468e5ee3a7ec029c4b8cbcd8c60eae24b11f Mon Sep 17 00:00:00 2001 From: Steve Yen Date: Sat, 20 Jun 2026 10:07:05 -0700 Subject: [PATCH 3/8] test: cover collectStoreList linked-list store (pre-perf-gar gap) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds 9 unit tests for the collectStoreList type in search/collector that had 0% coverage before this commit. Tests cover round-trip insertion order, size-capped eviction (AddNotExceedingSize), skip-based pagination (Final), Internal() ascending traversal, removeLast worst-doc eviction, single-element and equal-score edge cases, and fixup error propagation — all using existing scoreDesc / makeScoreDoc helpers from heap_test.go. These tests have no dependency on any perf-gar change and can be rebased or cherry-picked independently. Co-Authored-By: Claude Sonnet 4.6 (cherry picked from commit 7de10812a3222862126a9c1e5961143f88f504fd) --- search/collector/list_test.go | 218 ++++++++++++++++++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100644 search/collector/list_test.go diff --git a/search/collector/list_test.go b/search/collector/list_test.go new file mode 100644 index 000000000..e8a2ab240 --- /dev/null +++ b/search/collector/list_test.go @@ -0,0 +1,218 @@ +// Copyright (c) 2026 Couchbase, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + "errors" + "testing" + + "github.com/blevesearch/bleve/v2/search" +) + +var errTestFixup = errors.New("fixup error") + +// noFixup is a no-op fixup function used throughout the list tests. +var noFixup collectorFixup = func(*search.DocumentMatch) error { return nil } + +// TestCollectStoreListRoundTrip verifies that add() keeps elements in +// ascending-score order (Front=worst, Back=best) and that Final(0, ...) returns +// them best-first. This exercises the core insertion-sort invariant of the +// linked-list store, which had 0% coverage before this test. +func TestCollectStoreListRoundTrip(t *testing.T) { + l := newStoreList(20, scoreDesc) + for _, s := range []float64{3, 1, 4, 1, 5, 9, 2, 6} { + l.add(makeScoreDoc(s)) + } + if l.len() != 8 { + t.Fatalf("len=%d want 8", l.len()) + } + result, err := l.Final(0, noFixup) + if err != nil { + t.Fatal(err) + } + if len(result) != 8 { + t.Fatalf("Final len=%d want 8", len(result)) + } + for i := 1; i < len(result); i++ { + if result[i].Score > result[i-1].Score { + t.Errorf("Final not descending: result[%d]=%.2f > result[%d]=%.2f", + i, result[i].Score, i-1, result[i-1].Score) + } + } +} + +// TestCollectStoreListAddNotExceedingSize verifies that AddNotExceedingSize caps +// the list at k elements by evicting the worst (lowest-score) element. +func TestCollectStoreListAddNotExceedingSize(t *testing.T) { + const k = 3 + l := newStoreList(k, scoreDesc) + var evictedScores []float64 + for _, s := range []float64{1, 5, 3, 7, 2} { + ev := l.AddNotExceedingSize(makeScoreDoc(s), k) + if ev != nil { + evictedScores = append(evictedScores, ev.Score) + } + } + if l.len() != k { + t.Fatalf("list len=%d want %d after capping at k", l.len(), k) + } + // Inserted {1,5,3,7,2} with k=3 → evicted the 2 worst: 1 and 2. + if len(evictedScores) != 2 { + t.Fatalf("evicted %d docs want 2", len(evictedScores)) + } + // Remaining best-3: {7, 5, 3} in descending order. + result, err := l.Final(0, noFixup) + if err != nil { + t.Fatal(err) + } + want := []float64{7, 5, 3} + for i, w := range want { + if result[i].Score != w { + t.Errorf("result[%d]=%.2f want %.2f", i, result[i].Score, w) + } + } +} + +// TestCollectStoreListSkip verifies that Final(skip, ...) skips the top-skip +// best results and returns the remaining docs in descending order. +// This models pagination: skip=page*pageSize to start at a later page. +func TestCollectStoreListSkip(t *testing.T) { + l := newStoreList(20, scoreDesc) + for _, s := range []float64{1, 2, 3, 4, 5} { + l.add(makeScoreDoc(s)) + } + // skip=2 omits the 2 best (scores 5 and 4) → returns [3, 2, 1]. + result, err := l.Final(2, noFixup) + if err != nil { + t.Fatal(err) + } + if len(result) != 3 { + t.Fatalf("Final(skip=2) len=%d want 3", len(result)) + } + want := []float64{3, 2, 1} + for i, w := range want { + if result[i].Score != w { + t.Errorf("result[%d]=%.2f want %.2f", i, result[i].Score, w) + } + } +} + +// TestCollectStoreListSkipAll verifies Final returns empty when skip ≥ len. +func TestCollectStoreListSkipAll(t *testing.T) { + l := newStoreList(10, scoreDesc) + for _, s := range []float64{1, 2, 3} { + l.add(makeScoreDoc(s)) + } + result, err := l.Final(10, noFixup) // skip > len + if err != nil { + t.Fatal(err) + } + if len(result) != 0 { + t.Errorf("Final(skip=10) on 3-elem list returned %d docs, want 0", len(result)) + } +} + +// TestCollectStoreListInternal verifies Internal() returns all elements in +// ascending-score order (Front to Back of the linked list). +func TestCollectStoreListInternal(t *testing.T) { + l := newStoreList(10, scoreDesc) + for _, s := range []float64{3, 1, 4} { + l.add(makeScoreDoc(s)) + } + iv := l.Internal() + if len(iv) != 3 { + t.Fatalf("Internal len=%d want 3", len(iv)) + } + // Linked list: Front=worst→Back=best, so Internal() iterates Front→Back = ascending. + want := []float64{1, 3, 4} + for i, w := range want { + if iv[i].Score != w { + t.Errorf("Internal[%d]=%.2f want %.2f (ascending from worst)", i, iv[i].Score, w) + } + } +} + +// TestCollectStoreListRemoveLast verifies removeLast removes the Front element, +// which is the worst (lowest-score) document in the list. +func TestCollectStoreListRemoveLast(t *testing.T) { + l := newStoreList(10, scoreDesc) + for _, s := range []float64{3, 1, 5} { + l.add(makeScoreDoc(s)) + } + evicted := l.removeLast() + if evicted.Score != 1 { + t.Errorf("removeLast returned score=%.2f, want 1.0 (the worst)", evicted.Score) + } + if l.len() != 2 { + t.Errorf("len=%d after removeLast, want 2", l.len()) + } +} + +// TestCollectStoreListSingleElement verifies that a list with one element +// round-trips correctly through add / Final / Internal. +func TestCollectStoreListSingleElement(t *testing.T) { + l := newStoreList(5, scoreDesc) + l.add(makeScoreDoc(7.5)) + + result, err := l.Final(0, noFixup) + if err != nil { + t.Fatal(err) + } + if len(result) != 1 || result[0].Score != 7.5 { + t.Errorf("single-element Final: got %v", result) + } + + iv := l.Internal() + if len(iv) != 1 || iv[0].Score != 7.5 { + t.Errorf("single-element Internal: got %v", iv) + } +} + +// TestCollectStoreListEqualScores verifies correct handling of equal-scored +// documents: they should all be retained, and Final preserves their relative +// insertion order within equal-scored groups. +func TestCollectStoreListEqualScores(t *testing.T) { + l := newStoreList(10, scoreDesc) + for range 5 { + l.add(makeScoreDoc(3.0)) + } + if l.len() != 5 { + t.Fatalf("len=%d after 5 equal-scored adds, want 5", l.len()) + } + result, err := l.Final(0, noFixup) + if err != nil { + t.Fatal(err) + } + for _, dm := range result { + if dm.Score != 3.0 { + t.Errorf("expected all scores=3.0, got %.2f", dm.Score) + } + } +} + +// TestCollectStoreListFixupError verifies that an error returned by the fixup +// function propagates correctly from Final. +func TestCollectStoreListFixupError(t *testing.T) { + l := newStoreList(10, scoreDesc) + l.add(makeScoreDoc(1.0)) + + errFixup := func(*search.DocumentMatch) error { + return errTestFixup + } + _, err := l.Final(0, errFixup) + if err != errTestFixup { + t.Errorf("Final fixup error not propagated: got %v", err) + } +} From c7d06794ef321031eff3d5c35a1936824c620bab Mon Sep 17 00:00:00 2001 From: Steve Yen Date: Sat, 25 Jul 2026 10:13:16 -0700 Subject: [PATCH 4/8] perf: zero only the non-restored fields in DocumentMatch.Reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reset saves five backing arrays and a map, wipes the struct with *dm = DocumentMatch{}, then restores them. That wipe is a ~240-byte duffzero, and most of what it clears is either about to be overwritten or already zero. Zero the nine fields that are NOT restored below instead. Same postconditions, no duffzero. Reset runs once per collected document, so this is on the hottest per-document path there is — it shows up on single-term queries, not just disjunctions. Split out of perf-gar-mod-fix-2's 5d9689fb, which bundled this with a §15 minSegCeiling guard. The two are unrelated: the guard needs the per-segment score ceilings and therefore the v18 MaxTFNorm sidecar, while this is pure in-memory work that applies to any segment format. Only this half belongs on a v17-only branch. Co-Authored-By: Claude Opus 5 (1M context) --- search/search.go | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/search/search.go b/search/search.go index b89f8acc0..083978aa2 100644 --- a/search/search.go +++ b/search/search.go @@ -222,18 +222,14 @@ OUTER: // Reset allows an already allocated DocumentMatch to be reused func (dm *DocumentMatch) Reset() *DocumentMatch { - // remember the []byte used for the IndexInternalID + // Save backing arrays for reuse — these are restored below. indexInternalID := dm.IndexInternalID - // remember the []interface{} used for sort - sort := dm.Sort - // remember the []string used for decoded sort + sortBuf := dm.Sort decodedSort := dm.DecodedSort - // remember the FieldTermLocations backing array ftls := dm.FieldTermLocations for i := range ftls { // recycle the ArrayPositions of each location ftls[i].Location.ArrayPositions = ftls[i].Location.ArrayPositions[:0] } - // remember the score breakdown map scoreBreakdown := dm.ScoreBreakdown // clear out the score breakdown map if scoreBreakdown != nil { @@ -244,19 +240,24 @@ func (dm *DocumentMatch) Reset() *DocumentMatch { for i := range descendants { // recycle each IndexInternalID descendants[i] = descendants[i][:0] } - // idiom to copy over from empty DocumentMatch (0 allocations) - *dm = DocumentMatch{} - // reuse the []byte already allocated (and reset len to 0) + // Zero only the fields that are NOT restored below. This avoids a + // full-struct duffzero (~240 bytes) for the common case where most + // fields are already nil/zero. + dm.Index = "" + dm.ID = "" + dm.Score = 0 + dm.Expl = nil + dm.Locations = nil + dm.Fragments = nil + dm.Fields = nil + dm.HitNumber = 0 + dm.IndexNames = nil + // Restore reusable allocations. dm.IndexInternalID = indexInternalID[:0] - // reuse the []interface{} already allocated (and reset len to 0) - dm.Sort = sort[:0] - // reuse the []string already allocated (and reset len to 0) + dm.Sort = sortBuf[:0] dm.DecodedSort = decodedSort[:0] - // reuse the FieldTermLocations already allocated (and reset len to 0) dm.FieldTermLocations = ftls[:0] - // reuse the Descendants already allocated (and reset len to 0) dm.Descendants = descendants[:0] - // reuse the score breakdown map already allocated (after clearing it) dm.ScoreBreakdown = scoreBreakdown return dm } From f7e1a7c17780be0cc7aca62aaeeb54a950af72b5 Mon Sep 17 00:00:00 2001 From: Steve Yen Date: Sat, 20 Jun 2026 01:11:36 -0700 Subject: [PATCH 5/8] perf: nil-guard field zeroing in DocumentMatch.Reset (GC write barriers) Guard pointer/string/map field zeroing in Reset with nil/empty checks so the common case (no explain, no fragments, no field highlights) skips nil-to-nil stores and their GC write barriers (~5 cycles per field, across the millions of Reset calls in a large result collection). Note: the original perf-gar commit (1b30de45 on perf-gar-v17-only) also added the prepareDocumentMatch fast path and the non-KNN adjustDocumentMatch guard; both are already on master via MB-72489 (#2381) as canFastPrepare() and adjustKNNDocumentMatch, so this cherry-pick carries only the Reset() change. Co-Authored-By: Claude Sonnet 4.6 (cherry picked from commit 1b30de459b2cac9702288e1f42ef9cefc7f7a9e6) --- search/search.go | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/search/search.go b/search/search.go index 083978aa2..a39adb733 100644 --- a/search/search.go +++ b/search/search.go @@ -240,18 +240,33 @@ func (dm *DocumentMatch) Reset() *DocumentMatch { for i := range descendants { // recycle each IndexInternalID descendants[i] = descendants[i][:0] } - // Zero only the fields that are NOT restored below. This avoids a - // full-struct duffzero (~240 bytes) for the common case where most - // fields are already nil/zero. - dm.Index = "" - dm.ID = "" + // Zero only the fields that are NOT restored below. Nil/empty guards on + // pointer/string/map fields avoid triggering a GC write barrier for nil→nil + // stores, saving ~5 cycles per field across the many millions of Reset calls + // (common case: no explain, no fragments, no field highlights). + if dm.Index != "" { + dm.Index = "" + } + if dm.ID != "" { + dm.ID = "" + } dm.Score = 0 - dm.Expl = nil - dm.Locations = nil - dm.Fragments = nil - dm.Fields = nil + if dm.Expl != nil { + dm.Expl = nil + } + if dm.Locations != nil { + dm.Locations = nil + } + if dm.Fragments != nil { + dm.Fragments = nil + } + if dm.Fields != nil { + dm.Fields = nil + } dm.HitNumber = 0 - dm.IndexNames = nil + if dm.IndexNames != nil { + dm.IndexNames = nil + } // Restore reusable allocations. dm.IndexInternalID = indexInternalID[:0] dm.Sort = sortBuf[:0] From 6050b0c0ecb73ca0158445ea51de9d3489ea4915 Mon Sep 17 00:00:00 2001 From: Steve Yen Date: Sat, 25 Jul 2026 10:17:27 -0700 Subject: [PATCH 6/8] collector: early-stop (bounded scan) for score=none + Size=k MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With Score="none" and a bounded Size, the request means "return any Size+From matching documents" — nothing about the result depends on the matches beyond that. So the collector can stop pulling from the searcher once it has them, instead of draining the entire match set to count it. On a 500k-doc corpus this is the difference between scanning every match and scanning ten: BenchmarkEarlyStopTermTier1ScoreNone goes 18.15ms -> 8.4us and EarlyStopDisjMin2ScoreNone 41.79ms -> 22.6us on the full branch. Because the scan really does stop counting, Total becomes a lower bound, and saying otherwise would silently mislead callers. This adds SearchResult.TotalRelation ("eq" | "gte") to report that, plus the merge rule that any constituent reporting "gte" makes a merged Total "gte" too. Gated on the result being independent of unseen documents — no facets (every match must be counted into the buckets), no KNN (separate hit set), no SearchAfter (the cursor depends on the full ordering), no nested rollup, no reverse execution, and sort-by-score only (which degrades to arrival order under score="none"; a field sort could have its top-k anywhere in the match set). Two deviations from perf-gar-mod-fix-2's version of this work, both because that branch's WAND machinery is not present here: - TotalRelation is introduced here rather than in 0f565526, which bundled the public API with the WANDPruned collector plumbing and the disjunction-searcher changes that set it. On this branch the bounded scan is the only producer of a lower-bound Total, so the API arrives with its first user and nothing about it is dead. - index_impl derives TotalRelation from coll.EarlyStopped() alone, not "coll.WANDPruned() || coll.EarlyStopped()". The upstream commit shipped no tests. Added earlystop_test.go, because the preconditions above are the whole correctness argument and getting one wrong is a silent wrong-answer bug rather than a slowdown: it checks that a bounded scan still returns Size hits and reports "gte", that every returned hit is a real match, and that the scan does NOT engage for facets, field sort, SearchAfter or scoring — each of which must still report an exact Total. Co-Authored-By: Claude Opus 5 (1M context) --- earlystop_test.go | 184 +++++++++++++++++++++++++++++++++++++++ index_impl.go | 34 ++++++-- search.go | 30 +++++-- search/collector/topn.go | 34 ++++++++ 4 files changed, 269 insertions(+), 13 deletions(-) create mode 100644 earlystop_test.go diff --git a/earlystop_test.go b/earlystop_test.go new file mode 100644 index 000000000..150dba78d --- /dev/null +++ b/earlystop_test.go @@ -0,0 +1,184 @@ +// Copyright (c) 2026 Couchbase, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package bleve + +import ( + "fmt" + "os" + "testing" + + "github.com/blevesearch/bleve/v2/mapping" + "github.com/blevesearch/bleve/v2/search/query" +) + +// buildEarlyStopIndex indexes n documents that all match "common", so a bounded +// scan has plenty of matches left undrained. +func buildEarlyStopIndex(t *testing.T, n int) Index { + t.Helper() + + dir, err := os.MkdirTemp("", "earlystop") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(dir) }) + + im := mapping.NewIndexMapping() + im.DefaultAnalyzer = "standard" + idx, err := New(dir+"/i.bleve", im) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = idx.Close() }) + + batch := idx.NewBatch() + for i := 0; i < n; i++ { + if err := batch.Index(fmt.Sprintf("d%05d", i), map[string]interface{}{ + "body": fmt.Sprintf("common tag%d", i%10), + "num": float64(i), + }); err != nil { + t.Fatal(err) + } + } + if err := idx.Batch(batch); err != nil { + t.Fatal(err) + } + return idx +} + +func earlyStopQuery() query.Query { + q := query.NewTermQuery("common") + q.SetField("body") + return q +} + +// TestEarlyStopBoundedScan is the contract for the bounded scan: with +// Score="none" and a bounded Size, the request means "any Size+From matching +// docs", so collection may stop once that many hits are in hand instead of +// draining every match. +// +// Two things must hold. The caller must still get Size hits — stopping early must +// not lose results it asked for. And Total must be reported as a lower bound +// (TotalRelation "gte"), because the scan genuinely did not count the rest; a +// caller that reads Total as exact would otherwise be silently misled. +func TestEarlyStopBoundedScan(t *testing.T) { + const n = 5000 + idx := buildEarlyStopIndex(t, n) + + for _, size := range []int{1, 10, 100} { + req := NewSearchRequest(earlyStopQuery()) + req.Size = size + req.Score = ScoreNone + res, err := idx.Search(req) + if err != nil { + t.Fatal(err) + } + if len(res.Hits) != size { + t.Errorf("size=%d: got %d hits, want %d — early stop dropped results the "+ + "caller asked for", size, len(res.Hits), size) + } + if res.TotalRelation != TotalRelationGte { + t.Errorf("size=%d: TotalRelation=%q, want %q — Total is a lower bound once "+ + "the scan stops early, and saying otherwise misleads the caller", + size, res.TotalRelation, TotalRelationGte) + } + if res.Total > uint64(n) { + t.Errorf("size=%d: Total=%d exceeds the corpus size %d", size, res.Total, n) + } + } +} + +// TestEarlyStopDoesNotEngageWhenUnsafe pins the preconditions. Each of these +// requests depends on documents the bounded scan would never look at, so it must +// keep draining and report an exact Total. Getting any of these wrong is a silent +// wrong-answer bug, not a slowdown: +// +// facets every match must be counted into the facet buckets +// field sort the top-k by field value can lie anywhere in the match set +// SearchAfter the cursor position depends on the full ordering +// scoring Score != "none" means order depends on scores, not arrival +func TestEarlyStopDoesNotEngageWhenUnsafe(t *testing.T) { + const n = 2000 + idx := buildEarlyStopIndex(t, n) + + cases := []struct { + name string + tweak func(*SearchRequest) + }{ + {"facets", func(r *SearchRequest) { + r.Score = ScoreNone + r.AddFacet("tags", NewFacetRequest("body", 5)) + }}, + {"field-sort", func(r *SearchRequest) { + r.Score = ScoreNone + r.SortBy([]string{"num"}) + }}, + {"search-after", func(r *SearchRequest) { + r.Score = ScoreNone + r.SortBy([]string{"_id"}) + r.SearchAfter = []string{"d00010"} + }}, + {"scoring-enabled", func(r *SearchRequest) { + // Score defaults to full scoring; ordering depends on scores. + }}, + } + + for _, tc := range cases { + req := NewSearchRequest(earlyStopQuery()) + req.Size = 10 + tc.tweak(req) + res, err := idx.Search(req) + if err != nil { + t.Fatalf("%s: %v", tc.name, err) + } + if res.TotalRelation != TotalRelationEq { + t.Errorf("%s: TotalRelation=%q, want %q — the bounded scan engaged on a "+ + "request whose result depends on documents it would not visit", + tc.name, res.TotalRelation, TotalRelationEq) + } + if res.Total != uint64(n) { + t.Errorf("%s: Total=%d, want the exact %d", tc.name, res.Total, n) + } + } +} + +// TestEarlyStopHitsAreRealMatches guards the cheapest way for a bounded scan to be +// wrong: returning documents that do not match. Order is not part of the contract +// under Score="none", but membership is. +func TestEarlyStopHitsAreRealMatches(t *testing.T) { + idx := buildEarlyStopIndex(t, 1000) + + q := query.NewTermQuery("tag3") + q.SetField("body") + req := NewSearchRequest(q) + req.Size = 20 + req.Score = ScoreNone + res, err := idx.Search(req) + if err != nil { + t.Fatal(err) + } + if len(res.Hits) != 20 { + t.Fatalf("got %d hits, want 20", len(res.Hits)) + } + // tag3 was indexed on every doc where i%10 == 3. + for _, h := range res.Hits { + var i int + if _, err := fmt.Sscanf(h.ID, "d%05d", &i); err != nil { + t.Fatalf("unexpected id %q: %v", h.ID, err) + } + if i%10 != 3 { + t.Errorf("id %s does not match tag3 — the bounded scan returned a non-match", h.ID) + } + } +} diff --git a/index_impl.go b/index_impl.go index 1da655512..bad6d2fb2 100644 --- a/index_impl.go +++ b/index_impl.go @@ -786,6 +786,23 @@ func (i *indexImpl) SearchInContext(ctx context.Context, req *SearchRequest) (sr return nil, err } + // Early-stop (bounded scan): for score="none" + Size, the request means + // "return any Size+From matching docs", so the collector can stop pulling + // from the searcher once that many hits are in hand instead of draining the + // full result set. Valid only when result identity does not depend on unseen + // docs: no facets (need every match counted), no KNN (separate hit set), no + // pagination cursor, no nested rollup, and sort-by-score only (degrades to + // insertion order under score="none"; a field sort would need all docs). + if req.Score == ScoreNone && req.Size > 0 && + len(req.Facets) == 0 && + !requestHasKNN(req) && + req.SearchAfter == nil && !reverseQueryExecution && + len(req.Sort) == 1 && req.Sort[0].RequiresScoring() { + if nestedMode, ok := ctx.Value(search.NestedSearchKey).(bool); !ok || !nestedMode { + coll.SetEarlyStop(req.Size + req.From) + } + } + var knnHits []*search.DocumentMatch var skipKNNCollector bool @@ -1036,16 +1053,23 @@ func (i *indexImpl) SearchInContext(ctx context.Context, req *SearchRequest) (sr req.SearchAfter = nil } + totalRelation := TotalRelationEq + if coll.EarlyStopped() { + // Total is a lower bound: the early-stop bounded scan stopped before + // draining all matches. + totalRelation = TotalRelationGte + } rv := &SearchResult{ Status: &SearchStatus{ Total: 1, Successful: 1, }, - Hits: hits, - Total: coll.Total(), - MaxScore: coll.MaxScore(), - Took: searchDuration, - Facets: coll.FacetResults(), + Hits: hits, + Total: coll.Total(), + TotalRelation: totalRelation, + MaxScore: coll.MaxScore(), + Took: searchDuration, + Facets: coll.FacetResults(), } // rescore if fusion flag is set diff --git a/search.go b/search.go index 708be0871..52535a15c 100644 --- a/search.go +++ b/search.go @@ -525,6 +525,15 @@ func (ss *SearchStatus) Merge(other *SearchStatus) { } } +// TotalRelation constants describe the accuracy of SearchResult.Total. +const ( + // TotalRelationEq means Total is an exact count of all matching documents. + TotalRelationEq = "eq" + // TotalRelationGte means Total is a lower bound: collection stopped before + // draining every match, so the true match count is >= Total. + TotalRelationGte = "gte" +) + // A SearchResult describes the results of executing // a SearchRequest. // @@ -540,14 +549,15 @@ func (ss *SearchStatus) Merge(other *SearchStatus) { // Took - The time taken to execute the search. // Facets - The facet results for the search. type SearchResult struct { - Status *SearchStatus `json:"status"` - Request *SearchRequest `json:"request,omitempty"` - Hits search.DocumentMatchCollection `json:"hits"` - Total uint64 `json:"total_hits"` - Cost uint64 `json:"cost"` - MaxScore float64 `json:"max_score"` - Took time.Duration `json:"took"` - Facets search.FacetResults `json:"facets"` + Status *SearchStatus `json:"status"` + Request *SearchRequest `json:"request,omitempty"` + Hits search.DocumentMatchCollection `json:"hits"` + Total uint64 `json:"total_hits"` + TotalRelation string `json:"total_relation"` + Cost uint64 `json:"cost"` + MaxScore float64 `json:"max_score"` + Took time.Duration `json:"took"` + Facets search.FacetResults `json:"facets"` // special fields that are applicable only for search // results that are obtained from a presearch SynonymResult search.FieldTermSynonymMap `json:"synonym_result,omitempty"` @@ -675,6 +685,10 @@ func (sr *SearchResult) Merge(other *SearchResult) { sr.Status.Merge(other.Status) sr.Hits = append(sr.Hits, other.Hits...) sr.Total += other.Total + if other.TotalRelation == TotalRelationGte { + // Any constituent whose Total is a lower bound makes the merged Total one. + sr.TotalRelation = TotalRelationGte + } sr.Cost += other.Cost if other.MaxScore > sr.MaxScore { sr.MaxScore = other.MaxScore diff --git a/search/collector/topn.go b/search/collector/topn.go index d7fd27f23..82e9e5162 100644 --- a/search/collector/topn.go +++ b/search/collector/topn.go @@ -84,6 +84,15 @@ type TopNCollector struct { nestedStore *collectStoreNested fastPrepare bool + + // earlyStopN, when > 0, bounds the scan: once this many root hits have been + // collected, Collect() stops pulling from the searcher. Valid only when result + // identity does not depend on unseen docs (score="none", no facets, no field + // sort, no KNN, no nested, no SearchAfter). Set via SetEarlyStop before Collect. + earlyStopN int + // earlyStopped is set when Collect() actually broke out via earlyStopN, so the + // caller can report TotalRelation="gte" (Total is then a lower bound). + earlyStopped bool } // CheckDoneEvery controls how frequently we check the context deadline @@ -396,6 +405,15 @@ func (hc *TopNCollector) Collect(ctx context.Context, searcher search.Searcher, if err != nil { break } + // Early-stop (bounded scan): once earlyStopN root hits are collected, + // stop pulling. Valid because the store keeps the earliest size+skip + // hits (insertion order) and, with all scores equal, no later doc can + // displace them — so unseen docs cannot change the result. Breaking the + // pull loop is the entire early exit; the searcher stack is lazy. + if hc.earlyStopN > 0 && hc.total >= uint64(hc.earlyStopN) { + hc.earlyStopped = true + break + } } next, err = searcher.Next(searchContext) } @@ -692,6 +710,22 @@ func (hc *TopNCollector) Total() uint64 { return hc.total } +// SetEarlyStop bounds the scan: Collect() stops pulling from the searcher once n +// root hits have been collected (n is typically Size+From). Must be called before +// Collect(), and only when the result is order-independent of unseen docs — +// score="none", no facets, no field sort, no KNN, no nested, no SearchAfter. +// n <= 0 disables (the default), preserving the full-scan behavior. +func (hc *TopNCollector) SetEarlyStop(n int) { + hc.earlyStopN = n +} + +// EarlyStopped reports whether Collect() stopped before draining the searcher +// because the early-stop bound was reached. When true, Total() is a lower bound +// (the caller should report TotalRelation="gte"). +func (hc *TopNCollector) EarlyStopped() bool { + return hc.earlyStopped +} + // MaxScore returns the maximum score seen across all the hits func (hc *TopNCollector) MaxScore() float64 { return hc.maxScore From a5a670c8dfa3a3f342f16c52919204c6038f1d23 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Date: Tue, 28 Jul 2026 19:57:25 +0530 Subject: [PATCH 7/8] address review: drop unused collectStoreList, simplify Reset zeroing, trim comments - remove collectStoreList + its test (type is unused in the collector package) - Reset(): zero fields directly without nil guards - replace verbose early-stop comments with concise notes at the call site --- index_impl.go | 12 +- search.go | 1 - search/collector/list.go | 96 --------------- search/collector/list_test.go | 218 ---------------------------------- search/collector/topn.go | 28 ++--- search/search.go | 33 ++--- 6 files changed, 18 insertions(+), 370 deletions(-) delete mode 100644 search/collector/list.go delete mode 100644 search/collector/list_test.go diff --git a/index_impl.go b/index_impl.go index bad6d2fb2..c781f1ce1 100644 --- a/index_impl.go +++ b/index_impl.go @@ -786,13 +786,9 @@ func (i *indexImpl) SearchInContext(ctx context.Context, req *SearchRequest) (sr return nil, err } - // Early-stop (bounded scan): for score="none" + Size, the request means - // "return any Size+From matching docs", so the collector can stop pulling - // from the searcher once that many hits are in hand instead of draining the - // full result set. Valid only when result identity does not depend on unseen - // docs: no facets (need every match counted), no KNN (separate hit set), no - // pagination cursor, no nested rollup, and sort-by-score only (degrades to - // insertion order under score="none"; a field sort would need all docs). + // score="none" + Size means "return any Size+From matching docs", so the + // collector may stop scanning early — provided nothing below depends on + // unseen matches (facets, KNN, pagination cursor, nested rollup, field sort). if req.Score == ScoreNone && req.Size > 0 && len(req.Facets) == 0 && !requestHasKNN(req) && @@ -1055,8 +1051,6 @@ func (i *indexImpl) SearchInContext(ctx context.Context, req *SearchRequest) (sr totalRelation := TotalRelationEq if coll.EarlyStopped() { - // Total is a lower bound: the early-stop bounded scan stopped before - // draining all matches. totalRelation = TotalRelationGte } rv := &SearchResult{ diff --git a/search.go b/search.go index 52535a15c..a754a5a56 100644 --- a/search.go +++ b/search.go @@ -686,7 +686,6 @@ func (sr *SearchResult) Merge(other *SearchResult) { sr.Hits = append(sr.Hits, other.Hits...) sr.Total += other.Total if other.TotalRelation == TotalRelationGte { - // Any constituent whose Total is a lower bound makes the merged Total one. sr.TotalRelation = TotalRelationGte } sr.Cost += other.Cost diff --git a/search/collector/list.go b/search/collector/list.go deleted file mode 100644 index f73505e7d..000000000 --- a/search/collector/list.go +++ /dev/null @@ -1,96 +0,0 @@ -// Copyright (c) 2014 Couchbase, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package collector - -import ( - "container/list" - - "github.com/blevesearch/bleve/v2/search" -) - -type collectStoreList struct { - results *list.List - compare collectorCompare -} - -func newStoreList(capacity int, compare collectorCompare) *collectStoreList { - rv := &collectStoreList{ - results: list.New(), - compare: compare, - } - - return rv -} - -func (c *collectStoreList) AddNotExceedingSize(doc *search.DocumentMatch, size int) *search.DocumentMatch { - c.add(doc) - if c.len() > size { - return c.removeLast() - } - return nil -} - -func (c *collectStoreList) add(doc *search.DocumentMatch) { - for e := c.results.Front(); e != nil; e = e.Next() { - curr := e.Value.(*search.DocumentMatch) - if c.compare(doc, curr) >= 0 { - c.results.InsertBefore(doc, e) - return - } - } - // if we got to the end, we still have to add it - c.results.PushBack(doc) -} - -func (c *collectStoreList) removeLast() *search.DocumentMatch { - return c.results.Remove(c.results.Front()).(*search.DocumentMatch) -} - -func (c *collectStoreList) Final(skip int, fixup collectorFixup) (search.DocumentMatchCollection, error) { - if c.results.Len()-skip > 0 { - rv := make(search.DocumentMatchCollection, c.results.Len()-skip) - i := 0 - skipped := 0 - for e := c.results.Back(); e != nil; e = e.Prev() { - if skipped < skip { - skipped++ - continue - } - - rv[i] = e.Value.(*search.DocumentMatch) - err := fixup(rv[i]) - if err != nil { - return nil, err - } - i++ - } - return rv, nil - } - return search.DocumentMatchCollection{}, nil -} - -func (c *collectStoreList) Internal() search.DocumentMatchCollection { - rv := make(search.DocumentMatchCollection, c.results.Len()) - i := 0 - for e := c.results.Front(); e != nil; e = e.Next() { - rv[i] = e.Value.(*search.DocumentMatch) - i++ - } - return rv -} - -func (c *collectStoreList) len() int { - return c.results.Len() -} diff --git a/search/collector/list_test.go b/search/collector/list_test.go deleted file mode 100644 index e8a2ab240..000000000 --- a/search/collector/list_test.go +++ /dev/null @@ -1,218 +0,0 @@ -// Copyright (c) 2026 Couchbase, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package collector - -import ( - "errors" - "testing" - - "github.com/blevesearch/bleve/v2/search" -) - -var errTestFixup = errors.New("fixup error") - -// noFixup is a no-op fixup function used throughout the list tests. -var noFixup collectorFixup = func(*search.DocumentMatch) error { return nil } - -// TestCollectStoreListRoundTrip verifies that add() keeps elements in -// ascending-score order (Front=worst, Back=best) and that Final(0, ...) returns -// them best-first. This exercises the core insertion-sort invariant of the -// linked-list store, which had 0% coverage before this test. -func TestCollectStoreListRoundTrip(t *testing.T) { - l := newStoreList(20, scoreDesc) - for _, s := range []float64{3, 1, 4, 1, 5, 9, 2, 6} { - l.add(makeScoreDoc(s)) - } - if l.len() != 8 { - t.Fatalf("len=%d want 8", l.len()) - } - result, err := l.Final(0, noFixup) - if err != nil { - t.Fatal(err) - } - if len(result) != 8 { - t.Fatalf("Final len=%d want 8", len(result)) - } - for i := 1; i < len(result); i++ { - if result[i].Score > result[i-1].Score { - t.Errorf("Final not descending: result[%d]=%.2f > result[%d]=%.2f", - i, result[i].Score, i-1, result[i-1].Score) - } - } -} - -// TestCollectStoreListAddNotExceedingSize verifies that AddNotExceedingSize caps -// the list at k elements by evicting the worst (lowest-score) element. -func TestCollectStoreListAddNotExceedingSize(t *testing.T) { - const k = 3 - l := newStoreList(k, scoreDesc) - var evictedScores []float64 - for _, s := range []float64{1, 5, 3, 7, 2} { - ev := l.AddNotExceedingSize(makeScoreDoc(s), k) - if ev != nil { - evictedScores = append(evictedScores, ev.Score) - } - } - if l.len() != k { - t.Fatalf("list len=%d want %d after capping at k", l.len(), k) - } - // Inserted {1,5,3,7,2} with k=3 → evicted the 2 worst: 1 and 2. - if len(evictedScores) != 2 { - t.Fatalf("evicted %d docs want 2", len(evictedScores)) - } - // Remaining best-3: {7, 5, 3} in descending order. - result, err := l.Final(0, noFixup) - if err != nil { - t.Fatal(err) - } - want := []float64{7, 5, 3} - for i, w := range want { - if result[i].Score != w { - t.Errorf("result[%d]=%.2f want %.2f", i, result[i].Score, w) - } - } -} - -// TestCollectStoreListSkip verifies that Final(skip, ...) skips the top-skip -// best results and returns the remaining docs in descending order. -// This models pagination: skip=page*pageSize to start at a later page. -func TestCollectStoreListSkip(t *testing.T) { - l := newStoreList(20, scoreDesc) - for _, s := range []float64{1, 2, 3, 4, 5} { - l.add(makeScoreDoc(s)) - } - // skip=2 omits the 2 best (scores 5 and 4) → returns [3, 2, 1]. - result, err := l.Final(2, noFixup) - if err != nil { - t.Fatal(err) - } - if len(result) != 3 { - t.Fatalf("Final(skip=2) len=%d want 3", len(result)) - } - want := []float64{3, 2, 1} - for i, w := range want { - if result[i].Score != w { - t.Errorf("result[%d]=%.2f want %.2f", i, result[i].Score, w) - } - } -} - -// TestCollectStoreListSkipAll verifies Final returns empty when skip ≥ len. -func TestCollectStoreListSkipAll(t *testing.T) { - l := newStoreList(10, scoreDesc) - for _, s := range []float64{1, 2, 3} { - l.add(makeScoreDoc(s)) - } - result, err := l.Final(10, noFixup) // skip > len - if err != nil { - t.Fatal(err) - } - if len(result) != 0 { - t.Errorf("Final(skip=10) on 3-elem list returned %d docs, want 0", len(result)) - } -} - -// TestCollectStoreListInternal verifies Internal() returns all elements in -// ascending-score order (Front to Back of the linked list). -func TestCollectStoreListInternal(t *testing.T) { - l := newStoreList(10, scoreDesc) - for _, s := range []float64{3, 1, 4} { - l.add(makeScoreDoc(s)) - } - iv := l.Internal() - if len(iv) != 3 { - t.Fatalf("Internal len=%d want 3", len(iv)) - } - // Linked list: Front=worst→Back=best, so Internal() iterates Front→Back = ascending. - want := []float64{1, 3, 4} - for i, w := range want { - if iv[i].Score != w { - t.Errorf("Internal[%d]=%.2f want %.2f (ascending from worst)", i, iv[i].Score, w) - } - } -} - -// TestCollectStoreListRemoveLast verifies removeLast removes the Front element, -// which is the worst (lowest-score) document in the list. -func TestCollectStoreListRemoveLast(t *testing.T) { - l := newStoreList(10, scoreDesc) - for _, s := range []float64{3, 1, 5} { - l.add(makeScoreDoc(s)) - } - evicted := l.removeLast() - if evicted.Score != 1 { - t.Errorf("removeLast returned score=%.2f, want 1.0 (the worst)", evicted.Score) - } - if l.len() != 2 { - t.Errorf("len=%d after removeLast, want 2", l.len()) - } -} - -// TestCollectStoreListSingleElement verifies that a list with one element -// round-trips correctly through add / Final / Internal. -func TestCollectStoreListSingleElement(t *testing.T) { - l := newStoreList(5, scoreDesc) - l.add(makeScoreDoc(7.5)) - - result, err := l.Final(0, noFixup) - if err != nil { - t.Fatal(err) - } - if len(result) != 1 || result[0].Score != 7.5 { - t.Errorf("single-element Final: got %v", result) - } - - iv := l.Internal() - if len(iv) != 1 || iv[0].Score != 7.5 { - t.Errorf("single-element Internal: got %v", iv) - } -} - -// TestCollectStoreListEqualScores verifies correct handling of equal-scored -// documents: they should all be retained, and Final preserves their relative -// insertion order within equal-scored groups. -func TestCollectStoreListEqualScores(t *testing.T) { - l := newStoreList(10, scoreDesc) - for range 5 { - l.add(makeScoreDoc(3.0)) - } - if l.len() != 5 { - t.Fatalf("len=%d after 5 equal-scored adds, want 5", l.len()) - } - result, err := l.Final(0, noFixup) - if err != nil { - t.Fatal(err) - } - for _, dm := range result { - if dm.Score != 3.0 { - t.Errorf("expected all scores=3.0, got %.2f", dm.Score) - } - } -} - -// TestCollectStoreListFixupError verifies that an error returned by the fixup -// function propagates correctly from Final. -func TestCollectStoreListFixupError(t *testing.T) { - l := newStoreList(10, scoreDesc) - l.add(makeScoreDoc(1.0)) - - errFixup := func(*search.DocumentMatch) error { - return errTestFixup - } - _, err := l.Final(0, errFixup) - if err != errTestFixup { - t.Errorf("Final fixup error not propagated: got %v", err) - } -} diff --git a/search/collector/topn.go b/search/collector/topn.go index 82e9e5162..02497c571 100644 --- a/search/collector/topn.go +++ b/search/collector/topn.go @@ -85,14 +85,8 @@ type TopNCollector struct { fastPrepare bool - // earlyStopN, when > 0, bounds the scan: once this many root hits have been - // collected, Collect() stops pulling from the searcher. Valid only when result - // identity does not depend on unseen docs (score="none", no facets, no field - // sort, no KNN, no nested, no SearchAfter). Set via SetEarlyStop before Collect. - earlyStopN int - // earlyStopped is set when Collect() actually broke out via earlyStopN, so the - // caller can report TotalRelation="gte" (Total is then a lower bound). - earlyStopped bool + earlyStopN int // when > 0, Collect() stops after this many hits (see SetEarlyStop) + earlyStopped bool // set when Collect() stopped early; Total is then a lower bound } // CheckDoneEvery controls how frequently we check the context deadline @@ -405,11 +399,6 @@ func (hc *TopNCollector) Collect(ctx context.Context, searcher search.Searcher, if err != nil { break } - // Early-stop (bounded scan): once earlyStopN root hits are collected, - // stop pulling. Valid because the store keeps the earliest size+skip - // hits (insertion order) and, with all scores equal, no later doc can - // displace them — so unseen docs cannot change the result. Breaking the - // pull loop is the entire early exit; the searcher stack is lazy. if hc.earlyStopN > 0 && hc.total >= uint64(hc.earlyStopN) { hc.earlyStopped = true break @@ -710,18 +699,15 @@ func (hc *TopNCollector) Total() uint64 { return hc.total } -// SetEarlyStop bounds the scan: Collect() stops pulling from the searcher once n -// root hits have been collected (n is typically Size+From). Must be called before -// Collect(), and only when the result is order-independent of unseen docs — -// score="none", no facets, no field sort, no KNN, no nested, no SearchAfter. -// n <= 0 disables (the default), preserving the full-scan behavior. +// SetEarlyStop makes Collect() stop pulling from the searcher once n hits have +// been collected; n <= 0 disables. Callers must ensure unseen docs cannot +// change the result. func (hc *TopNCollector) SetEarlyStop(n int) { hc.earlyStopN = n } -// EarlyStopped reports whether Collect() stopped before draining the searcher -// because the early-stop bound was reached. When true, Total() is a lower bound -// (the caller should report TotalRelation="gte"). +// EarlyStopped reports whether Collect() stopped early; if true, Total() is a +// lower bound. func (hc *TopNCollector) EarlyStopped() bool { return hc.earlyStopped } diff --git a/search/search.go b/search/search.go index a39adb733..8088c11e3 100644 --- a/search/search.go +++ b/search/search.go @@ -240,33 +240,16 @@ func (dm *DocumentMatch) Reset() *DocumentMatch { for i := range descendants { // recycle each IndexInternalID descendants[i] = descendants[i][:0] } - // Zero only the fields that are NOT restored below. Nil/empty guards on - // pointer/string/map fields avoid triggering a GC write barrier for nil→nil - // stores, saving ~5 cycles per field across the many millions of Reset calls - // (common case: no explain, no fragments, no field highlights). - if dm.Index != "" { - dm.Index = "" - } - if dm.ID != "" { - dm.ID = "" - } + // Zero only the fields that are not restored below. + dm.Index = "" + dm.ID = "" dm.Score = 0 - if dm.Expl != nil { - dm.Expl = nil - } - if dm.Locations != nil { - dm.Locations = nil - } - if dm.Fragments != nil { - dm.Fragments = nil - } - if dm.Fields != nil { - dm.Fields = nil - } + dm.Expl = nil + dm.Locations = nil + dm.Fragments = nil + dm.Fields = nil dm.HitNumber = 0 - if dm.IndexNames != nil { - dm.IndexNames = nil - } + dm.IndexNames = nil // Restore reusable allocations. dm.IndexInternalID = indexInternalID[:0] dm.Sort = sortBuf[:0] From 39d3cddc0feb488bf4107d40960bfa2d48e0124d Mon Sep 17 00:00:00 2001 From: Rohit Kumar Date: Tue, 28 Jul 2026 22:26:23 +0530 Subject: [PATCH 8/8] address review: drop earlyStop field comments (func comments suffice) --- search/collector/topn.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/search/collector/topn.go b/search/collector/topn.go index 02497c571..296984fe7 100644 --- a/search/collector/topn.go +++ b/search/collector/topn.go @@ -85,8 +85,8 @@ type TopNCollector struct { fastPrepare bool - earlyStopN int // when > 0, Collect() stops after this many hits (see SetEarlyStop) - earlyStopped bool // set when Collect() stopped early; Total is then a lower bound + earlyStopN int + earlyStopped bool } // CheckDoneEvery controls how frequently we check the context deadline