Skip to content

runtime: add memprofile attribution support#1905

Draft
cpunion wants to merge 5 commits into
xgo-dev:mainfrom
cpunion:codex/goroot-heapsampling-coverage
Draft

runtime: add memprofile attribution support#1905
cpunion wants to merge 5 commits into
xgo-dev:mainfrom
cpunion:codex/goroot-heapsampling-coverage

Conversation

@cpunion

@cpunion cpunion commented May 22, 2026

Copy link
Copy Markdown
Collaborator

Summary:

  • add a lightweight LLGo runtime memprofile snapshot with synthetic frames for runtime.MemProfile
  • preserve heap attribution frames by disabling inline/tail-call elimination for packages using runtime.MemProfile/MemProfileRate
  • add stable test/go coverage and un-xfail GOROOT heapsampling.go

Dependency / merge order:

Tests:

  • go test ./test/go -run TestRuntimeMemProfileAttribution -count=1
  • go test ./test/goroot -run TestGoRootRunCases -count=1 -args -goroot /opt/homebrew/Cellar/go@1.24/1.24.11/libexec -dirs . -case '^heapsampling.go$' -directive-mode runlike
  • go test ./cl ./ssa
  • cd runtime && go test ./internal/lib/runtime ./internal/runtime
  • git diff --cached --check

@cpunion

cpunion commented May 22, 2026

Copy link
Copy Markdown
Collaborator Author

Verification update for commit 622e1e5:\n\n- Root cause: Linux CI could not reliably recover LLGo Go function names from libunwind for memory profile stack attribution; the profile saw frames like n / __libc_start_main, so TestRuntimeMemProfileAttribution could not find main.profiledAlloc.\n- Fix: for packages that use runtime.MemProfile, the compiler now emits lightweight MemProfileEnter/Exit calls and the runtime uses that Go call stack before falling back to libunwind.\n- Local tests:\n - go test ./test/go -run TestRuntimeMemProfileAttribution -count=1 -v\n - go test ./test/go -count=1\n - go test ./ssa -count=1\n - go test ./cl -count=1\n\nPushed only to cpunion/llgo:codex/goroot-heapsampling-coverage. Monitoring the new CI run now.

@cpunion

cpunion commented May 22, 2026

Copy link
Copy Markdown
Collaborator Author

CI update for 622e1e5a0e3eeccd097599b6f4a6abb8639740f9:

  • Previously failing test/go memprofile attribution failure is fixed by keeping a lightweight Go call stack for runtime.MemProfile test packages.
  • Local verification passed:
    • go test ./test/go -run TestRuntimeMemProfileAttribution -count=1 -v
    • go test ./test/go -count=1
    • go test ./ssa -count=1
    • go test ./cl -count=1
  • New CI: the three previously failing jobs now pass:
    • test (ubuntu-latest, 19)
    • test (ubuntu-latest, 19, 1.24.2, 2)
    • test (ubuntu-latest, 19, 1.26.0, 2)

At the time of this update, the remaining visible checks are queued macOS jobs; no new failure is showing on the PR.

@codecov-commenter

codecov-commenter commented May 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cpunion

cpunion commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator Author

Resolved the latest merge conflict with xgo-dev/main (51d665e). The merge keeps main's generic/linkonce and BDWGC MemStats changes while preserving this PR's memprofile attribution instrumentation and heapsampling xfail removal.\n\nLocal verification:\n- go test -timeout 20m ./test/go/memprofile -count=1\n- go test -timeout 20m ./test/go -run 'Test(RuntimeMemProfileAttribution|RuntimeReadMemStatsAfterInitGC)' -count=1\n- (runtime module) go test -timeout 20m ./internal/runtime ./internal/lib/runtime -count=1\n- git diff --check

@cpunion

cpunion commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator Author

Added focused coverage for the memprofile attribution instrumentation paths.

Changes:

  • Added cl tests for runtime.MemProfile / runtime.MemProfileRate AST detection, command-line-arguments frame-name normalization, and generated MemProfileEnter / MemProfileExit plus noinline / disable-tail-calls IR attributes.
  • Added ssa tests for Builder.MemProfileEnter, Builder.MemProfileExit, and Function.DisableTailCalls IR emission.

Local verification:

  • go test ./cl -run 'TestPackageUsesRuntimeMemProfile|TestMemProfileFunctionName|TestCompileRuntimeMemProfileInstrumentation' -count=1
  • go test -coverprofile=/tmp/llgo-cl-focused.cover ./cl -run 'TestPackageUsesRuntimeMemProfile|TestMemProfileFunctionName|TestCompileRuntimeMemProfileInstrumentation' -count=1
    • covered applyNoInline, memProfileFunctionName, and packageUsesRuntimeMemProfile at 100% in the focused profile.
  • go test ./ssa -run TestMemProfileBuilderAndTailCallAttribute -count=1
  • go test -coverprofile=/tmp/llgo-ssa-focused.cover ./ssa -run TestMemProfileBuilderAndTailCallAttribute -count=1
    • covered DisableTailCalls, MemProfileEnter, and MemProfileExit at 100% in the focused profile.
  • go test ./test/go -run TestRuntimeMemProfileAttribution -count=1
  • go test ./test/go/memprofile -count=1
  • go test ./ssa -count=1
  • go test ./cl -count=1
  • git diff --check

CI note: the previous LLGo/download-model failure is still the known workflow-side HuggingFace download issue tracked by #1945. The job failed while downloading stories15M.bin from HuggingFace with 429 Too Many Requests; I did not change workflow/model download behavior in this PR.

@cpunion cpunion force-pushed the codex/goroot-heapsampling-coverage branch 2 times, most recently from a6f4e51 to e5ab91c Compare June 7, 2026 11:55
@cpunion cpunion marked this pull request as ready for review June 10, 2026 06:58
@cpunion cpunion force-pushed the codex/goroot-heapsampling-coverage branch from e5ab91c to e6875a7 Compare June 27, 2026 12:10
Comment thread cl/compile.go
goTyps: pkgTypes,
goPkg: pkg,
patches: patches,
noInlineForMemProfile: packageUsesRuntimeMemProfile(files),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个noinline的决策似乎只影响直接使用了MemProfile的包,间接的堆栈可能还是不准确?inline函数的栈和行号追踪是不是可以考虑像go一样自己建立一个表,在同一个函数内区分出来不同的部分。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhouguangyuan0718 是的,这里是有意的范围控制。

这个 PR 只在直接使用 runtime.MemProfile / MemProfileRate 的包里禁用 inline/tail-call 并加轻量 MemProfileEnter/Exit,目的是稳定当前 heapsampling/MemProfile 的直接归因,同时避免给所有包引入全局 instrumentation 开销。

间接包里的分配目前仍依赖 native stack fallback;如果符号或 inline 信息不足,确实不能保证像 Go 那样还原完整 inline call stack,也不能在同一个函数内区分所有 inlined call site。要做到这一点需要单独的 inline frame / line table 机制,应该放到后续 debug/lineinfo 方向处理,不放进这个 PR 扩大 scope。

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok 可以理解

@cpunion cpunion marked this pull request as draft June 28, 2026 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants