Remove SpanOperation.allocations#1891
Merged
Merged
Conversation
marcotc
marked this pull request as draft
February 8, 2022 19:14
marcotc
force-pushed
the
remove-event-mutex
branch
from
February 9, 2022 00:36
2a91615 to
2142840
Compare
marcotc
force-pushed
the
remove-span-allocations
branch
from
February 9, 2022 00:39
4cfb5ea to
af4a46d
Compare
marcotc
marked this pull request as ready for review
February 9, 2022 00:40
Codecov Report
@@ Coverage Diff @@
## 1.0 #1891 +/- ##
==========================================
- Coverage 98.23% 98.22% -0.02%
==========================================
Files 966 966
Lines 47931 47897 -34
==========================================
- Hits 47084 47045 -39
- Misses 847 852 +5
Continue to review full report at Codecov.
|
ivoanjo
approved these changes
Feb 9, 2022
ivoanjo
left a comment
Member
There was a problem hiding this comment.
Slightly-sadface-emoji lol.
I had recently added #1805 after discussing it with a customer, you make a good point that this value can be misleading on multi-threaded Ruby apps.
Anyway, we hope to provide a better alternative via profiling instead, so let's take the speed improvement :)
ivoanjo
added a commit
that referenced
this pull request
Feb 20, 2023
**What does this PR do?**:
This PR adds a new profiler public API:
`Datadog::Profiling.allocation_count`.
The public documentation for this API is as follows:
> Returns an ever-increasing counter of the number of allocations
> observed by the profiler in this thread.
>
> Note 1: This counter may not start from zero on new threads. It
> should only be used to measure how many
> allocations have happened between two calls to this API:
> ```ruby
> allocations_before = Datadog::Profiling.allocation_count
> do_some_work()
> allocations_after = Datadog::Profiling.allocation_count
> puts "Allocations during do_some_work: #{allocations_after - allocations_before}"
> ```
> (This is similar to some OS-based time representations.)
>
> Note 2: All fibers in the same thread will share the same counter
> values.
>
> Only available when the profiler is running, the new CPU Profiling
> 2.0 profiler is in use, and allocation-related
> features are not disabled via configuration.
> For instructions on enabling CPU Profiling 2.0 see the ddtrace
> release notes.
As long as CPU Profiling 2.0 is in use, this API is enabled by
default. To disable it, this PR adds a new setting:
```ruby
Datadog.configure do |c|
c.profiling.advanced.allocation_counting_enabled = # ...
end
```
**Motivation**:
This feature has long been something we want to provide with ddtrace,
see issues #2164 and #468, as well as PRs #1891, #1805, #597
As part of the ongoing work of enabling allocation profiling,
counting the number of allocations comes at a very cheap cost since
the profiler needs to have a `RUBY_INTERNAL_EVENT_NEWOBJ`
tracepoint anyway -- it's just a matter of also incrementing a
counter inside it.
**Additional Notes**:
Note that this does not yet change any user-visible feature for
ddtrace. I'm counting on @marcotc to pick up the task of using this
API to make some tracing magic :)
**How to test the change?**:
This change includes code coverage.
---
Fixes #2164
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The allocations metric on Span has been around for a long time, sitting unused, but only recently exposed as a metric.
Because we only collect global statistics, the number of allocations reported on each span can be grossly exaggerated. Besides, it has the highest cost of any operation in the
SpanOperationclass, affecting the critical path performance.Breaking change
allocationsmetric.