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.

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

[PROF-7307] Wire up allocation sampling into CpuAndWallTimeWorker #3103

Merged
merged 3 commits into from
Sep 5, 2023
Merged

[PROF-7307] Wire up allocation sampling into CpuAndWallTimeWorker #3103

merged 3 commits into from
Sep 5, 2023

Commits on Sep 1, 2023

  1. [PROF-7307] Wire up allocation sampling into CpuAndWallTimeWorker 

    **What does this PR do?**

In #2657 we added support for the `Collectors::ThreadContext` to
sample a given object allocation.

But until now, triggering these kinds of allocations was only done in
our test suite.

This PR adds the missing part: actually sampling objects as Ruby
allocates them.

In the `Collectors::CpuAndWallTimeWorker`, we already
had a `TracePoint` that is called on every new object allocation
(`RUBY_INTERNAL_EVENT_NEWOBJ`), but we were only using it to
count allocations.

This PR changes the `TracePoint` code to actually call into the
`Collectors::ThreadContext` and thus allow allocation sampling to
happen.

**Motivation:**

This PR is another step towards the allocation profiling feature.

**Additional Notes:**

It's not yet possible to turn on allocation profiling via configuration.

I am reserving that change for later. But it's easy to do:
change the `profiling/component.rb` to use `alloc_samples_enabled: true`
and `allocation_sample_every: 1`.

This will result in a big overhead impact to the application. I'll be
working on this in later PRs.

In particular, the sampling approach we're using
(`allocation_sample_every`), is only a placeholder; it will get
replaced before we get to beta. This is why I don't want to add it
as a real user-available setting -- because it's temporary.

**How to test the change?**

This change includes test coverage. You can also manually check it out,
by following the instructions in the "Additional Notes".

(There's a feature flag needed for this data to show up in the
Datadog UX).
    @ivoanjo
    ivoanjo committed Sep 1, 2023
    Configuration menu
    Copy the full SHA
    0128298 View commit details
    Browse the repository at this point in the history

  2. Fix crash when sampling weird internal VM objects 

    Today I learned that sometimes Ruby allocates objects without a class!

See notes in diff for more details.
    @ivoanjo
    ivoanjo committed Sep 1, 2023
    Configuration menu
    Copy the full SHA
    4a7115d View commit details
    Browse the repository at this point in the history

  3. Add pretty name for T_IMEMO VM objects 

    These show up from time to time and will be user-visible, so it's nice
to have a name for them.

We could consider hiding them, but my thinking is: what if there's
something in the user's code that causes a lot of them to be created?

That seems relevant to report on, even if they are VM objects, since
the user will probably want to change their code to avoid this.
    @ivoanjo
    ivoanjo committed Sep 1, 2023
    Configuration menu
    Copy the full SHA
    b045835 View commit details
    Browse the repository at this point in the history