[NO-TICKET] Fix over-counting of the first heap/allocation sample at profiler startup#5881
Conversation
…ler startup **What does this PR do?** This PR lowers the allocation profiler's base sampling interval (`BASE_SAMPLING_INTERVAL` in the `DiscreteDynamicSampler`) from 50 to 1, so we start out sampling every allocation until the sampler has measured enough to back off on its own. The sampler primes itself on every reset so that the first allocation is always sampled. But the same `events_since_last_sample` counter that drives that decision is also reused as the sample's *weight* -- how many allocations the sample stands in for. With a base interval of 50, that first post-reset sample was therefore weighted as 50, even though only a single allocation had actually happened, overshooting the real count by ~49 phantom allocations. (Resets happen whenever the profiler starts or restarts) Starting at an interval of 1 makes the priming offset 0, so the first sample -- and every sample until the first readjustment -- is weighted as 1, exactly the allocations it represents. The overshoot is gone. **Motivation:** This improves accuracy of allocation and heap profiles from the very first sample, including on short-lived and forking processes. This is expected to address the issue we saw in https://github.com/DataDog/prof-correctness/actions/runs/27137527347 where a big object, combined with the high original weight can throw off short-running tests. **Change log entry** Yes. Fix over-counting of the first allocation sample at profiler startup **Additional Notes:** We actually started with 1 back when the dynamic allocation sampling feature was still in development and then raised it to 50 in commit 4a18b4d, "Improve tests and initial sampling state" (#3395) as a conservative initial guess -- at the time, the only thing bounding the initial "blind" window was a 1-second adjustment timer. What makes 1 safe now is the sample-count preemptive readjustment (`ADJUSTMENT_WINDOW_SAMPLES`), which was added afterwards in #3434. With it, even at interval 1 the initial "blind" window is bounded to at most 100 samples before we readjust based on observed overhead, so sampling everything at startup can no longer "run away" regardless of load. The impact is low in both directions: - For typical long-running processes the old over-count was a one-time +49 against a background of millions of allocations, i.e. negligible; the real win is for short or heavily-forking workloads. - The only downside of sampling everything at startup is a slightly noisier first rate estimate (the first readjustment runs over a shorter window). It errs on the safe side though: a fast startup produces a high estimated rate and therefore a larger interval, never an overhead blow-up. **How to test the change?** Added two unit tests to `discrete_dynamic_sampler_spec.rb`: one pinning the new starting interval, and one that drives the startup window to show we sample every event up to the 100-sample cap, readjust exactly there, and then back off. The existing sampler and allocation specs stay green.
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: fc6b7b4 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-06-09 17:49:38 Comparing candidate commit fc6b7b4 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 45 metrics, 1 unstable metrics.
|
What does this PR do?
This PR lowers the allocation profiler's base sampling interval (
BASE_SAMPLING_INTERVALin theDiscreteDynamicSampler) from 50 to 1, so we start out sampling every allocation until the sampler has measured enough to back off on its own.The sampler primes itself on every reset so that the first allocation is always sampled. But the same
events_since_last_samplecounter that drives that decision is also reused as the sample's weight -- how many allocations the sample stands in for. With a base interval of 50, that first post-reset sample was therefore weighted as 50, even though only a single allocation had actually happened, overshooting the real count by ~49 phantom allocations. (Resets happen whenever the profiler starts or restarts)Starting at an interval of 1 makes the priming offset 0, so the first sample -- and every sample until the first readjustment -- is weighted as 1, exactly the allocations it represents. The overshoot is gone.
Motivation:
This improves accuracy of allocation and heap profiles from the very first sample, including on short-lived and forking processes.
This is expected to address the issue we saw in
https://github.com/DataDog/prof-correctness/actions/runs/27137527347 where a big object, combined with the high original weight can throw off short-running tests.
Change log entry
Yes. Fix over-counting of the first allocation sample at profiler startup
Additional Notes:
We actually started with 1 back when the dynamic allocation sampling feature was still in development and then raised it to 50 in commit 4a18b4d, "Improve tests and initial sampling state" (#3395) as a conservative initial guess -- at the time, the only thing bounding the initial "blind" window was a 1-second adjustment timer.
What makes 1 safe now is the sample-count preemptive readjustment (
ADJUSTMENT_WINDOW_SAMPLES), which was added afterwards in #3434. With it, even at interval 1 the initial "blind" window is bounded to at most 100 samples before we readjust based on observed overhead, so sampling everything at startup can no longer "run away" regardless of load.The impact is low in both directions:
How to test the change?
Added two unit tests to
discrete_dynamic_sampler_spec.rb: one pinning the new starting interval, and one that drives the startup window to show we sample every event up to the 100-sample cap, readjust exactly there, and then back off. The existing sampler and allocation specs stay green.