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-11394] Revert changes to heap profiling in PRs #4401, #4376, #4331 #4409

Merged
merged 3 commits into from
Feb 19, 2025

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Feb 19, 2025

What does this PR do?

This PR reverts PRs #4401, #4376, #4331 which all relate to the heap profiling feature.

Motivation:

I'm doing this to unblock master for the next release, as we've detected this feature may be producing incorrect profiles in some situation.

I'm still investigating what's going on here.

Change log entry

None. Please remember not to include in the changelog entries related to heap profiling, as they've been reverted

Additional Notes:

N/A

How to test the change?

The diff is a bit weird, since these PRs touched a bunch of things. I've validated it myself in an easier way: git diff v2.10.0 ext/ lib/datadog/profiling shows that no heap profiling changes are being shipped after this branch is applied.

…-profiler-preview"

This reverts commit 6f63c6d, reversing
changes made to 5e20f11.
…t-disabled-benchmark"

This reverts commit c6d1822, reversing
changes made to 744c421.
…ed-string-storage-try2"

This reverts commit 744c421, reversing
changes made to e9adecb.
@ivoanjo ivoanjo requested review from a team as code owners February 19, 2025 13:56
@github-actions github-actions bot added core Involves Datadog core libraries profiling Involves Datadog profiling labels Feb 19, 2025
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.70%. Comparing base (6fa4413) to head (ded9fcb).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4409      +/-   ##
==========================================
- Coverage   97.71%   97.70%   -0.01%     
==========================================
  Files        1366     1366              
  Lines       83397    83402       +5     
  Branches     4231     4231              
==========================================
- Hits        81491    81490       -1     
- Misses       1906     1912       +6     

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

@datadog-datadog-prod-us1
Copy link
Contributor

Datadog Report

Branch report: ivoanjo/prof-11394-revert-heap-profiling
Commit report: ded9fcb
Test service: dd-trace-rb

✅ 0 Failed, 20607 Passed, 1372 Skipped, 3m 21.55s Total Time

@pr-commenter
Copy link

pr-commenter bot commented Feb 19, 2025

Benchmarks

Benchmark execution time: 2025-02-19 14:15:52

Comparing candidate commit ded9fcb in PR branch ivoanjo/prof-11394-revert-heap-profiling with baseline commit 6fa4413 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 31 metrics, 2 unstable metrics.

@ivoanjo ivoanjo merged commit 30f6e21 into master Feb 19, 2025
497 checks passed
@ivoanjo ivoanjo deleted the ivoanjo/prof-11394-revert-heap-profiling branch February 19, 2025 14:18
@github-actions github-actions bot added this to the 2.11.0 milestone Feb 19, 2025
@ivoanjo ivoanjo removed the core Involves Datadog core libraries label Feb 25, 2025
ivoanjo added a commit that referenced this pull request Mar 5, 2025
…econd try

**What does this PR do?**

This reverts commit 569bf56.

We had previously reverted that commit in
#4376 but due to a bug
had to undo all the heap profiling-related changes in
#4409 .

After #4460 gets merged,
we can finally enable this benchmark again.

**Motivation:**

The benchmark was disabled for the reason mentioned in the comment.

**Additional Notes:**

I'm going to open this PR stacked on top of
#4460, but this PR should
only be merged after that other PR is merged to master.

**How to test the change?**

Validate this benchmark is now running in the benchmarking platform
results page.
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