Skip to content

perf(vector): prepare embeddings once and add portable SIMD scoring - #218

Merged
steipete merged 1 commit into
mainfrom
perf/simd-cluster-cosine
Sep 28, 2026
Merged

steipete merged 1 commit into
mainfrom
perf/simd-cluster-cosine

Conversation

@steipete

Copy link
Copy Markdown
Contributor

Summary

Makes cluster and exact neighbors much faster, in two layers:

  1. Prepare once (all builds). vector.Cosine re-validated both vectors for NaN/Inf, rescaled them by max-abs, and accumulated dot plus both magnitudes on every call. The O(n²) cluster loop repeated that work for every pair. The new vector.Prepare does validation, scaling and the magnitude once per vector, so each pair costs a single dot product. Scores are bit-identical to Cosine (same scaled values, same summation order), and Cosine itself is unchanged. The pairwise loop is extracted as scoreClusterEdges without behaviour changes. queryExact prepares the query once and reuses one candidate buffer.
  2. Portable SIMD (GOEXPERIMENT=simd). Uses Go 1.27's experimental simd package for the prepared dot product and for preparation (NaN/Inf detection via masks, scaling, magnitude), with four independent float64 accumulators. Emulated or unsupported hardware and GODEBUG=simd=0 fall back to the scalar kernels.

⚠️ Decision for review: release builds opt into the experiment

.goreleaser.yaml (both build blocks) and the Dockerfile now build with GOEXPERIMENT=simd, because SIMD cleared the >1.3× bar over the new scalar default on user-visible operations. The trade-off is that release binaries depend on an experimental API that may change in Go 1.28. When the toolchain is bumped, a breaking change would fail the build loudly rather than silently. To support that, CI now runs the full test suite under GOEXPERIMENT=simd, since that's the configuration that ships, plus a forced-emulation pass. If you'd rather not ship an experiment, drop the two goreleaser lines and the Dockerfile flag; the scalar improvement stands on its own.

Benchmarks

Default model text-embedding-3-small at gitcrawl's 1,024 dimensions, Apple M3 Ultra, Go 1.27.1, -count=10. The shared host was heavily loaded, so intervals are wide:

Operation origin/main This PR (default) This PR (GOEXPERIMENT=simd)
Cluster edge scoring, 2,000 threads 7.48 s 2.83 s 1.07 s
End-to-end cluster, 2,000 threads 5.42 s 4.50 s 2.59 s
Exact neighbors query, 20,000 threads 140.6 ms 70.8 ms 42.3 ms
Prepared cosine kernel 9,030 ns 1,362 ns 605 ns

A maintainer re-run measured cluster edges at 2.25 s (default) against about 1.0 s (SIMD), and neighbors at 61 ms against 33 ms. Cluster preparation holds one extra float64 copy of the vectors (about 16 MiB at 2,000 × 1,024).

A float32 SIMD variant was also evaluated. It was another 2.2× faster, but max cosine error reached 1.5e-6 at 3,072 dims, which is over the 1e-6 budget, so float64 stays.

Correctness

  • Default build: max deviation from the original scoring is 0. SIMD: 4.1e-15 (tolerance 1e-12). Existing tests are unchanged, and new tests pin the extracted edge loop against the original.
  • go test ./... and GOEXPERIMENT=simd go test ./... pass. So do the forced-emulation run, go vet in both modes, make check (85.7% coverage), and amd64 SIMD tests under Rosetta (128-bit). GoReleaser snapshot builds pass for all six SIMD targets.
  • Lane buffers are sized for 2048-bit vectors, so a future SVE-enabled toolchain can't make Store panic in a release binary.

@steipete
steipete requested a review from a team as a code owner September 27, 2026 21:12
@clawsweeper

clawsweeper Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 27, 2026
@clawsweeper

clawsweeper Bot commented Sep 27, 2026

Copy link
Copy Markdown

Codex review: blocked before merge. Reviewed September 27, 2026, 5:15 PM ET / 21:15 UTC.

ClawSweeper review

What this changes

The branch prepares embeddings once for cluster and exact-neighbor scoring, adds portable SIMD kernels, and enables SIMD in release and Docker builds.

Merge readiness

⛔ Blocked before merge - 3 items remain

Keep open. Current main and v0.12.0 lack this optimization, and the measured patch appears sound. Shipping the experimental SIMD path in official builds still needs an explicit maintainer decision.

Priority: P2
Reviewed head: 88b685b84b8a73f1295c4129fa6c86295f36b0d5
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The focused optimization has measured production-path benchmarks and broad numerical coverage, with release inclusion left as an explicit human choice.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (terminal): The captured PR body reports before-and-after measurements on Apple M3 Ultra with Go 1.27.1 for the changed production cluster command and exact-neighbor scoring paths; the cluster benchmark invokes app.Run against a populated SQLite fixture and reports 5.42 to 2.59 seconds with SIMD. The fixture is synthetic, but it exercises the real command owner and shows the intended after-fix performance result. No stored-data format or schema contract changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The captured PR body reports before-and-after measurements on Apple M3 Ultra with Go 1.27.1 for the changed production cluster command and exact-neighbor scoring paths; the cluster benchmark invokes app.Run against a populated SQLite fixture and reports 5.42 to 2.59 seconds with SIMD. The fixture is synthetic, but it exercises the real command owner and shows the intended after-fix performance result. No stored-data format or schema contract changes.
Evidence reviewed 11 items Introduced patch: The pinned main-to-head diff adds prepared scalar and SIMD scoring and changes release build flags; the optimization is unique to this branch.
Current main behavior: The exact-query path on pinned current main calls Cosine for every candidate, and Cosine validates and normalizes both vectors on each call.
Cluster scoring boundary: The changed cluster loop prepares each stored vector once, then applies the existing threshold, title-overlap, and cross-kind filters to pair scores.
Findings None None.
Security None None.

How this fits together

Gitcrawl stores embeddings for synced GitHub threads. Clustering compares them to form durable groups, while exact-neighbor search ranks related threads.

flowchart LR
A[Stored thread embeddings] --> B[Validate and scale vectors]
B --> C[Cosine scoring]
C --> D{Use case}
D --> E[Threshold cluster edges]
E --> F[Durable clusters]
D --> G[Rank exact neighbors]
G --> H[Neighbor results]
Loading

Decision needed

Question Recommendation
Should official Gitcrawl archives and Docker images opt into GOEXPERIMENT=simd now, accepting maintenance when Go changes its experimental API? Ship SIMD in releases: Keep the release and Docker flags, gain the measured speedup, and explicitly own compatibility work during future Go upgrades.

Why: The measured speedup and current test coverage support the code path, but choosing an experimental dependency for every distributed binary is a release policy decision that the patch and bot-only discussion cannot settle.

Before merge

  • Resolve merge risk (P1) - Official archives and Docker images would depend on Go's experimental SIMD API. A later toolchain upgrade could stop release builds until the SIMD code is updated, and the discussion has not yet recorded acceptance of that maintenance commitment.
  • Complete next step (P2) - Confirm whether experimental SIMD belongs in official release and Docker builds; select scalar release builds if the maintenance commitment is not accepted.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Code delta production +193 net lines, tests +381 lines The production growth implements the stated performance path and is accompanied by scoring and benchmark coverage.
Reported command benchmarks cluster 5.42s → 2.59s; exact neighbors 140.6ms → 42.3ms These are the reported end-to-end gains on the stated Apple M3 Ultra and Go 1.27.1 setup.

Merge-risk options

Maintainer options:

  1. Accept the release build dependency (recommended)
    Record maintainer approval for experimental SIMD in official builds and own required changes when the Go toolchain is upgraded.
  2. Keep releases scalar
    Remove GOEXPERIMENT=simd from GoReleaser and Docker while keeping the measured scalar optimization and optional SIMD source builds.

Technical review

Best possible solution:

Retain prepared scalar scoring in every build and ship SIMD in official artifacts only after the release owner explicitly accepts the experimental API maintenance commitment.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a bug reproduction. The captured PR body gives controlled 2,000-thread cluster and 20,000-thread neighbor benchmark scenarios with before-and-after measurements.

Is this the best way to solve the issue?

Yes for avoiding repeated vector preparation in the two scoring paths; the release-wide experimental build choice needs explicit acceptance.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 0a4cefe1858b.

Labels

Label changes:

  • add P2: This is a useful, bounded performance improvement to existing cluster and neighbor workflows.
  • add merge-risk: 🚨 automation: The diff makes official release and Docker builds depend on a Go experimental API that may require repair on a future toolchain upgrade.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The captured PR body reports before-and-after measurements on Apple M3 Ultra with Go 1.27.1 for the changed production cluster command and exact-neighbor scoring paths; the cluster benchmark invokes app.Run against a populated SQLite fixture and reports 5.42 to 2.59 seconds with SIMD. The fixture is synthetic, but it exercises the real command owner and shows the intended after-fix performance result. No stored-data format or schema contract changes.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The captured PR body reports before-and-after measurements on Apple M3 Ultra with Go 1.27.1 for the changed production cluster command and exact-neighbor scoring paths; the cluster benchmark invokes app.Run against a populated SQLite fixture and reports 5.42 to 2.59 seconds with SIMD. The fixture is synthetic, but it exercises the real command owner and shows the intended after-fix performance result. No stored-data format or schema contract changes.

Label justifications:

  • P2: This is a useful, bounded performance improvement to existing cluster and neighbor workflows.
  • merge-risk: 🚨 automation: The diff makes official release and Docker builds depend on a Go experimental API that may require repair on a future toolchain upgrade.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The captured PR body reports before-and-after measurements on Apple M3 Ultra with Go 1.27.1 for the changed production cluster command and exact-neighbor scoring paths; the cluster benchmark invokes app.Run against a populated SQLite fixture and reports 5.42 to 2.59 seconds with SIMD. The fixture is synthetic, but it exercises the real command owner and shows the intended after-fix performance result. No stored-data format or schema contract changes.
  • proof: sufficient: Contributor real behavior proof is sufficient. The captured PR body reports before-and-after measurements on Apple M3 Ultra with Go 1.27.1 for the changed production cluster command and exact-neighbor scoring paths; the cluster benchmark invokes app.Run against a populated SQLite fixture and reports 5.42 to 2.59 seconds with SIMD. The fixture is synthetic, but it exercises the real command owner and shows the intended after-fix performance result. No stored-data format or schema contract changes.

Evidence

What I checked:

  • Introduced patch: The pinned main-to-head diff adds prepared scalar and SIMD scoring and changes release build flags; the optimization is unique to this branch. (internal/vector/prepared.go:14, 88b685b84b8a)
  • Current main behavior: The exact-query path on pinned current main calls Cosine for every candidate, and Cosine validates and normalizes both vectors on each call. (internal/vector/exact.go:52, 0a4cefe1858b)
  • Cluster scoring boundary: The changed cluster loop prepares each stored vector once, then applies the existing threshold, title-overlap, and cross-kind filters to pair scores. (internal/cli/cluster_graph.go:142, 88b685b84b8a)
  • Numerical coverage: New tests compare prepared scores with the original scorer across dimensions and special values, including NaN, infinity, zero, and subnormal vectors. (internal/vector/prepared_test.go:14, 88b685b84b8a)
  • Production-path benchmark: The benchmark populates a SQLite fixture and invokes the production cluster command through app.Run; the captured PR body reports after-fix timings on Apple M3 Ultra with Go 1.27.1, including 5.42 to 2.59 seconds for the end-to-end cluster case. (internal/cli/cluster_graph_bench_test.go:61, 88b685b84b8a)
  • Release-mode choice: Both archive build blocks set GOEXPERIMENT=simd. The captured PR body expressly asks reviewers to decide whether official releases should depend on the experimental API. (.goreleaser.yaml:14, 88b685b84b8a)

Likely related people:

  • Peter Steinberger: Raw commit 777a404 adds internal/cli/cluster_graph.go:53 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 777a404d28a7; files: internal/cli/cluster_graph.go)
  • Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit 290d943 into main Sep 28, 2026
14 checks passed
@steipete
steipete deleted the perf/simd-cluster-cosine branch September 28, 2026 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant