Skip to content

Move rt_regs from stack to record scratch#1286

Merged
fabled merged 1 commit into
open-telemetry:mainfrom
parca-dev:shrink-stack
Apr 16, 2026
Merged

Move rt_regs from stack to record scratch#1286
fabled merged 1 commit into
open-telemetry:mainfrom
parca-dev:shrink-stack

Conversation

@gnurizen
Copy link
Copy Markdown
Contributor

@gnurizen gnurizen commented Mar 25, 2026

This is another prep the patient PR for a hybrid python native unwinder. Combining the two unwinders exhausted the bpf stack and this fixes it.

@gnurizen gnurizen changed the title shrink stack Move rt_regs from stack to record scratch Mar 25, 2026
@gnurizen
Copy link
Copy Markdown
Contributor Author

I was hoping for this PR to just be one commit relative to PR 1280 but GH requires base branch to be in this repo.

@gnurizen gnurizen marked this pull request as ready for review March 25, 2026 18:45
@gnurizen gnurizen requested review from a team as code owners March 25, 2026 18:45
@gnurizen gnurizen force-pushed the shrink-stack branch 2 times, most recently from c8ee363 to 0cf56fc Compare March 26, 2026 16:16
@fabled
Copy link
Copy Markdown
Contributor

fabled commented Apr 8, 2026

I think the commit to move rt_regs to scratch looks good. Could this be done before the other PR? That is, can you rebase this directly on top of main?

@gnurizen
Copy link
Copy Markdown
Contributor Author

gnurizen commented Apr 8, 2026

Yeah I can rebase this to main and make it standalone and pull this out of the commit set in #1288

@gnurizen
Copy link
Copy Markdown
Contributor Author

gnurizen commented Apr 8, 2026

Rebased to main and RFAL.

@gnurizen
Copy link
Copy Markdown
Contributor Author

gnurizen commented Apr 15, 2026

@fabled @florianl @christos68k whats the best way to move this forward? I feel like I shouldn't bother rebasing/blob rebuilding again until it gets some green checkmarks...

Copy link
Copy Markdown
Member

@christos68k christos68k left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread support/ebpf/__pycache__/bpf_budget.cpython-312.pyc Outdated
@gnurizen gnurizen force-pushed the shrink-stack branch 2 times, most recently from fd026c0 to 8a865af Compare April 15, 2026 17:51
On arm64, rt_regs[34] consumes 272 bytes of the 512-byte BPF stack.
When unwind_one_frame is inlined into interpreter unwinders, this
exceeds the stack limit. Move rt_regs into the PerCPURecord scratch
union which is already 1024+ bytes and unused during signal frame
handling.
@gnurizen
Copy link
Copy Markdown
Contributor Author

rebased and python droppings removed

Copy link
Copy Markdown
Contributor

@fabled fabled left a comment

Choose a reason for hiding this comment

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

lgtm

@fabled fabled merged commit c1760f0 into open-telemetry:main Apr 16, 2026
32 checks passed
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