Repository navigation
Conversation
A map made while a scoped or goroutine arena is current is allocated from that
arena and dies with it. A map made on the MAIN goroutine's arena is
malloc-backed instead — a long-lived map has to survive arena_reset(), and
nothing at make() distinguishes it from a transient one — so it lives until
the process exits and arena_reset() does not touch it.
That is harmless for the long-lived caches the malloc path exists for. It
bites exactly one shape: a single-actor server whose request loop runs on main
because its engine is not goroutine-safe, so it can neither hand each
connection to a goroutine nor wrap the loop body in arena { } (the engine
writes to globals and ARENA001 correctly refuses the escape). Such a server
leaks one lookup table per request. Measured: 200k maps cost 104 MB.
map_free(m) hands that memory back — entries, their keys and values, the
bucket array and the struct. On a map the arena owns it is a deliberate no-op
rather than an error, because the caller usually cannot tell which path make()
took, and a silently-correct no-op beats a footgun that only fires in the
configuration nobody tested.
Measured on the same workload: 104,320 kB without, 10,624 kB with. Clean under
AddressSanitizer on both paths. The real consumer — microexits, whose
per-request headers map comes from machweb's parse_headers on the main
goroutine — went from 1.5 kB/request to 0.07.
The test compares peak RSS of the same workload built twice, with and without
the call, rather than against an absolute threshold. The first version read
/proc/self/status from MFL to measure RSS and silently compared 0 against 0,
because read_file returns 0 bytes on procfs (st_size is 0 there); it passed
with map_free entirely stubbed out. Verified the current one fails when the
implementation is disabled.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJdASnjD4R5h6xX2fzfMfk
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The self-hosted compiler embeds its own copy of the C runtime, so adding mfl_map_free to codegen.go without re-running selfhost/gen-prelude.py would leave the self-hosted machin building programs against a runtime with no map_free in it. CI caught this; go test . does not, because the check is a separate step rather than a Go test. Note for next time: gen-prelude.py shells out to ./bin/machin, a build artifact. Running it without rebuilding that binary first regenerates the prelude with the OLD compiler and silently produces no diff, which looks like 'already up to date'. CORE goes 215,533 -> 216,551 bytes here; the decoded block contains mfl_map_free. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJdASnjD4R5h6xX2fzfMfk
javimosch
added a commit
that referenced
this pull request
Sep 22, 2026
…outine's arena) (#670) A map made while a scoped or goroutine arena is current is allocated from that arena and dies with it. A map made on the MAIN goroutine's arena is malloc-backed instead — a long-lived map has to survive arena_reset(), and nothing at make() distinguishes it from a transient one — so it lived until the process exited and arena_reset() did not touch it. That is right for the long-lived caches the malloc path exists for, and wrong for one shape: a single-actor server whose request loop runs on main because its engine is not goroutine-safe, so it can neither hand each connection to a goroutine nor wrap the loop body in arena { }. map_free(m) hands that memory back; on a map the arena owns it is a deliberate no-op. 94 MB of growth without it on a 200k-map workload, 1.5 MB with (#667, #669). Claude-Session: https://claude.ai/code/session_01XJdASnjD4R5h6xX2fzfMfk Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Closes #667.
A map made while a scoped or goroutine arena is current is allocated from that arena and dies with it. A map made on the main goroutine's arena is malloc-backed instead — a long-lived map has to survive
arena_reset(), and nothing atmake()distinguishes it from a transient one — so it lives until the process exits andarena_reset()does not touch it.That is right for the long-lived caches the malloc path exists for. It bites exactly one shape: a single-actor server whose request loop runs on main because its engine is not goroutine-safe, so it can neither hand each connection to a goroutine nor wrap the loop body in
arena { }(the engine writes to globals and ARENA001 correctly refuses the escape).map_free(m)hands that memory back — entries, their keys and values, the bucket array, the struct. On a map the arena owns it is a deliberate no-op rather than an error: the caller usually cannot tell which pathmake()took, and a silently-correct no-op beats a footgun that only fires in the configuration nobody tested.Measured
map_freemap_freeClean under AddressSanitizer on both the main-arena and arena-owned paths. Non-map arguments and wrong arity are refused at compile time.
Real consumer: microexits' per-request headers map comes from machweb's
parse_headerson the main goroutine. Freeing it took that service from 1.5 kB/request to 0.07.On the test
It compares peak RSS of the same workload built twice, with and without the call, minus a measured floor. Two earlier versions of this test were wrong in ways worth recording:
/proc/self/statusfrom MFL and compared 0 against 0, becauseread_filereturns empty on procfs (now read_file returns empty for /proc and /sys files (st_size is 0 there) #668) — it passed withmap_freeentirely stubbed out;ru_maxrssfor a child includes what it inherited at fork, so it passed alone and failed inside the suite where the parent's RSS had grown.The current one is verified to fail when the implementation is stubbed.
Full suite green:
ok machin 458.005s.🤖 Generated with Claude Code
https://claude.ai/code/session_01XJdASnjD4R5h6xX2fzfMfk
Summary by CodeRabbit
New Features
map_freebuiltin to release memory used by temporary maps that are no longer needed.Documentation
map_freeshould be used and added a related memory note.Tests