Skip to content

refactor: keep only the best k candidates instead of sorting every one - #3

Merged
shaikn6 merged 1 commit into
mainfrom
perf/hot-path
Oct 19, 2026
Merged

shaikn6 merged 1 commit into
mainfrom
perf/hot-path

Conversation

@shaikn6

@shaikn6 shaikn6 commented Oct 19, 2026

Copy link
Copy Markdown
Owner

Change

Recall picked its candidates by sorting everything:

  • memory.candidates computed the cosine of every live vector in the namespace, put all of them in a slice and sort.Sliced it, to keep the top Candidates (default 50).
  • index.Search sorted every keyword hit to return the top n.

Both now stream into a bounded heap (internal/topk, about 80 lines, generic) that keeps only the best k: O(N log k) instead of O(N log N), and the semantic path no longer allocates an N-sized slice. The ordering is unchanged (score descending, ties by document number), so the selected set and order are identical.

Also adds BenchmarkRecall (1k / 10k / 50k memories, 64-dim embeddings, text + vector query) and a CI step that runs it once (-benchtime=1x) so it keeps compiling.

Measured (Apple M2 Pro, go1.26.9, -count=5, medians via benchstat)

Command (same benchmark file on both sides; "before" is origin/main with only bench_test.go added):

go test ./internal/memory -run '^$' -bench BenchmarkRecall -benchmem -count=5
memories before after change
1,000 447.1 us/op 313.9 us/op -29.8%
10,000 3.615 ms/op 1.440 ms/op -60.2%
50,000 19.386 ms/op 6.504 ms/op -66.5%

Memory per recall: 3.871 MiB -> 2.341 MiB at 50k (-39.5%); 173.2 KiB -> 142.8 KiB at 1k. All p=0.008, n=5.

Raw output:

before 50000: 19291803 19370729 19873034 19385916 19548000 ns/op, 4058665 B/op, 325 allocs/op
after  50000:  6416557  6446887  6503783  6822199  6535898 ns/op, 2454609 B/op, 319 allocs/op

Tests

  • internal/topk: heap result equals a full sort for n in {0,1,5,50,1000}, k in {0,1,5,50,2000}, with many ties.

  • internal/index: TestSearchLimitIsPrefixOfFullRanking, Search(n) equals the first n of the full ranking, on equal-score data.

  • Existing recall and ranking tests unchanged and passing.

    go vet ./... && go test -race -count=1 ./...
    ok github.com/shaikn6/lethe/cmd/lethe-mcp
    ok github.com/shaikn6/lethe/internal/events
    ok github.com/shaikn6/lethe/internal/index
    ok github.com/shaikn6/lethe/internal/memory
    ok github.com/shaikn6/lethe/internal/policy
    ok github.com/shaikn6/lethe/internal/server
    ok github.com/shaikn6/lethe/internal/topk

Scope and caveats

  • This is a latency/allocation improvement of the recall hot path in-process. I did not re-run the HTTP load test or LoCoMo, so the README's req/s and accuracy figures are untouched and unchanged by this PR.
  • Gains grow with namespace size; small namespaces see a smaller win (about 30% at 1k).
  • Not touched: the linear cosine scan itself is still O(N); an ANN index would be a separate change.

Rollback: revert the single commit.

Recall sorted every vector in the namespace and every keyword hit to take
the top Candidates. A bounded heap keeps the same set and order (ties are
still broken by document number) in O(n log k). Adds BenchmarkRecall and a
CI step that keeps it compiling and running.
@shaikn6
shaikn6 merged commit 2b4d909 into main Oct 19, 2026
3 checks passed
@shaikn6
shaikn6 deleted the perf/hot-path branch October 19, 2026 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant