From 591d6ad94ab45fc3d598f31c5cd7e0d5e8bcc280 Mon Sep 17 00:00:00 2001 From: Arnab Nandy Date: Fri, 26 Jun 2026 02:31:00 +0530 Subject: [PATCH 1/3] perf: optimize classic-only histogram observe path Signed-off-by: Arnab Nandy --- .../prometheus-metrics-core.txt | 2 + .../metrics/core/metrics/Histogram.java | 48 ++++++++++++++++- .../metrics/core/metrics/HistogramTest.java | 51 +++++++++++++++++++ 3 files changed, 99 insertions(+), 2 deletions(-) diff --git a/docs/apidiffs/current_vs_latest/prometheus-metrics-core.txt b/docs/apidiffs/current_vs_latest/prometheus-metrics-core.txt index ffb4a1d52..136f7f6f1 100644 --- a/docs/apidiffs/current_vs_latest/prometheus-metrics-core.txt +++ b/docs/apidiffs/current_vs_latest/prometheus-metrics-core.txt @@ -1,4 +1,6 @@ Comparing source compatibility of prometheus-metrics-core-1.8.1-SNAPSHOT.jar against prometheus-metrics-core-1.8.0.jar *** MODIFIED CLASS: PUBLIC io.prometheus.metrics.core.exemplars.ExemplarSampler (not serializable) === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 +*** MODIFIED CLASS: PUBLIC io.prometheus.metrics.core.metrics.Histogram$DataPoint (not serializable) + === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Histogram.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Histogram.java index 9a7f9b7c9..c6b3ee783 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Histogram.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Histogram.java @@ -205,6 +205,9 @@ public class DataPoint implements DistributionDataPoint { private final LongAdder nativeZeroCount = new LongAdder(); private final LongAdder count = new LongAdder(); private final DoubleAdder sum = new DoubleAdder(); + private final long[] classicOnlyBuckets; + private long classicOnlyCount; + private double classicOnlySum; private volatile int nativeSchema = nativeInitialSchema; // integer in [-4, 8] or CLASSIC_HISTOGRAM private volatile double nativeZeroThreshold = Histogram.this.nativeMinZeroThreshold; @@ -223,16 +226,27 @@ private DataPoint() { for (int i = 0; i < classicUpperBounds.length; i++) { classicBuckets[i] = new LongAdder(); } + classicOnlyBuckets = new long[classicUpperBounds.length]; maybeScheduleNextReset(); } @Override public double getSum() { + if (isClassicOnly()) { + synchronized (this) { + return classicOnlySum; + } + } return sum.sum(); } @Override public long getCount() { + if (isClassicOnly()) { + synchronized (this) { + return classicOnlyCount; + } + } return count.sum(); } @@ -242,7 +256,9 @@ public void observe(double value) { // See https://github.com/prometheus/client_golang/issues/1275 on ignoring NaN observations. return; } - if (!buffer.append(value)) { + if (isClassicOnly()) { + doObserveClassicOnly(value); + } else if (!buffer.append(value)) { doObserve(value, false); } if (exemplarSampler != null) { @@ -256,7 +272,9 @@ public void observeWithExemplar(double value, Labels labels) { // See https://github.com/prometheus/client_golang/issues/1275 on ignoring NaN observations. return; } - if (!buffer.append(value)) { + if (isClassicOnly()) { + doObserveClassicOnly(value); + } else if (!buffer.append(value)) { doObserve(value, false); } if (exemplarSampler != null) { @@ -264,6 +282,22 @@ public void observeWithExemplar(double value, Labels labels) { } } + private boolean isClassicOnly() { + return Histogram.this.nativeInitialSchema == CLASSIC_HISTOGRAM; + } + + private synchronized void doObserveClassicOnly(double value) { + for (int i = 0; i < classicUpperBounds.length; ++i) { + // The last bucket is +Inf, so we always increment. + if (value <= classicUpperBounds[i]) { + classicOnlyBuckets[i]++; + break; + } + } + classicOnlySum += value; + classicOnlyCount++; + } + private void doObserve(double value, boolean fromBuffer) { // classicUpperBounds is an empty array if this is a native histogram only. for (int i = 0; i < classicUpperBounds.length; ++i) { @@ -301,6 +335,16 @@ private void doObserve(double value, boolean fromBuffer) { private HistogramSnapshot.HistogramDataPointSnapshot collect(Labels labels) { Exemplars exemplars = exemplarSampler != null ? exemplarSampler.collect() : Exemplars.EMPTY; + if (isClassicOnly()) { + synchronized (this) { + return new HistogramSnapshot.HistogramDataPointSnapshot( + ClassicHistogramBuckets.of(classicUpperBounds, classicOnlyBuckets), + classicOnlySum, + labels, + exemplars, + createdTimeMillis); + } + } return buffer.run( expectedCount -> count.sum() == expectedCount, () -> { diff --git a/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/HistogramTest.java b/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/HistogramTest.java index 69518205d..408901cd8 100644 --- a/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/HistogramTest.java +++ b/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/HistogramTest.java @@ -1565,6 +1565,57 @@ void testObserveMultithreaded() assertThat(executor.awaitTermination(5, TimeUnit.SECONDS)).isTrue(); } + @Test + void testClassicOnlyObserveMultithreaded() + throws InterruptedException, ExecutionException, TimeoutException { + Histogram histogram = + Histogram.builder().name("test").classicOnly().labelNames("status").build(); + int nThreads = 8; + DistributionDataPoint obs = histogram.labelValues("200"); + ExecutorService executor = Executors.newFixedThreadPool(nThreads); + CompletionService> completionService = + new ExecutorCompletionService<>(executor); + CountDownLatch startSignal = new CountDownLatch(nThreads); + for (int t = 0; t < nThreads; t++) { + completionService.submit( + () -> { + List snapshots = new ArrayList<>(); + startSignal.countDown(); + startSignal.await(); + for (int i = 0; i < 10; i++) { + for (int j = 0; j < 1000; j++) { + obs.observe(1.1); + } + snapshots.add(histogram.collect()); + } + return snapshots; + }); + } + long maxCount = 0; + for (int i = 0; i < nThreads; i++) { + Future> future = completionService.take(); + List snapshots = future.get(5, TimeUnit.SECONDS); + long count = 0; + for (HistogramSnapshot snapshot : snapshots) { + assertThat(snapshot.getDataPoints().size()).isOne(); + HistogramSnapshot.HistogramDataPointSnapshot data = + snapshot.getDataPoints().stream().findFirst().orElseThrow(RuntimeException::new); + assertThat(data.getCount()).isGreaterThanOrEqualTo(count + 1000); + assertThat(data.getSum()).isCloseTo(data.getCount() * 1.1, offset(0.0000001)); + count = data.getCount(); + } + if (count > maxCount) { + maxCount = count; + } + } + assertThat(maxCount).isEqualTo(nThreads * 10_000L); + assertThat(obs.getCount()).isEqualTo(nThreads * 10_000L); + assertThat(obs.getSum()).isCloseTo(nThreads * 10_000L * 1.1, offset(0.0000001)); + assertThat(nThreads * 10_000).isEqualTo(getBucket(histogram, 2.5, "status", "200").getCount()); + executor.shutdown(); + assertThat(executor.awaitTermination(5, TimeUnit.SECONDS)).isTrue(); + } + @Test void testNativeResetDuration() { // Test that nativeResetDuration can be configured without error and the histogram From e53934404588df4892249f6659133e449b291977 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Thu, 2 Jul 2026 14:28:49 +0000 Subject: [PATCH 2/3] ci: add opt-in PR benchmark runs Signed-off-by: Gregor Zeitlinger --- .github/workflows/pr-benchmark-report.yml | 81 +++++++++++++++++++++++ .github/workflows/pr-benchmarks.yml | 76 +++++++++++++++++++++ 2 files changed, 157 insertions(+) create mode 100644 .github/workflows/pr-benchmark-report.yml create mode 100644 .github/workflows/pr-benchmarks.yml diff --git a/.github/workflows/pr-benchmark-report.yml b/.github/workflows/pr-benchmark-report.yml new file mode 100644 index 000000000..bcf5c01b1 --- /dev/null +++ b/.github/workflows/pr-benchmark-report.yml @@ -0,0 +1,81 @@ +--- +name: PR Benchmark Report + +on: + # zizmor: ignore[dangerous-triggers] -- this workflow never checks out or executes PR code; + # it only reads artifacts produced by the benchmark run and posts a PR comment. + workflow_run: + workflows: + - PR Benchmarks + types: + - completed + +permissions: {} + +jobs: + comment: + if: > + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.conclusion != 'cancelled' + runs-on: ubuntu-24.04 + permissions: + actions: read + contents: read + pull-requests: write + steps: + - name: Comment on PR with benchmark results + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + RUN_ID: ${{ github.event.workflow_run.id }} + RUN_URL: ${{ github.event.workflow_run.html_url }} + CONCLUSION: ${{ github.event.workflow_run.conclusion }} + run: | + PR_NUMBER=$(gh api "repos/${REPO}/actions/runs/${RUN_ID}" --jq '.pull_requests[0].number') + HEAD_SHA=$(gh api "repos/${REPO}/actions/runs/${RUN_ID}" --jq '.head_sha') + + COMMENT_ARTIFACT_ID=$(gh api "repos/${REPO}/actions/runs/${RUN_ID}/artifacts" \ + --jq '.artifacts[] | select(.name == "pr-benchmark-comment-pr-'"${PR_NUMBER}"'-'"${HEAD_SHA}"'") | .id' \ + | head -1) + + RESULTS_ARTIFACT_NAME="pr-benchmark-results-pr-${PR_NUMBER}-${HEAD_SHA}" + MARKER="" + + if [[ -n "${COMMENT_ARTIFACT_ID}" ]]; then + gh api \ + -H "Accept: application/octet-stream" \ + "repos/${REPO}/actions/artifacts/${COMMENT_ARTIFACT_ID}/zip" > /tmp/pr-benchmark-comment.zip + unzip -p /tmp/pr-benchmark-comment.zip pr-benchmark-comment.md > /tmp/pr-benchmark-comment.md + SUMMARY_BODY=$(cat /tmp/pr-benchmark-comment.md) + else + SUMMARY_BODY="_Benchmark summary artifact was not found; see the workflow run for details._" + fi + + if [[ "${CONCLUSION}" == "success" ]]; then + STATUS_LINE="Benchmark run succeeded for \`${HEAD_SHA}\`." + else + STATUS_LINE="Benchmark run finished with conclusion \`${CONCLUSION}\` for \`${HEAD_SHA}\`." + fi + + body=$(cat < Date: Fri, 3 Jul 2026 07:35:03 +0000 Subject: [PATCH 3/3] ci: move PR benchmark workflows to separate PR Signed-off-by: Gregor Zeitlinger --- .github/workflows/pr-benchmark-report.yml | 81 ----------------------- .github/workflows/pr-benchmarks.yml | 76 --------------------- 2 files changed, 157 deletions(-) delete mode 100644 .github/workflows/pr-benchmark-report.yml delete mode 100644 .github/workflows/pr-benchmarks.yml diff --git a/.github/workflows/pr-benchmark-report.yml b/.github/workflows/pr-benchmark-report.yml deleted file mode 100644 index bcf5c01b1..000000000 --- a/.github/workflows/pr-benchmark-report.yml +++ /dev/null @@ -1,81 +0,0 @@ ---- -name: PR Benchmark Report - -on: - # zizmor: ignore[dangerous-triggers] -- this workflow never checks out or executes PR code; - # it only reads artifacts produced by the benchmark run and posts a PR comment. - workflow_run: - workflows: - - PR Benchmarks - types: - - completed - -permissions: {} - -jobs: - comment: - if: > - github.event.workflow_run.event == 'pull_request' && - github.event.workflow_run.conclusion != 'cancelled' - runs-on: ubuntu-24.04 - permissions: - actions: read - contents: read - pull-requests: write - steps: - - name: Comment on PR with benchmark results - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - RUN_ID: ${{ github.event.workflow_run.id }} - RUN_URL: ${{ github.event.workflow_run.html_url }} - CONCLUSION: ${{ github.event.workflow_run.conclusion }} - run: | - PR_NUMBER=$(gh api "repos/${REPO}/actions/runs/${RUN_ID}" --jq '.pull_requests[0].number') - HEAD_SHA=$(gh api "repos/${REPO}/actions/runs/${RUN_ID}" --jq '.head_sha') - - COMMENT_ARTIFACT_ID=$(gh api "repos/${REPO}/actions/runs/${RUN_ID}/artifacts" \ - --jq '.artifacts[] | select(.name == "pr-benchmark-comment-pr-'"${PR_NUMBER}"'-'"${HEAD_SHA}"'") | .id' \ - | head -1) - - RESULTS_ARTIFACT_NAME="pr-benchmark-results-pr-${PR_NUMBER}-${HEAD_SHA}" - MARKER="" - - if [[ -n "${COMMENT_ARTIFACT_ID}" ]]; then - gh api \ - -H "Accept: application/octet-stream" \ - "repos/${REPO}/actions/artifacts/${COMMENT_ARTIFACT_ID}/zip" > /tmp/pr-benchmark-comment.zip - unzip -p /tmp/pr-benchmark-comment.zip pr-benchmark-comment.md > /tmp/pr-benchmark-comment.md - SUMMARY_BODY=$(cat /tmp/pr-benchmark-comment.md) - else - SUMMARY_BODY="_Benchmark summary artifact was not found; see the workflow run for details._" - fi - - if [[ "${CONCLUSION}" == "success" ]]; then - STATUS_LINE="Benchmark run succeeded for \`${HEAD_SHA}\`." - else - STATUS_LINE="Benchmark run finished with conclusion \`${CONCLUSION}\` for \`${HEAD_SHA}\`." - fi - - body=$(cat <