Repository navigation
compiler: stack allocate map key and range temporaries - #3039
Merged
cpunion merged 2 commits intoNov 10, 2026
Merged
Conversation
Collaborator
Author
|
@fennoai Please review the final runtime-temporary work for #3012: caller-owned map key copies and map/string range iterators, with typed GC roots, shared stack budgets, heap fallback, and preserved heap-owning iterator APIs. Local compiler, native/full-LTO and J32/J64/W32 regression suites pass; the PR body includes Go/main/fix measurements. |
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review summary
This PR lands #3012: map-key temporaries and map/string range iterators now use caller/invocation-owned entry-frame stack slots under the shared 64 KiB per-function escape-stack budget, with a heap fallback when the budget is exhausted. The design is clean and the change is well-tested.
Correctness / memory safety — looks sound. I traced the critical lifetime claims:
RangeBorrowedis applied to every*ssa.Range; Go'srangeIteris opaque, non-addressable, and consumed byNextin the same frame, so it cannot escape (return/store/closure/async). Entry-block allocas survive Asyncify/SjLj.mapKeyPtrstores the key before the map call, andmapassigncopies both direct and indirect keys into map-owned storage before returning — the temporary address is never retained. Fast key variants bypassmapKeyPtr.- GC roots:
uninitializedStackSlot→registerStackRootsregisters the iterator/key pointer fields andinitializeStackRootszero-inits the slot at entry, so GC never scans garbage, including on nil/empty-map restarts.NewMapIterBuffer/NewStringIterBufferfully re-init the buffer on every range restart. reserveEscapeStackarithmetic is safe: theescapeStackBytes > MaxEscapeStackBytesguard prevents theremainingsubtraction from underflowing, and addingStackRootBytescan only over-reject, never over-grant.
Nested/restarted ranges get distinct slots, and the range_temporary_test.go / map_key_storage_test.go suites exercise restart, nesting, mutation-after-yield, GC survival, and budget-exhaustion paths. Docs (doc/escape-analysis*.md, benchmark/escape/README.md) are accurate and consistent with the implementation.
Two non-blocking notes are inline. One additional minor doc observation: the new mapkeys benchmark ships without reference numbers in the "Reference measurements" table in benchmark/escape/README.md, so there are no expected values to compare a run against — fine if it's intended purely as a reproducer.
No blocking issues found.
Contributor
|
Confirmed. The heap-owning The diff is identical to the fully-reviewed version, and I already submitted a complete four-dimension review in the previous turn. Since this turn is managed mode, I return the final result as Markdown directly. Review: #3039 — runtime temporary ownership for #3012I reviewed the complete diff ( Verdict: No blocking issues. The design is clean and well-tested; the correctness-critical lifetime and GC claims hold. What I verified against your summary
Tests ( Non-blocking notes (carried from the detailed review)
These notes were also posted as inline comments on the PR during the earlier |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Collaborator
Author
|
@fennoai Updated the budget-policy comment and iterator-reset/ownership wording, and added the seven-case reference measurements plus the shared-budget limitation to the benchmark README. The data is the same pinned c70305d batch used in the PR and proposal tracker. This push changes comments/docs only; please re-review. |
Contributor
|
Confirmed. The only added Go lines in this push are comments — no executable code changed since the fully-reviewed All verified. This is managed mode, so I return the final result as Markdown. Re-review: #3039 — docs/comments update (
|
LLGo WebAssembly build benchmarks
WebAssembly output sizes
LLGo WebAssembly build measurements
Compared with |
LLGo baseline benchmarks
String equality benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
visualfc
approved these changes
Nov 10, 2026
visualfc
left a comment
visualfc
left a comment
Collaborator
There was a problem hiding this comment.
审查通过。
调用方持有的 map key 临时槽和 map/string range iterator 都落在共享 64KiB escape-stack 预算内，超预算走原有堆路径。mapassign 在返回前拷贝到 map 自有存储；Go 的 rangeIter 不能逃出当前调用；GC root 在入口清零。lowering 顺序的贪心预算只影响落点，不阻塞合并。
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.
Finish #3012's remaining runtime temporaries: caller-owned map keys and map/string range iterators. Reuse the shared 64 KiB stack budget and typed field roots; retain heap fallback and the existing heap-owning iterator APIs. Inserted keys keep map-owned copies, and restarted ranges reset all iterator fields.
M4 Max, macOS/arm64, LLVM 22.1.8, Go 1.27.3; five alternating rounds of 100,000 calls. LLGo uses
-O3 -lto=full, Go defaults. Baseline is main4a7f42648, including #3021–#3023; measured fixc70305d7f. Bytes are BDWGC/Go allocation accounting per call; timings are isolated kernels.Native probe: 2,138,144 → 2,137,760 bytes. Reproduce with
benchmark/escape/mapkeys.Validation: full compiler/SSA/C ABI/escape suites (
-short); native language, sync and synctest suites; native full LTO; J32/J64/W32 and two-worker J32 execution; six-target IR/budget checks; Go semantic tests; reproducible litgen checks. Both allocation guards fail on baseline and report 0 bytes after the fix.go vet ./...retains the same 12 baseline diagnostics.