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-9476] Revert "Temporary disable benchmark to make CI happy" #4376

Merged
merged 2 commits into from
Feb 18, 2025

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Feb 13, 2025

What does this PR do?

This reverts commit 569bf56.

Motivation:

We temporarily disabled this benchmark in
#4331
for the reasons explained in the comment.

Once that PR is in master we can re-enable the benchmark.

Change log entry
None.

Additional Notes:

This sharp edge of our benchmarks setup is annoying, but I'll fight it another day...

How to test the change?

Validate this benchmark is now running in the benchmarking platform results page.

**What does this PR do?**

This reverts commit 569bf56.

**Motivation:**

We temporarily disabled this benchmark in
#4331
for the reasons explained in the comment.

Once that PR is in master we can re-enable the benchmark.

**Additional Notes:**

This sharp edge of our benchmarks setup is annoying, but I'll fight
it another day...

**How to test the change?**

Validate this benchmark is now running in the benchmarking
platform results page.
@ivoanjo ivoanjo added dev/testing Involves testing processes (e.g. RSpec) profiling Involves Datadog profiling dev/internal Other internal work that does not need to be included in the changelog labels Feb 13, 2025
@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Feb 13, 2025

Datadog Report

Branch report: ivoanjo/prof-9476-revert-disabled-benchmark
Commit report: 9f5f5fa
Test service: dd-trace-rb

✅ 0 Failed, 20589 Passed, 1463 Skipped, 3m 14.33s Total Time

@codecov-commenter
Copy link

codecov-commenter commented Feb 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.70%. Comparing base (e9adecb) to head (9f5f5fa).
Report is 40 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4376      +/-   ##
==========================================
- Coverage   97.71%   97.70%   -0.02%     
==========================================
  Files        1361     1361              
  Lines       83245    83239       -6     
  Branches     4226     4226              
==========================================
- Hits        81344    81328      -16     
- Misses       1901     1911      +10     

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

@pr-commenter
Copy link

pr-commenter bot commented Feb 13, 2025

Benchmarks

Benchmark execution time: 2025-02-18 13:48:09

Comparing candidate commit 9f5f5fa in PR branch ivoanjo/prof-9476-revert-disabled-benchmark with baseline commit 744c421 in branch master.

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

Base automatically changed from ivoanjo/prof-9476-managed-string-storage-try2 to master February 18, 2025 13:22
@ivoanjo ivoanjo marked this pull request as ready for review February 18, 2025 13:23
@ivoanjo ivoanjo requested a review from a team as a code owner February 18, 2025 13:23
@ivoanjo ivoanjo merged commit c6d1822 into master Feb 18, 2025
487 checks passed
@ivoanjo ivoanjo deleted the ivoanjo/prof-9476-revert-disabled-benchmark branch February 18, 2025 14:05
@github-actions github-actions bot added this to the 2.11.0 milestone Feb 18, 2025
ivoanjo added a commit that referenced this pull request Feb 19, 2025
…t-disabled-benchmark"

This reverts commit c6d1822, reversing
changes made to 744c421.
ivoanjo added a commit that referenced this pull request Feb 19, 2025
…profiling

[PROF-11394] Revert changes to heap profiling in PRs #4401, #4376, #4331
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
dev/internal Other internal work that does not need to be included in the changelog dev/testing Involves testing processes (e.g. RSpec) profiling Involves Datadog profiling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants