Skip to content

perf: reduce transcript and repeated-secret scan allocations - #2291

Open
suhaanthayyil wants to merge 2 commits into
mainfrom
codex/cli-performance-only
Open

perf: reduce transcript and repeated-secret scan allocations#2291
suhaanthayyil wants to merge 2 commits into
mainfrom
codex/cli-performance-only

Conversation

@suhaanthayyil

@suhaanthayyil suhaanthayyil commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

https://entire.io/gh/entireio/cli/trails/1241

Summary

  • Search each distinct betterleaks finding value once per String call, avoiding duplicate whole-input searches and duplicate regions. For valid JSONL, this benefit is within one string field, such as a large tool result containing repeated request-log entries; equal values in separate fields or lines are not deduplicated together.
  • Read transcript blobs directly into a size-reserved byte buffer instead of copying through File.Contents() strings; preallocate the chunk slice as well. The size hint covers both normal chunks and current unchunked shadow transcripts, up to a 1 GiB sanity ceiling. Above that, reads retain incremental growth. This ceiling is an allocation hint limit, not a read-size limit.
  • Preserve transcript bytes, fallback behavior, and redaction output. Each duplicate finding previously generated the same unlabeled regions; eliminating duplicate work does not change output, so no configFingerprintVersion bump is needed. No API or configuration changes.
  • Keep characterization coverage for base, legacy, present-empty, absent, and numerically ordered chunked transcripts. The present-empty result must remain non-nil because ephemeral reads use nil to select the legacy fallback. Use filename constants consistently.

Benchmarks

Original before/after medians from five runs with -benchtime=500ms -benchmem, Go 1.26.6 on darwin/arm64 (Apple M5 Pro):

Workload Time before → after Allocated bytes before → after
1,000 repeated credentials in one String call 175.428 → 116.936 ms 292.007 → 119.687 MB
1 MiB transcript read 380.988 → 69.679 µs 6,291,204 → 1,057,024
8 MiB transcript read 1,233.019 → 414.637 µs 50,331,431 → 8,397,057

The existing ClaudeFull2 fixture was effectively unchanged (167.462 → 163.730 ms).

Soph's review identified that the original 50 MB hint ceiling excluded current unchunked shadow transcripts. The new 64 MiB benchmark exercises that case. Fresh A/B medians comparing the previous PR implementation (caf73c69b) with the amendment, using the same new benchmark, Go 1.26.6, Apple M5 Pro, GOMAXPROCS=4, -benchtime=20x -count=3 -benchmem:

64 MiB transcript Previous PR Amendment
Time 6.299 ms 2.952 ms
Allocated bytes 268,435,280 67,117,320
Allocations 24 6

That is approximately 75% fewer allocated bytes. The 1 MiB and 8 MiB allocation counts remain unchanged. These benchmarks use real in-memory Git objects to isolate read/allocation costs; they are not end-to-end CLI speedup, peak-RSS, or production-frequency claims. The frequency of transcripts above 50 MB has not been measured here.

Benchmark output assertions check correctness. Performance comparisons are manual against a base ref; CI does not enforce an allocation or timing baseline.

go test ./redact -run '^$' -bench '^BenchmarkRedactStringRepeatedSecret$/Occurrences1000$' -benchtime=500ms -count=5 -benchmem
GOMAXPROCS=4 go test ./cmd/entire/cli/checkpoint -run '^TestReadTranscriptFromTreeFormats$' -bench '^BenchmarkReadTranscriptFromTree$' -benchtime=20x -count=3 -benchmem

Scope

agent.ReassembleJSONL pre-sizing, image-asset reads, and direct legacy fallback reads remain separate follow-ups, as requested in review.

Validation

  • Same-benchmark A/B for 1 MiB, 8 MiB, and 64 MiB; three runs per side.
  • TestReadTranscriptFromTreeFormats and focused redaction checks.
  • mise run check: formatting, lint (0 issues), race-enabled unit/integration suite, and deterministic Vogon/Roger-Roger canaries; exit 0 in 241.96 seconds.
  • golangci-lint config verify separately passed. Its online schema fetch was blocked inside the otherwise network-restricted full-check run; the actual lint analysis passed there.
  • Independent read-only review and git diff --check.

Vogon reports 56/56 and Roger-Roger 4/4; raw Vogon output includes four expected agent-specific skips. Reassembly and image-read follow-ups were not implemented.

@suhaanthayyil
suhaanthayyil requested a review from a team as a code owner September 7, 2026 01:34
Copilot AI lite review requested due to automatic review settings September 7, 2026 01:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The newly added transcript read test file contains a Go syntax error (map[string][]byte{"full.jsonl": {}}) that will fail compilation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR improves performance in two hot paths: redaction (betterleaks repeated-secret handling) and checkpoint transcript blob reading, while adding benchmarks/tests to lock in output and format behavior.

Changes:

  • Avoids repeated whole-input rescans in betterleaks by deduplicating findings by secret value per redaction call.
  • Reads transcript blobs directly into pre-sized byte buffers (avoiding string round-trips) and preallocates chunk slices.
  • Adds benchmarks and tests covering transcript read formats (base/legacy/empty/absent/chunk ordering) and repeated-secret redaction output.
File summaries
File Description
redact/redact.go Deduplicates betterleaks findings by secret to prevent quadratic rescans/region growth.
redact/redact_bench_test.go Adds a benchmark ensuring repeated-secret redaction remains output-preserving and allocation-efficient.
cmd/entire/cli/checkpoint/persistent.go Reworks transcript blob reading to avoid Contents() string copies; preallocates chunk storage.
cmd/entire/cli/checkpoint/transcript_read_test.go Adds tests/benchmarks for transcript read behavior across base/legacy/chunked formats.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +24 to +27
{name: "base", files: map[string][]byte{"full.jsonl": []byte("first\nsecond\n")}, want: []byte("first\nsecond\n")},
{name: "legacy", files: map[string][]byte{paths.TranscriptFileNameLegacy: []byte("legacy\n")}, want: []byte("legacy\n")},
{name: "empty", files: map[string][]byte{"full.jsonl": {}}, want: []byte{}},
{name: "absent", files: map[string][]byte{}, want: nil},
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants