Skip to content

cherry-pick upstream ruby yjit related unmerged changes#53

Merged
korniltsev-grafanista merged 11 commits intopyroscope_alloyfrom
kk/ruby-dogfooding
Mar 26, 2026
Merged

cherry-pick upstream ruby yjit related unmerged changes#53
korniltsev-grafanista merged 11 commits intopyroscope_alloyfrom
kk/ruby-dogfooding

Conversation

@korniltsev-grafanista
Copy link
Copy Markdown

@korniltsev-grafanista korniltsev-grafanista commented Mar 17, 2026

There will be conflicts once upstream changes land, well need to tackle them later

@korniltsev-grafanista korniltsev-grafanista changed the title Kk/ruby dogfooding ruby dogfooding Mar 17, 2026
@korniltsev-grafanista korniltsev-grafanista marked this pull request as ready for review March 25, 2026 06:39
@korniltsev-grafanista korniltsev-grafanista requested a review from a team March 25, 2026 06:39
@korniltsev-grafanista korniltsev-grafanista changed the title ruby dogfooding cherry-pick upstream ruby yjit related changes Mar 25, 2026
@korniltsev-grafanista korniltsev-grafanista changed the title cherry-pick upstream ruby yjit related changes cherry-pick upstream ruby yjit related unmerged changes Mar 25, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5c77c6b27d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1218 to +1219
fmt.Printf("YJIT DEBUG {Vaddr: %#x, Length: %#x, Flags: %#x, Path: libpf.Intern(%q)},\n",
m.Vaddr, m.Length, uint32(m.Flags), m.Path.String())
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Remove unconditional YJIT map dump from sync path

When YJIT detection fails, this fmt.Printf runs for every mapping on every SynchronizeMappings call, which can emit massive stdout volume for long-running Ruby processes that never enable YJIT (or before YJIT initializes). That adds avoidable CPU/I/O overhead in a hot path and leaks full mapping paths into logs by default; this should be guarded behind debug logging or removed.

Useful? React with 👍 / 👎.

}
continue
}
withinFoundRange := m.Vaddr < end && m.Vaddr+m.Length <= end
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject partially overlapping mappings in YJIT range check

The overlap test only treats mappings as "in range" when they end at or before end, so a file-backed mapping that starts before end but extends past it is ignored. In that case findYJITRegion can still return found=true even though the candidate interval contains non-anonymous code, leading to false YJIT region detection and incorrect interpreter mapping for that process.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@simonswine simonswine left a comment

Choose a reason for hiding this comment

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

LGTM

@korniltsev-grafanista korniltsev-grafanista merged commit a1d1e41 into pyroscope_alloy Mar 26, 2026
38 checks passed
blewis12 pushed a commit to grafana/alloy that referenced this pull request Mar 26, 2026
Update go.opentelemetry.io/ebpf-profiler replacement to latest
pyroscope_alloy branch commit from
github.com/grafana/opentelemetry-ebpf-profiler.

Old: v0.0.202602-0.20260216144214-241376220646
New: v0.0.202602-0.20260326091923-bd31a19190b9

This is to include this fix for customer escalation
grafana/opentelemetry-ebpf-profiler#53
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