Skip to content
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

[PROF-8667] Heap Profiling - Part 4 - Labels #3329

Merged
merged 3 commits into from
Jan 3, 2024

Conversation

AlexJF
Copy link
Contributor

@AlexJF AlexJF commented Dec 13, 2023

What does this PR do?

This PR follows #3328 by adding allocation class and gc gen age labels to the heap samples included in the Ruby profiles.

How does it work?

For allocation class, we simply hold on to the resolved class that was already being computed during the normal allocation sampling path.

For gc gen age, we record the GC Generation/Epoch (i.e. GC.count) when we saw the allocation happening. Then, when writing the heap samples in a new profile, we calculate the difference between the allocation generation and the current generation and that gives us the age of the recorded object in terms of "how many GCs have run since we saw this object being allocated".

Motivation:

  • Understand the type of objects that are being kept alive in our heap.
  • Understand the age dynamics of the objects being kept alive in heap (are they all "new" and likely

Additional Notes:

How to test the change?
Heap samples included in profiles when an app is executed with:

DD_PROFILING_EXPERIMENTAL_ALLOCATION_ENABLEd=true DD_PROFILING_EXPERIMENTAL_HEAP_ENABLED=true ddtracerb exec ...

should now include the above labels. This can be trivially tested by downloading these profiles and looking at them with some pprof parsing tool like https://github.com/felixge/pprofutils

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

@github-actions github-actions bot added the profiling Involves Datadog profiling label Dec 13, 2023
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part4-labels branch from a9a7a49 to 94bb1ae Compare December 13, 2023 17:31
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part3-queue branch from b4d7f97 to 4b21516 Compare December 13, 2023 17:33
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part4-labels branch 2 times, most recently from 720656c to 413d441 Compare December 13, 2023 17:39
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part3-queue branch 2 times, most recently from 8577e49 to fb50bbd Compare December 14, 2023 12:49
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part4-labels branch 2 times, most recently from 6578308 to 13275fb Compare December 14, 2023 13:08
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part3-queue branch from fb50bbd to 6674f5c Compare December 15, 2023 12:11
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part4-labels branch from 13275fb to 18d37c7 Compare December 15, 2023 12:14
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part3-queue branch from 6674f5c to ef2dfbf Compare December 15, 2023 12:22
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part4-labels branch from 18d37c7 to c935ff5 Compare December 15, 2023 12:22
@AlexJF AlexJF marked this pull request as ready for review December 15, 2023 12:34
@AlexJF AlexJF requested review from a team as code owners December 15, 2023 12:34
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part4-labels branch from c935ff5 to b8baedf Compare December 15, 2023 12:47
Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

Left some notes, but this looks sane! 👍

ext/ddtrace_profiling_native_extension/heap_recorder.c Outdated Show resolved Hide resolved
ext/ddtrace_profiling_native_extension/heap_recorder.c Outdated Show resolved Hide resolved
ext/ddtrace_profiling_native_extension/heap_recorder.c Outdated Show resolved Hide resolved
ext/ddtrace_profiling_native_extension/stack_recorder.c Outdated Show resolved Hide resolved
spec/datadog/profiling/stack_recorder_spec.rb Outdated Show resolved Hide resolved
spec/datadog/profiling/stack_recorder_spec.rb Outdated Show resolved Hide resolved
spec/datadog/profiling/stack_recorder_spec.rb Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (28dc187) 98.23% compared to head (1a2d02d) 98.23%.
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3329      +/-   ##
==========================================
- Coverage   98.23%   98.23%   -0.01%     
==========================================
  Files        1253     1253              
  Lines       73039    73048       +9     
  Branches     3431     3428       -3     
==========================================
+ Hits        71752    71760       +8     
- Misses       1287     1288       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part4-labels branch 2 times, most recently from 8e657f1 to 6a8c6ee Compare December 19, 2023 16:26
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part3-queue branch 2 times, most recently from b8bb16e to a598746 Compare December 20, 2023 17:58
Base automatically changed from alexjf/prof-8667-heap-profiling-part3-queue to master December 21, 2023 11:42
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part4-labels branch 5 times, most recently from 4610e55 to c20d042 Compare December 21, 2023 15:01
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part4-labels branch 2 times, most recently from 66dfb9d to 4f3b556 Compare December 21, 2023 15:10
AlexJF added 2 commits January 3, 2024 09:38
[PROF-8667] Rework queue system into a snapshot system.

[PROF-8667] Address some comments

[PROF-8667] Address comments
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part4-labels branch from 4f3b556 to 021d9b5 Compare January 3, 2024 10:39
Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Left a few final comments/suggestions

ext/ddtrace_profiling_native_extension/stack_recorder.c Outdated Show resolved Hide resolved
spec/datadog/profiling/stack_recorder_spec.rb Outdated Show resolved Hide resolved
spec/datadog/profiling/stack_recorder_spec.rb Outdated Show resolved Hide resolved
@AlexJF AlexJF force-pushed the alexjf/prof-8667-heap-profiling-part4-labels branch from 4b3d636 to 1a2d02d Compare January 3, 2024 16:36
@AlexJF AlexJF merged commit 7088ace into master Jan 3, 2024
218 checks passed
@AlexJF AlexJF deleted the alexjf/prof-8667-heap-profiling-part4-labels branch January 3, 2024 16:59
@github-actions github-actions bot added this to the 1.19.0 milestone Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
profiling Involves Datadog profiling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants