perf(session): count messages without cloning session history - #4321
Merged
Merged
Conversation
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
aheritier
approved these changes
Sep 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 takinglenof the result.GetAllMessagesclones every message from every session in the tree into a new slice just to answer that question, allocating O(N) heap on every call.AllMessageCountreplaces that pattern with a read-locked recursive walk that counts nodes without materialising any slice. The three call sites inpkg/runtime/loop.go,pkg/server/server.go, andpkg/server/session_manager.goare updated to use it.The test file covers unit correctness on flat and deeply-nested trees, equivalence against
GetAllMessageson 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:GetAllMessagesis 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, plusgo test -race ./pkg/session. Fulltask build,task test, andtask lintwere 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.