Repository navigation
blocklog: look a block up by index, 51us down to 0.5us on a full log - #516
Merged
Merged
Conversation
Gemini PR ReviewReviewed commit:
|
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.
Profiling a blocked execute_step with memory on put blocklog.Summary at about a quarter of the time. It scanned every kept entry (up to 10,000) under the log's mutex, on the first block of each step in a run, so the cost grew with the log and held up every other recorder.
The log now keeps, per (workflow, version, step, rule, missing state), the sequence numbers of its block entries in order. Summary reads only those. Trimming the oldest entry pops it from the front of its list, so nothing stale is left behind, and the result is the same aggregation in the same order as before.
Same machine, full 10,000-entry log: Summary 51,015 ns to 493 ns (0 allocs both ways). The whole memory-on blocked execute_step through the in-process MCP client went from about 159 us to 127 us per call; the rest of that is the MCP library's JSON handling, which this does not touch.
TestSummaryIndexAgreesWithAFullScan records 600 random blocks, runs and recoveries through trims and expiry with a small cap and a short TTL, and after every step checks all lookups against the old scan (kept in the test file as the reference, and as BenchmarkSummaryByScan for comparison). It also checks the index lists exactly the block entries still held. go test -race passes for blocklog, mcpserver, cmd/sop-mcp-server and verify.
Not changed: the barrier itself (43 ns allowed, 350 ns blocked, no allocations on the allowed path) is already tight, and Summaries and Stats still scan, since they are called on connect and on read_lessons, not per block.
Thanks, Gerard Recinto