Conversation
PR SummaryLow Risk Overview Introduces a set of Go benchmarks covering repo creation, transcript generation, session state creation, and checkpoint seeding, and adds Written by Cursor Bugbot for commit 2c2e550. Configure here. |
There was a problem hiding this comment.
Pull request overview
Adds a dedicated benchmark fixture utility package to help create realistic git/session/checkpoint data for performance testing, and wires up new mise tasks to run benchmarks (plus updates community docs).
Changes:
- Add
cmd/entire/cli/benchutilpackage with helpers to construct benchmark repos, session state, transcripts, and seeded checkpoint branches. - Add
misetasks for running benchmarks (and generating CPU/memory profiles). - Update Discord invite links in
CONTRIBUTING.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
mise.toml |
Adds bench, bench:cpu, and bench:mem tasks for running benchmarks and profiles. |
cmd/entire/cli/benchutil/benchutil.go |
Introduces benchmark fixture helpers for building temp repos, transcripts, session state, and seeded checkpoint history. |
cmd/entire/cli/benchutil/benchutil_test.go |
Adds benchmark entrypoints that exercise the new fixture helpers. |
CONTRIBUTING.md |
Updates Discord URLs. |
| metadataDir := paths.SessionMetadataDirFromSessionID(sessionID) | ||
| metadataDirAbs := filepath.Join(br.Dir, metadataDir) | ||
| if err := os.MkdirAll(metadataDirAbs, 0o750); err != nil { | ||
| b.Fatalf("mkdir metadata: %v", err) | ||
| } | ||
|
|
||
| // Write a minimal transcript to the metadata dir | ||
| transcriptPath := filepath.Join(metadataDirAbs, "full.jsonl") | ||
| transcript := GenerateTranscript(TranscriptOpts{MessageCount: 5, AvgMessageBytes: 200}) | ||
| if err := os.WriteFile(transcriptPath, transcript, 0o600); err != nil { | ||
| b.Fatalf("write transcript: %v", err) | ||
| } | ||
|
|
||
| _, err := br.Store.WriteTemporary(context.Background(), checkpoint.WriteTemporaryOptions{ | ||
| SessionID: sessionID, | ||
| BaseCommit: br.HeadHash, | ||
| WorktreeID: br.WorktreeID, | ||
| ModifiedFiles: modified, | ||
| MetadataDir: metadataDir, | ||
| MetadataDirAbs: metadataDirAbs, | ||
| CommitMessage: fmt.Sprintf("Checkpoint %d", i+1), | ||
| AuthorName: "Bench User", | ||
| AuthorEmail: "bench@example.com", | ||
| IsFirstCheckpoint: i == 0, | ||
| }) |
There was a problem hiding this comment.
SeedShadowBranch calls GitStore.WriteTemporary, which internally relies on paths.RepoRoot() and git status from the current working directory. Since this helper never switches the process CWD to br.Dir (and clears paths' repo-root cache), it will resolve files against the caller’s repo instead of the benchmark repo, producing incorrect checkpoint trees/changed-file detection. Wrap the WriteTemporary call (and any first-checkpoint collection) in a temporary chdir to br.Dir and call paths.ClearRepoRootCache() after switching (restore CWD afterward).
| func BenchmarkSeedShadowBranch(b *testing.B) { | ||
| repo := NewBenchRepo(b, RepoOpts{FileCount: 10}) | ||
| sessionID := repo.CreateSessionState(b, SessionOpts{}) | ||
|
|
||
| b.ResetTimer() | ||
| for b.Loop() { | ||
| // Each iteration seeds a fresh shadow branch | ||
| // (will append to existing, but that's fine for benchmarking) | ||
| repo.SeedShadowBranch(b, sessionID, 5, 3) | ||
| } | ||
| } | ||
|
|
||
| func BenchmarkSeedMetadataBranch(b *testing.B) { | ||
| repo := NewBenchRepo(b, RepoOpts{FileCount: 10}) | ||
|
|
||
| b.ResetTimer() | ||
| for b.Loop() { | ||
| repo.SeedMetadataBranch(b, 10) | ||
| } | ||
| } |
There was a problem hiding this comment.
These benchmarks reuse the same temp repo across iterations while mutating git history/metadata on each loop. That makes later iterations operate on a larger repo/branch than early iterations, which can skew benchmark results and reduce comparability. Consider moving repo/session setup inside the loop with the timer stopped (or recreate/reset the repo/branches per iteration) so each iteration measures the same steady-state work.
| run = "go test -bench=. -benchmem -run='^$' -cpuprofile=cpu.prof -timeout=10m ./... && echo 'Profile saved to cpu.prof. View with: go tool pprof -http=:8080 cpu.prof'" | ||
|
|
||
| [tasks."bench:mem"] | ||
| description = "Run benchmarks with memory profile" | ||
| run = "go test -bench=. -benchmem -run='^$' -memprofile=mem.prof -timeout=10m ./... && echo 'Profile saved to mem.prof. View with: go tool pprof -http=:8080 mem.prof'" |
There was a problem hiding this comment.
The bench:cpu/bench:mem tasks run go test ./... with -cpuprofile/-memprofile and then print a message implying a single profile file in the current directory. With multiple packages, Go runs each package’s test binary from its package directory, so profiles may be written per-package (and/or not end up where the message suggests). Consider documenting where the profiles land, or change the task to target a single package/output path to avoid confusion.
| run = "go test -bench=. -benchmem -run='^$' -cpuprofile=cpu.prof -timeout=10m ./... && echo 'Profile saved to cpu.prof. View with: go tool pprof -http=:8080 cpu.prof'" | |
| [tasks."bench:mem"] | |
| description = "Run benchmarks with memory profile" | |
| run = "go test -bench=. -benchmem -run='^$' -memprofile=mem.prof -timeout=10m ./... && echo 'Profile saved to mem.prof. View with: go tool pprof -http=:8080 mem.prof'" | |
| run = "go test -bench=. -benchmem -run='^$' -cpuprofile=cpu.prof -timeout=10m ./... && echo 'CPU profiles saved as cpu.prof in each benchmarked package directory. List them with: find . -name cpu.prof -print. View one with: go tool pprof -http=:8080 path/to/cpu.prof'" | |
| [tasks."bench:mem"] | |
| description = "Run benchmarks with memory profile" | |
| run = "go test -bench=. -benchmem -run='^$' -memprofile=mem.prof -timeout=10m ./... && echo 'Memory profiles saved as mem.prof in each benchmarked package directory. List them with: find . -name mem.prof -print. View one with: go tool pprof -http=:8080 path/to/mem.prof'" |
| err = br.Store.WriteCommitted(context.Background(), checkpoint.WriteCommittedOptions{ | ||
| CheckpointID: cpID, | ||
| SessionID: sessionID, | ||
| Strategy: "manual-commit", | ||
| Transcript: transcript, | ||
| Prompts: []string{fmt.Sprintf("Implement feature %d", i)}, | ||
| FilesTouched: files, | ||
| CheckpointsCount: 3, | ||
| AuthorName: "Bench User", | ||
| AuthorEmail: "bench@example.com", | ||
| Agent: agent.AgentTypeClaudeCode, | ||
| }) |
There was a problem hiding this comment.
SeedMetadataBranch hard-codes WriteCommittedOptions.Strategy to "manual-commit" regardless of the RepoOpts.Strategy used to initialize .entire/settings.json. If callers create a bench repo with a different strategy, the seeded committed checkpoints won’t match repo configuration and may not reflect realistic metadata. Consider threading the strategy through BenchRepo (store it from NewBenchRepo) or accepting it as a parameter here.
1 participant
No description provided.