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-11405] Revert "Temporary disable benchmark to make CI happy", second try #4461

Open
wants to merge 1 commit into
base: ivoanjo/prof-11394-heap-profiling-graduate-try2
Choose a base branch
from

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Mar 5, 2025

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.

Change log entry
None.

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.

…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.
@ivoanjo ivoanjo requested a review from a team as a code owner March 5, 2025 15:33
@ivoanjo ivoanjo added dev/internal Other internal work that does not need to be included in the changelog profiling Involves Datadog profiling labels Mar 5, 2025
@github-actions github-actions bot added the dev/testing Involves testing processes (e.g. RSpec) label Mar 5, 2025
@pr-commenter
Copy link

pr-commenter bot commented Mar 5, 2025

Benchmarks

Benchmark execution time: 2025-03-05 15:56:10

Comparing candidate commit f7a92e6 in PR branch ivoanjo/prof-11394-revert-disabled-benchmark-try2 with baseline commit f850521 in branch ivoanjo/prof-11394-heap-profiling-graduate-try2.

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

scenario:tracing - Propagation - Datadog

  • 🟩 throughput [+2910.574op/s; +2989.490op/s] or [+9.874%; +10.142%]

Copy link
Member

@Strech Strech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit confused, but 👍🏼

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.

2 participants