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-7307] Record allocation type when sampling objects #3096

Merged
merged 6 commits into from
Sep 1, 2023

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Aug 31, 2023

What does this PR do?

This PR extends the Profiling::Collectors::ThreadContext support for sampling allocated objects to also record the object's class.

Motivation:

This will enable us to provide the "Allocations by Allocated Type" feature in the Datadog UX.

Additional Notes:

N/A

How to test the change?

This change includes test coverage. Right now, there's no way to exercise this code other than through tests, because we haven't yet wired it up to trigger during actual Ruby VM allocations. This will come at a later date.

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.

**What does this PR do?**

This PR extends the `Profiling::Collectors::ThreadContext` support for
sampling allocated objects to also record the object's class.

**Motivation:**

This will enable us to provide the "Allocations by Allocated Type"
feature in the Datadog UX.

**Additional Notes:**

N/A

**How to test the change?**

This change includes test coverage. Right now, there's no way to
exercise this code other than through tests, because we haven't yet
wired it up to trigger during actual Ruby VM allocations. This will
come at a later date.
I suspect the allocation types may get a bit expensive on legacy Rubies,
and plan to experiment more about this at some point.

For now, to enable quick experiments, I've decided to add a flag to make
it easy to toggle this behavior. I haven't decided if we'll ever expose
this flag to customers or not.

If you're reading this in a few months' time, and this disabling
feature was never used, go ahead and delete it.
Hopefully we'll be able to deprecate Ruby 2.3 soon...
I guess we'd need to use pattern matching on Ruby 3 to do something
similar as Ruby 2 code, but we can't make that work on all Rubies we
support, so let's just do it the manual way.
After thinking about it for a bit longer, let's just treat immediates
the same way as every other type in the allocations code.

They are never allocated, but the extra codepaths/complexity to treat
them separately seems more brittle than just treating them as any other
type.
@ivoanjo ivoanjo requested a review from a team August 31, 2023 10:06
@github-actions github-actions bot added the profiling Involves Datadog profiling label Aug 31, 2023
@ivoanjo ivoanjo merged commit 9914e86 into master Sep 1, 2023
@ivoanjo ivoanjo deleted the ivoanjo/prof-7307-record-allocation-class branch September 1, 2023 08:10
@github-actions github-actions bot added this to the 1.15.0 milestone Sep 1, 2023
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.

2 participants