Skip to content

perf(session): count messages without cloning session history - #4321

Merged
aheritier merged 1 commit into
mainfrom
perf/session-all-message-count
Sep 16, 2026
Merged

aheritier merged 1 commit into
mainfrom
perf/session-all-message-count

Conversation

@dgageot

@dgageot dgageot commented Sep 16, 2026

Copy link
Copy Markdown
Member

Three call sites in the runtime and server packages needed a message count from a session — including its child sessions — and all three did it by calling GetAllMessages() and taking len of the result. GetAllMessages clones every message from every session in the tree into a new slice just to answer that question, allocating O(N) heap on every call.

AllMessageCount replaces that pattern with a read-locked recursive walk that counts nodes without materialising any slice. The three call sites in pkg/runtime/loop.go, pkg/server/server.go, and pkg/server/session_manager.go are updated to use it.

The test file covers unit correctness on flat and deeply-nested trees, equivalence against GetAllMessages on random inputs, a concurrent-writer stress test for the lock path, and a benchmark across three fixture sizes (50 messages, 300 direct + 5×40 child, 2000 + 20×100 child). Before/after benchmark results against the three fixtures:

BenchmarkAllMessageCount/small_50msg
  before: 63808 B/op  58 allocs/op
  after:      0 B/op   0 allocs/op

BenchmarkAllMessageCount/medium_300direct+5x40child
  before: 798594 B/op  551 allocs/op
  after:       0 B/op    0 allocs/op

BenchmarkAllMessageCount/large_2000+20x100child
  before: 8356224 B/op  4197 allocs/op
  after:        0 B/op     0 allocs/op

GetAllMessages is untouched; callers that need actual messages continue to use it. The count preserves its system-message filter and sub-session selection, including items containing both a message and a sub-session.

Validation passed: affected-package build, tests, and scoped golangci-lint (0 issues) with CGO_ENABLED=0, plus go test -race ./pkg/session. Full task build, task test, and task lint were attempted locally but blocked by the unaccepted Xcode licence; the full tests also encountered an unavailable Docker Model Runner. Full-suite validation remains for CI. The figures above describe allocation savings on synthetic fixtures, not whole-product speedups.

Replace three len(GetAllMessages()) call sites with a read-locked
recursive count that walks the message tree without cloning slices.
Includes unit, concurrency, equivalence and benchmark tests.

Assisted-By: Claude
@dgageot
dgageot requested a review from a team as a code owner September 16, 2026 15:48
@aheritier aheritier added area/runtime Runtime engine, agent loop execution, tool dispatch, loop detection area/sessions For features/issues/fixes related to session lifecycle (resume, persistence, export) status/needs-triage For issues that need to be triaged kind/chore Maintenance, deps, CI, tooling (maps to chore: commit prefix) and removed status/needs-triage For issues that need to be triaged labels Sep 16, 2026
@aheritier
aheritier enabled auto-merge September 16, 2026 16:08
@aheritier
aheritier added this pull request to the merge queue Sep 16, 2026
Merged via the queue into main with commit 6512a3d Sep 16, 2026
26 of 28 checks passed
@aheritier
aheritier deleted the perf/session-all-message-count branch September 16, 2026 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/runtime Runtime engine, agent loop execution, tool dispatch, loop detection area/sessions For features/issues/fixes related to session lifecycle (resume, persistence, export) kind/chore Maintenance, deps, CI, tooling (maps to chore: commit prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants