Skip to content

Increase frame buffer to max 1024 frames per trace #908

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dalehamel wants to merge 1 commit into from
Closed

Increase frame buffer to max 1024 frames per trace #908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified support/ebpf/tracer.ebpf.amd64
View file
Open in desktop
Binary file not shown.
Binary file modified support/ebpf/tracer.ebpf.arm64
View file
Open in desktop
Binary file not shown.
4 changes: 3 additions & 1 deletion support/ebpf/types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,9 @@ typedef enum TraceOrigin {
// MAX_FRAME_UNWINDS defines the maximum number of frames per
// Trace we can unwind and respect the limit of eBPF instructions,
// limit of tail calls and limit of stack size per eBPF program.
#define MAX_FRAME_UNWINDS 128
// 1024 is the maximum power of 2 we can fit in a single perf event, and
// in a single per CPU array entry
#define MAX_FRAME_UNWINDS 1024
Copy link
Copy Markdown
Member

@florianl florianl Nov 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Not every environment does run ruby application and so will not directly benefit from this change. As with this change, eight times more memory is required for every trace that is reported from kernel to user space, did you measure the impact of this change for various workloads?

Copy link
Copy Markdown
Contributor

@korniltsev-grafanista korniltsev-grafanista Nov 5, 2025

Choose a reason for hiding this comment

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

The link you shared is for race sanitizer to workaround exactly that fact(only for race sanitizer) that we do not send the whole trace to userspace.
We should not be sending empty frames

opentelemetry-ebpf-profiler/support/ebpf/tracemgmt.h

Line 381 in e413d8c

const u64 send_size = sizeof(Trace) - sizeof(Frame) * num_empty_frames;

Copy link
Copy Markdown
Member

@florianl florianl Nov 5, 2025

Choose a reason for hiding this comment

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

Sorry that I mixed up the race sanitizer work around. As this was just resently changed, it was the first that dig pop into my head.

Another component that will grow with this increase are the eBPF maps per_cpu_records.

How close are we getting to unwind 1024 frames? Without further changes, we might be limited:

Unwinder Unwinder limit Unwinder limit * tail call limit
native NATIVE_FRAMES_PER_PROGRAM = 4 4 * 32 = 128
perl PERL_FRAMES_PER_PROGRAM = 12 12 * 32 = 384
php FRAMES_PER_WALK_PHP_STACK = 19 19 * 32 = 608
python FRAMES_PER_WALK_PYTHON_STACK = 12 12 * 32 = 384
ruby FRAMES_PER_WALK_RUBY_STACK = 27 27 * 32 = 864
v8 V8_FRAMES_PER_PROGRAM = 8 8 * 32 = 256
dotnet DOTNET_FRAMES_PER_PROGRAM = 6 6 * 12 = 192

With further jumps between the different unwinders, the actual number might be lower.

Copy link
Copy Markdown
Contributor Author

@dalehamel dalehamel Nov 5, 2025

Choose a reason for hiding this comment

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

@florianl yeah so just about everything is potentially truncated right now.

Another component that will grow with this increase are the eBPF maps per_cpu_records.

I picked 1024 because It is the largest number that still fits into a single perf event, which is the limit for the per cpu record. Any higher power of 2 won't fit. There is a discussion about how to potentially go higher than that, as the per cpu record is currently an array of size of 1 but it doesn't necessarily need to be.

It is actually half of the size of other ruby profilers (stackprof and vernier allow a buffer of 2048, believe it or not - and rbspy is theoretically unlimited). My PR #907 raises the number of frames per tail call to 32, so the max theoretical number for ruby would actually be ~ 32 * 32 = 1024.

I don't believe we will really be wasting any memory here as profilers that don't write this many frames won't really allocate it. The per CPU array entry size does increase to something like 24Kb, but this isn't much of an increase on the system over all even if you have lots of processors. The benefit of being able to have untruncated stacks I think is worth it.

On the golang side, we should only end up actually transferring as much data as was actually written, so if memory increases it should be because we legitimately are sending more (useful) data to userspace.


// MAX_NON_ERROR_FRAME_UNWINDS defines the maximum number of frames
// to be pushed by unwinders while still leaving space for an error frame.
Expand Down
6 changes: 3 additions & 3 deletions support/types.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.