-
Notifications
You must be signed in to change notification settings - Fork 440
fix(profiling): increase default max frames for stack profiler to 512 #13323
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 272 ± 3 ms. The average import time from base is: 278 ± 4 ms. The import time difference between this PR and base is: -6.6 ± 0.1 ms. Import time breakdownThe following import paths have shrunk:
|
BenchmarksBenchmark execution time: 2025-06-06 16:17:22 Comparing candidate commit 57b104d in PR branch Found 0 performance improvements and 5 performance regressions! Performance is the same for 494 metrics, 3 unstable metrics. scenario:iastaspects-lower_aspect
scenario:iastaspects-replace_aspect
scenario:iastaspects-upper_aspect
scenario:iastaspectsospath-ospathjoin_aspect
scenario:iastaspectsospath-ospathnormcase_aspect
|
@taegyunkim is this ready for review? |
Background
max_frames (
DD_PROFILING_MAX_FRAMES
) is used to control how many frames we sample/export and show to the user. stack profiler, memory profiler, and lock profiler uses the same max_frames even though they have different performance characteristics with regards to increasing that number. We believe stack_v2 sampler is performant enough as it by default unwinds 2048 frames, set in here. But still, we only exportDD_PROFILING_MAX_FRAMES
number of frames to the backend.One thing we can do is simply hardcoding up to 512 (the maximum number of frames that can be handled by the backend) frames in sample_manager.cpp as below
Instead of current implementation here
Then, we don't need to plumb
max_frames
toddup.config()
and that for stack profilerDD_PROFILING_MAX_FRAMES
would be relevant only for the legacy stack profiler, lock profiler and memory profiler.Checklist
Reviewer Checklist