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

.gitlab/scripts: remove BenchmarkConcurrentTracing from CI benchmarks #1845

Merged
merged 3 commits into from Jul 25, 2023

Conversation

knusbaum
Copy link
Contributor

@knusbaum knusbaum commented Mar 29, 2023

What does this PR do?

We replace BenchmarkConcurrentTracing with BenchmarkTracerAddSpans, which should provide more reliable benchmarks about the cost of starting spans.

Motivation

BenchmarkConcurrentTracing is not a reliable test and seems to mostly end up testing the contention characteristics of the system it's running on.

Describe how to test/QA your changes

Reviewer's Checklist

  • If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
  • Changed code has unit tests for its functionality.
  • If this interacts with the agent in a new way, a system test has been added.

BenchmarkConcurrentTracing is not a reliable test and seems to mostly end
up testing the contention characteristics of the system it's running on.
Instead we replace it with BenchmarkTracerAddSpans, which should provide
more reliable benchmarks about the cost of starting spans.
@knusbaum knusbaum requested a review from a team as a code owner March 29, 2023 20:21
@knusbaum knusbaum added this to the v1.50.0 milestone Mar 29, 2023
@pr-commenter
Copy link

pr-commenter bot commented Mar 29, 2023

Benchmarks

Benchmark execution time: 2023-07-25 16:31:41

Comparing candidate commit 2a1e577 in PR branch knusbaum/change-benchmarks with baseline commit 0373ddc in branch main.

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

@Hellzy Hellzy modified the milestones: v1.50.0, v1.51.0 Apr 26, 2023
@knusbaum knusbaum self-assigned this Jun 29, 2023
@knusbaum knusbaum requested a review from a team as a code owner July 25, 2023 16:20
@knusbaum knusbaum merged commit 7f5857b into main Jul 25, 2023
52 checks passed
@knusbaum knusbaum deleted the knusbaum/change-benchmarks branch July 25, 2023 16:32
katiehockman pushed a commit that referenced this pull request Jul 26, 2023
…#1845)

We replace BenchmarkConcurrentTracing with BenchmarkTracerAddSpans, which
should provide more reliable benchmarks about the cost of starting spans.

BenchmarkConcurrentTracing is not a reliable test and seems to mostly end
up testing the contention characteristics of the system it's running on.
@felixge
Copy link
Member

felixge commented Mar 1, 2024

Please see #2590 (comment) for the downsides of this PR. I'm considering to revert it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants