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.

Sign up for GitHub

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

Jump to bottom

[NO-TICKET] Raise benchmark default durations #3869

Closed
wants to merge 4 commits into from
Closed

[NO-TICKET] Raise benchmark default durations #3869

wants to merge 4 commits into from

Commits on Aug 28, 2024

  1. [NO-TICKET] Raise benchmark default durations 

    **What does this PR do?**

This PR goes through our existing benchmarks, and for those that used
a default duration of 10 or 12 seconds, raises the duration to 30s.

**Motivation:**

We've observed most of these benchmarks having flaky
regressions/improvements on PRs that shouldn't affect them.

I suspect that the running the benchmarks for such short time may
be contributing to flakiness. For instance, one of the benchmarks
that flakes the most often is `profiler_sample_serialize.rb`, and
concidentally that's a benchmark where each iteration does a lot
of work, and thus in a typical 10 second run we may only see around
70 iterations.

Hopefully by running the benchmarks for slightly longer we'll have
more consistent results.

**Additional Notes:**

There is a downside to this -- because our benchmarks are currently
executed sequentially in CI, this will make the benchmark run took
quite longer than it used to.

Hopefully this trade-off is reasonable; if not, we can re-evaluate.

**How to test the change?**

Check the latest benchmarks CI run, and confirm the tests are running
for 30 seconds, rather than 10/12.
    @ivoanjo
    ivoanjo committed Aug 28, 2024
    Configuration menu
    Copy the full SHA
    9c5916f View commit details
    Browse the repository at this point in the history

  2. Further tweak duration of profiler_hold_resume_interruptions benchmark 

    Seems to still be unstable in a benchmarkiing run:

```
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
Warming up --------------------------------------
profiler - hold / resume
                       203.634k i/100ms
Calculating -------------------------------------
profiler - hold / resume
                          2.037M (± 0.2%) i/s -     61.294M in  30.096368s
```

vs

```
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
Warming up --------------------------------------
profiler - hold / resume
                       210.917k i/100ms
Calculating -------------------------------------
profiler - hold / resume
                          2.099M (± 0.2%) i/s -     63.064M in  30.040197s
```

Let's see if as a last-ditch attempt, raising the duration a bit
more fixes it, or if we need to look into other options.
    @ivoanjo
    ivoanjo committed Aug 28, 2024
    Configuration menu
    Copy the full SHA
    01b37f8 View commit details
    Browse the repository at this point in the history

  3. Try raising tracing benchmark durations again 

    Still got an unstable result on the last run, let's see if 60 also
helps here or not...
    @ivoanjo
    ivoanjo committed Aug 28, 2024
    Configuration menu
    Copy the full SHA
    51d2271 View commit details
    Browse the repository at this point in the history

  4. Last ditch attempt: raise every benchmark to 60 seconds 

    I suspect this won't be enough to make the benchmarks consistent, but
let's try and see how it goes.
    @ivoanjo
    ivoanjo committed Aug 28, 2024
    Configuration menu
    Copy the full SHA
    ee157a1 View commit details
    Browse the repository at this point in the history