Skip to content

Commit 4510d4f

Browse files
committed
perf: lift seen-map, strings.Builder in buildExplanation, drop dead RepoRoot field
- recordCommit: allocate seen map once in buildCoChangePairs, clear with range-delete instead of make() per commit → −97% allocs at 1k commits - buildExplanation: strings.Builder + strconv replaces fmt.Sprintf → −40% latency, allocs halved (6→3 per site) - Remove ScanOptions.RepoRoot (dead field — Analyzer.repoRoot used throughout) - Update bench baselines and document results in docs/performance_log.md
1 parent 19c92f4 commit 4510d4f

9 files changed

Lines changed: 214 additions & 64 deletions

File tree

cmd/ckb/perf.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ func runPerfCoupling(cmd *cobra.Command, args []string) {
8080

8181
ctx := context.Background()
8282
result, err := analyzer.Scan(ctx, perf.ScanOptions{
83-
RepoRoot: repoRoot,
8483
Scope: perfCouplingScope,
8584
MinCorrelation: perfCouplingMinCorrelation,
8685
MinCoChanges: perfCouplingMinCoChanges,

docs/performance_log.md

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
# Performance Log
2+
3+
Benchmark results over time. Run with `benchstat` for before/after comparisons.
4+
5+
```bash
6+
go test -bench=. ./internal/compliance/... -benchmem -count=6 > before.txt
7+
# make changes
8+
go test -bench=. ./internal/compliance/... -benchmem -count=6 > after.txt
9+
benchstat before.txt after.txt
10+
```
11+
12+
---
13+
14+
## 2026-04-10 — `internal/perf` package: hidden-coupling scanner + structural perf analysis (Apple M4 Pro, arm64, -count=3)
15+
16+
Branch: `bench/compliance-scanner-baselines`
17+
18+
New package implementing two scan modes:
19+
- **Hidden coupling** (`Scan`): git log → co-change pair counts → correlation filter → import-edge check
20+
- **Structural perf** (`AnalyzeStructural`, CGO only): git churn → tree-sitter loop/call-site detection → severity ranking
21+
22+
### Optimization 1: lift seen-map out of `recordCommit`
23+
24+
`buildCoChangePairs` calls `recordCommit` once per commit. Originally each call allocated a fresh `make(map[string]bool)` for deduplication. Changed to allocate once before the loop and pass it in; `recordCommit` clears it with `for k := range seen { delete(seen, k) }`.
25+
26+
Effect on `CoChangePipelineSimulated` (the dominant CPU path):
27+
28+
| Scenario | allocs/op before | allocs/op after | Δ |
29+
|---|---|---|---|
30+
| 500 commits × 10 files | 1526 | 29 | **−98%** |
31+
| 1k commits × 20 files | 3072 | 75 | **−97.6%** |
32+
| 1k commits × 20 files (B/op) | 2,522,031 | 1,586,994 | **−37%** |
33+
| 1k commits × 20 files (ns/op) | ~5.4 ms | ~4.8 ms | −11% |
34+
35+
The bulk of the pre-optimization allocs were 1× `make(map[string]bool)` per commit — invisible in profiling but compounding across thousands of commits in real repos.
36+
37+
### Optimization 2: `buildExplanation``fmt.Sprintf``strings.Builder`
38+
39+
`buildExplanation` is called once per loop call site in the structural scan. Replaced chained `fmt.Sprintf` with a pre-grown `strings.Builder` (`b.Grow(320)`) + `strconv.Quote` / `strconv.Itoa`.
40+
41+
| Variant | ns/op before | ns/op after | allocs/op before | allocs/op after |
42+
|---|---|---|---|---|
43+
| non-entrypoint | 352 | 208 | 6 | **3** |
44+
| entrypoint | 350 | 188 | 7 | **3** |
45+
46+
~40–46% faster, allocs halved. The remaining 3 allocs are the `strings.Builder` internal buffer, `strconv.Quote`'s escape output, and the final `b.String()` copy — irreducible without a pre-allocated byte pool.
47+
48+
### Hot path baselines
49+
50+
#### `internal/perf` — co-change scanner
51+
52+
| Benchmark | ns/op | B/op | allocs/op |
53+
|---|---|---|---|
54+
| `recordCommit/2files` | 79 | 0 | 0 |
55+
| `recordCommit/10files` | 2,713 | 5,816 | 13 |
56+
| `recordCommit/50files` | 73,730 | 197,000 | 34 |
57+
| `recordCommit_Reuse/10files` | 1,027 | 0 | 0 |
58+
| `CoChangePipeline/100c_5f` | 38,257 | 55,888 | 12 |
59+
| `CoChangePipeline/500c_10f` | 628,257 | 406,802 | 29 |
60+
| `CoChangePipeline/1kc_20f` | 4,793,762 | 1,586,994 | 75 |
61+
| `importCouldReferTo/10imports` | 277 | 0 | 0 |
62+
| `importCouldReferTo_Miss` | 513 | 0 | 0 |
63+
| `shouldIgnore` | 1.8–7.1 | 0 | 0 |
64+
| `correlationLevel` | 0.26 | 0 | 0 |
65+
| `correlationFilter/~20k pairs` | ~372,000 | 0 | 0 |
66+
67+
#### `internal/perf` — structural (CGO)
68+
69+
| Benchmark | ns/op | B/op | allocs/op |
70+
|---|---|---|---|
71+
| `computeSeverity` | 0.26 | 0 | 0 |
72+
| `buildExplanation/non_ep` | 208 | 432 | 3 |
73+
| `buildExplanation/entrypoint` | 188 | 416 | 3 |
74+
| `findEnclosingFunction/10fns` | 3.0 | 0 | 0 |
75+
| `findEnclosingFunction/50fns` | 14 | 0 | 0 |
76+
| `CallSitePipeline/100sites` | 33,000 | 34,266 | 620 |
77+
| `CallSitePipeline/500sites` | 160,000 | 171,334 | 3,100 |
78+
79+
### Known bottlenecks
80+
81+
- `recordCommit` is O(files²) per commit — formatting sweeps and mass renames produce commits with 100+ files, which hit ~75 µs/commit and ~197 KB/call. No fix yet; caller could skip commits above a file-count threshold.
82+
- `correlationFilter` iterates all ~N²/2 pairs in memory — fine up to ~200 files but will need chunking or a threshold-based early prune for monorepos with thousands of hot files.
83+
- `buildExplanation`'s 3 remaining allocs are irreducible without a `sync.Pool` on the `strings.Builder` buffer. Not worth it at current call volumes.
84+
85+
---
86+
87+
## 2026-04-09 — Compliance scanner baseline (Apple M4 Pro, arm64, -count=3)
88+
89+
Branch: `bench/compliance-scanner-baselines`
90+
91+
### Hot path functions
92+
93+
| Benchmark | ns/op | B/op | allocs/op |
94+
|---|---|---|---|
95+
| `NormalizeIdentifier` | 137 | 138 | 4 |
96+
| `NormalizeIdentifier_Long` | 673 | 1352 | 9 |
97+
| `ExtractIdentifiers` | 555 | 219 | 6 |
98+
| `ExtractContainer` | 517 | 24 | 0 |
99+
| `IsNonPIIIdentifier` | 205 | 0 | 0 |
100+
| `matchPII (mixed hit/miss)` | 739 | 0 | 0 |
101+
| `matchPII (miss, full scan)` | 1267 | 0 | 0 |
102+
| `NewPIIScanner` | 2421 | 13048 | 6 |
103+
| `NewPIIScannerWithExtras` | 5853 | 20640 | 32 |
104+
105+
### Scanner pipeline (per-file, single file)
106+
107+
Scales linearly: ~14 allocs/line, ~3.7 µs/line.
108+
109+
| Lines | ns/op | MB/s | B/op | allocs/op |
110+
|---|---|---|---|---|
111+
| 500 | 1,854,122 | 8.23 | 209,857 | 6,989 |
112+
| 5k | 18,692,730 | 8.19 | 2,098,509 | 69,845 |
113+
| 50k | 185,942,685 | 8.23 | 20,857,361 | 698,378 |
114+
115+
### Audit file set (full repo scan simulation)
116+
117+
| Files (×300 lines) | Total lines | ns/op | MB/s | B/op | allocs/op |
118+
|---|---|---|---|---|---|
119+
| 100 | ~30k | 110,690,443 | 8.29 | 12,602,896 | 419,038 |
120+
| 1k | ~300k | 1,114,492,597 | 8.24 | 126,979,875 | 4,190,378 |
121+
| 5k | ~1.5M | 6,325,314,514 | 7.33 | 629,844,261 | 20,951,883 |
122+
123+
**Notable:** 5k-file run shows 24% variance across 3 runs (5.85s–7.25s) and MB/s drops from ~8.3 to ~6.3–7.8. GC pressure from 630 MB heap allocation. Root cause: `extractIdentifiers` allocates a map + slice per line — ~4.2M allocs for a 1.5M-line scan.
124+
125+
### Pattern scale (`matchPII`, miss path)
126+
127+
| Patterns | ns/op |
128+
|---|---|
129+
| ~80 (default) | 1,174 |
130+
| 100 | 1,386 |
131+
| 200 | 2,355 |
132+
| 500 | 5,238 |
133+
134+
Linear degradation with custom pattern count. 80→500 patterns = ~4.5× slower on the miss path. Relevant for users with large custom PII configs on big repos.
135+
136+
### Known bottlenecks
137+
138+
- `extractIdentifiers`: allocates `map[string]bool` + `[]string` per source line. At 5k files × 300 lines, this is ~4.2M allocs per audit run. Pooling the map would be the highest-leverage fix.
139+
- GC pressure at repo scale: 630 MB allocated for a 1.5M-line scan. MB/s degrades ~10% at this scale due to GC pauses.
140+
- Custom PII patterns scale linearly on the miss path — no trie or bloom filter in front of the suffix scan.

internal/mcp/tool_impls_perf.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ func (s *MCPServer) toolScanPerformance(params map[string]interface{}) (*envelop
4747

4848
ctx := context.Background()
4949
result, err := analyzer.Scan(ctx, perf.ScanOptions{
50-
RepoRoot: repoRoot,
5150
Scope: scope,
5251
MinCorrelation: minCorrelation,
5352
MinCoChanges: minCoChanges,

internal/perf/analyzer.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -251,13 +251,16 @@ func (a *Analyzer) buildCoChangePairs(ctx context.Context, since time.Time, scop
251251
pairCounts := make(map[filePair]int)
252252
fileTotals := make(map[string]int)
253253

254-
// Parse output: groups of files per commit, separated by blank lines.
254+
// Reusable buffers — allocated once, cleared between commits.
255+
seen := make(map[string]bool, 32)
255256
var currentFiles []string
257+
258+
// Parse output: groups of files per commit, separated by blank lines.
256259
for _, line := range strings.Split(string(out), "\n") {
257260
line = strings.TrimSpace(line)
258261
if strings.HasPrefix(line, "COMMIT ") {
259262
// Flush previous group.
260-
a.recordCommit(currentFiles, pairCounts, fileTotals)
263+
a.recordCommit(currentFiles, pairCounts, fileTotals, seen)
261264
currentFiles = currentFiles[:0]
262265
continue
263266
}
@@ -266,17 +269,21 @@ func (a *Analyzer) buildCoChangePairs(ctx context.Context, since time.Time, scop
266269
}
267270
currentFiles = append(currentFiles, line)
268271
}
269-
a.recordCommit(currentFiles, pairCounts, fileTotals)
272+
a.recordCommit(currentFiles, pairCounts, fileTotals, seen)
270273

271274
return pairCounts, fileTotals, nil
272275
}
273276

274-
func (a *Analyzer) recordCommit(files []string, pairCounts map[filePair]int, fileTotals map[string]int) {
277+
func (a *Analyzer) recordCommit(files []string, pairCounts map[filePair]int, fileTotals map[string]int, seen map[string]bool) {
275278
if len(files) == 0 {
276279
return
277280
}
278-
// Only count each file once per commit; skip ignored paths.
279-
seen := make(map[string]bool, len(files))
281+
// Clear the reusable seen map from the previous commit.
282+
for k := range seen {
283+
delete(seen, k)
284+
}
285+
// Deduplicate and filter in a single pass, writing unique entries
286+
// back into the files slice (safe: we only write index j where j <= i).
280287
unique := files[:0]
281288
for _, f := range files {
282289
if shouldIgnore(f) {

internal/perf/analyzer_test.go

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ func TestRecordCommit_Empty(t *testing.T) {
134134
a := &Analyzer{}
135135
pairs := make(map[filePair]int)
136136
totals := make(map[string]int)
137-
a.recordCommit(nil, pairs, totals)
137+
a.recordCommit(nil, pairs, totals, make(map[string]bool))
138138
if len(pairs) != 0 || len(totals) != 0 {
139139
t.Error("empty files should produce no pairs or totals")
140140
}
@@ -144,7 +144,7 @@ func TestRecordCommit_SingleFile(t *testing.T) {
144144
a := &Analyzer{}
145145
pairs := make(map[filePair]int)
146146
totals := make(map[string]int)
147-
a.recordCommit([]string{"a.go"}, pairs, totals)
147+
a.recordCommit([]string{"a.go"}, pairs, totals, make(map[string]bool))
148148
if len(pairs) != 0 {
149149
t.Errorf("single file should produce no pairs, got %d", len(pairs))
150150
}
@@ -157,7 +157,7 @@ func TestRecordCommit_TwoFiles(t *testing.T) {
157157
a := &Analyzer{}
158158
pairs := make(map[filePair]int)
159159
totals := make(map[string]int)
160-
a.recordCommit([]string{"a.go", "b.go"}, pairs, totals)
160+
a.recordCommit([]string{"a.go", "b.go"}, pairs, totals, make(map[string]bool))
161161
if len(pairs) != 1 {
162162
t.Fatalf("expected 1 pair, got %d", len(pairs))
163163
}
@@ -173,7 +173,7 @@ func TestRecordCommit_Deduplication(t *testing.T) {
173173
a := &Analyzer{}
174174
pairs := make(map[filePair]int)
175175
totals := make(map[string]int)
176-
a.recordCommit([]string{"a.go", "a.go", "b.go"}, pairs, totals)
176+
a.recordCommit([]string{"a.go", "a.go", "b.go"}, pairs, totals, make(map[string]bool))
177177
if totals["a.go"] != 1 {
178178
t.Errorf("a.go total = %d, want 1 (dedup within commit)", totals["a.go"])
179179
}
@@ -187,7 +187,7 @@ func TestRecordCommit_OrderedKey(t *testing.T) {
187187
a := &Analyzer{}
188188
pairs := make(map[filePair]int)
189189
totals := make(map[string]int)
190-
a.recordCommit([]string{"z.go", "a.go"}, pairs, totals)
190+
a.recordCommit([]string{"z.go", "a.go"}, pairs, totals, make(map[string]bool))
191191
key := filePair{"a.go", "z.go"}
192192
if pairs[key] != 1 {
193193
t.Errorf("pair not found with ordered key %v", key)
@@ -199,7 +199,7 @@ func TestRecordCommit_IgnoredPaths(t *testing.T) {
199199
a := &Analyzer{}
200200
pairs := make(map[filePair]int)
201201
totals := make(map[string]int)
202-
a.recordCommit([]string{"testdata/foo.go", "internal/bar.go"}, pairs, totals)
202+
a.recordCommit([]string{"testdata/foo.go", "internal/bar.go"}, pairs, totals, make(map[string]bool))
203203
if _, ok := totals["testdata/foo.go"]; ok {
204204
t.Error("testdata file should be ignored")
205205
}
@@ -212,7 +212,7 @@ func TestRecordCommit_ThreeFiles(t *testing.T) {
212212
a := &Analyzer{}
213213
pairs := make(map[filePair]int)
214214
totals := make(map[string]int)
215-
a.recordCommit([]string{"a.go", "b.go", "c.go"}, pairs, totals)
215+
a.recordCommit([]string{"a.go", "b.go", "c.go"}, pairs, totals, make(map[string]bool))
216216
// 3 files → 3 pairs: (a,b), (a,c), (b,c)
217217
if len(pairs) != 3 {
218218
t.Errorf("expected 3 pairs for 3 files, got %d", len(pairs))
@@ -232,8 +232,8 @@ func TestScanOptionsDefaults(t *testing.T) {
232232
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
233233
a := NewAnalyzer(dir, logger)
234234

235-
// A zero ScanOptions (except RepoRoot) should not panic and should use defaults.
236-
result, err := a.Scan(context.Background(), ScanOptions{RepoRoot: dir})
235+
// A zero ScanOptions should not panic and should use defaults.
236+
result, err := a.Scan(context.Background(), ScanOptions{})
237237
if err != nil {
238238
t.Fatalf("Scan() with zero opts error = %v", err)
239239
}
@@ -254,7 +254,7 @@ func TestScan_EmptyRepo(t *testing.T) {
254254
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
255255
a := NewAnalyzer(dir, logger)
256256

257-
result, err := a.Scan(context.Background(), ScanOptions{RepoRoot: dir, WindowDays: 365, MinCoChanges: 2})
257+
result, err := a.Scan(context.Background(), ScanOptions{WindowDays: 365, MinCoChanges: 2})
258258
if err != nil {
259259
t.Fatalf("Scan() error = %v", err)
260260
}
@@ -281,7 +281,6 @@ func TestScan_DetectsHiddenCoupling(t *testing.T) {
281281
a := NewAnalyzer(dir, logger)
282282

283283
result, err := a.Scan(context.Background(), ScanOptions{
284-
RepoRoot: dir,
285284
WindowDays: 365,
286285
MinCorrelation: 0.3,
287286
MinCoChanges: 3,
@@ -316,7 +315,6 @@ func TestScan_SkipsPairWithImportEdge(t *testing.T) {
316315
a := NewAnalyzer(dir, logger)
317316

318317
result, err := a.Scan(context.Background(), ScanOptions{
319-
RepoRoot: dir,
320318
WindowDays: 365,
321319
MinCorrelation: 0.3,
322320
MinCoChanges: 3,
@@ -355,7 +353,6 @@ func TestScan_MinCorrelationFilters(t *testing.T) {
355353

356354
// High threshold: should still find the pair (correlation is 1.0).
357355
result, err := a.Scan(context.Background(), ScanOptions{
358-
RepoRoot: dir,
359356
WindowDays: 365,
360357
MinCorrelation: 0.9,
361358
MinCoChanges: 3,
@@ -386,7 +383,6 @@ func TestScan_MinCoChangesFilters(t *testing.T) {
386383
a := NewAnalyzer(dir, logger)
387384

388385
result, err := a.Scan(context.Background(), ScanOptions{
389-
RepoRoot: dir,
390386
WindowDays: 365,
391387
MinCorrelation: 0.3,
392388
MinCoChanges: 3, // requires at least 3
@@ -400,7 +396,6 @@ func TestScan_MinCoChangesFilters(t *testing.T) {
400396

401397
// Lower the threshold: pair should now appear.
402398
result2, err := a.Scan(context.Background(), ScanOptions{
403-
RepoRoot: dir,
404399
WindowDays: 365,
405400
MinCorrelation: 0.3,
406401
MinCoChanges: 2,
@@ -433,7 +428,6 @@ func TestScan_LimitRespected(t *testing.T) {
433428
a := NewAnalyzer(dir, logger)
434429

435430
result, err := a.Scan(context.Background(), ScanOptions{
436-
RepoRoot: dir,
437431
WindowDays: 365,
438432
MinCorrelation: 0.3,
439433
MinCoChanges: 3,
@@ -465,7 +459,6 @@ func TestScan_FilterTestdataPaths(t *testing.T) {
465459
a := NewAnalyzer(dir, logger)
466460

467461
result, err := a.Scan(context.Background(), ScanOptions{
468-
RepoRoot: dir,
469462
WindowDays: 365,
470463
MinCorrelation: 0.3,
471464
MinCoChanges: 3,
@@ -497,7 +490,6 @@ func TestScan_SummaryFields(t *testing.T) {
497490
a := NewAnalyzer(dir, logger)
498491

499492
result, err := a.Scan(context.Background(), ScanOptions{
500-
RepoRoot: dir,
501493
WindowDays: 365,
502494
MinCorrelation: 0.3,
503495
MinCoChanges: 3,

0 commit comments

Comments
 (0)