From 78d7a94e468a715c09f5a2983232aff5017a76e8 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Mon, 4 Nov 2024 20:26:59 +0100 Subject: [PATCH 1/9] add: linear search implementation (+ benchmarks) Signed-off-by: Ivan Goncharov --- prometheus/histogram.go | 38 ++++++++++++---- prometheus/histogram_test.go | 88 ++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 9 deletions(-) diff --git a/prometheus/histogram.go b/prometheus/histogram.go index 08bef3d87..c0b6ecd7b 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -858,15 +858,35 @@ func (h *histogram) Write(out *dto.Metric) error { // findBucket returns the index of the bucket for the provided value, or // len(h.upperBounds) for the +Inf bucket. func (h *histogram) findBucket(v float64) int { - // TODO(beorn7): For small numbers of buckets (<30), a linear search is - // slightly faster than the binary search. If we really care, we could - // switch from one search strategy to the other depending on the number - // of buckets. - // - // Microbenchmarks (BenchmarkHistogramNoLabels): - // 11 buckets: 38.3 ns/op linear - binary 48.7 ns/op - // 100 buckets: 78.1 ns/op linear - binary 54.9 ns/op - // 300 buckets: 154 ns/op linear - binary 61.6 ns/op + n := len(h.upperBounds) + if n == 0 { + return 0 + } + + // Early exit: if v is less than or equal to the first upper bound, return 0 + if v <= h.upperBounds[0] { + return 0 + } + + // Early exit: if v is greater than the last upper bound, return len(h.upperBounds) + if v > h.upperBounds[n-1] { + return n + } + + // For small arrays, use simple linear search + // "magic number" 35 is result of tests on couple different (AWS and baremetal) servers + // see more details here: https://github.com/prometheus/client_golang/pull/1662 + if n < 35 { + for i, bound := range h.upperBounds { + if v <= bound { + return i + } + } + // If v is greater than all upper bounds, return len(h.upperBounds) + return n + } + + // For larger arrays, use stdlib's binary search return sort.SearchFloat64s(h.upperBounds, v) } diff --git a/prometheus/histogram_test.go b/prometheus/histogram_test.go index c2a14ae72..9ed5971c6 100644 --- a/prometheus/histogram_test.go +++ b/prometheus/histogram_test.go @@ -1455,3 +1455,91 @@ func compareNativeExemplarValues(t *testing.T, exps []*dto.Exemplar, values []fl } } } + +var resultFindBucket int + +func benchmarkFindBucket(b *testing.B, l int) { + h := &histogram{upperBounds: make([]float64, l)} + for i := range h.upperBounds { + h.upperBounds[i] = float64(i) + } + v := float64(l / 2) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + resultFindBucket = h.findBucket(v) + } +} + +func BenchmarkFindBucketShort(b *testing.B) { + benchmarkFindBucket(b, 20) +} + +func BenchmarkFindBucketMid(b *testing.B) { + benchmarkFindBucket(b, 40) +} + +func BenchmarkFindBucketLarge(b *testing.B) { + benchmarkFindBucket(b, 100) +} + +func BenchmarkFindBucketHuge(b *testing.B) { + benchmarkFindBucket(b, 500) +} + +func BenchmarkFindBucketInf(b *testing.B) { + h := &histogram{upperBounds: make([]float64, 500)} + for i := range h.upperBounds { + h.upperBounds[i] = float64(i) + } + v := 1000.5 + + b.ResetTimer() + for i := 0; i < b.N; i++ { + resultFindBucket = h.findBucket(v) + } +} + +func BenchmarkFindBucketLow(b *testing.B) { + h := &histogram{upperBounds: make([]float64, 500)} + for i := range h.upperBounds { + h.upperBounds[i] = float64(i) + } + v := -1.1 + + b.ResetTimer() + for i := 0; i < b.N; i++ { + resultFindBucket = h.findBucket(v) + } +} + +func TestFindBucket(t *testing.T) { + smallHistogram := &histogram{upperBounds: []float64{1, 2, 3, 4, 5}} + largeHistogram := &histogram{upperBounds: make([]float64, 50)} + for i := range largeHistogram.upperBounds { + largeHistogram.upperBounds[i] = float64(i) + } + + tests := []struct { + h *histogram + v float64 + expected int + }{ + {smallHistogram, -1, 0}, + {smallHistogram, 0.5, 0}, + {smallHistogram, 2.5, 2}, + {smallHistogram, 5.5, 5}, + {largeHistogram, -1, 0}, + {largeHistogram, 25.5, 26}, + {largeHistogram, 49.5, 50}, + {largeHistogram, 50.5, 50}, + {largeHistogram, 5000.5, 50}, + } + + for _, tt := range tests { + result := tt.h.findBucket(tt.v) + if result != tt.expected { + t.Errorf("findBucket(%v) = %d; expected %d", tt.v, result, tt.expected) + } + } +} From 8aea698d6a280781f889d7d4c9aaad7ddbc61a34 Mon Sep 17 00:00:00 2001 From: prombot Date: Tue, 5 Nov 2024 17:47:44 +0000 Subject: [PATCH 2/9] Update common Prometheus files Signed-off-by: prombot --- .github/workflows/golangci-lint.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 1c099932b..7183091ac 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -26,14 +26,14 @@ jobs: - name: Checkout repository uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 - name: Install Go - uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2 + uses: actions/setup-go@41dfa10bad2bb2ae585af6ee5bb4d7d973ad74ed # v5.1.0 with: go-version: 1.23.x - name: Install snmp_exporter/generator dependencies run: sudo apt-get update && sudo apt-get -y install libsnmp-dev if: github.repository == 'prometheus/snmp_exporter' - name: Lint - uses: golangci/golangci-lint-action@aaa42aa0628b4ae2578232a66b541047968fac86 # v6.1.0 + uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 # v6.1.1 with: args: --verbose version: v1.60.2 From 3c21cc0ecf6ece86fa921a829762a735158140a8 Mon Sep 17 00:00:00 2001 From: Matt Harbison Date: Thu, 7 Nov 2024 17:22:22 -0500 Subject: [PATCH 3/9] process_collector: avoid a compiler warning on macOS (fixes #1660) The header has a warning when included, with no way to shut it off, and no alternative to obtain these symbols. They're technically architecture specific values, but they aren't different between amd64 and arm64, so combine the definitions. Signed-off-by: Matt Harbison --- prometheus/process_collector_cgo_darwin.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/prometheus/process_collector_cgo_darwin.c b/prometheus/process_collector_cgo_darwin.c index f83cf02b8..1554f674d 100644 --- a/prometheus/process_collector_cgo_darwin.c +++ b/prometheus/process_collector_cgo_darwin.c @@ -15,12 +15,16 @@ #include #include -// Compiler warns that shared_memory_server.h is deprecated, use this instead. -// But this doesn't define SHARED_DATA_REGION_SIZE or SHARED_TEXT_REGION_SIZE. -//#include -#include #include +// The compiler warns that mach/shared_memory_server.h is deprecated, and to use +// mach/shared_region.h instead. But that doesn't define +// SHARED_DATA_REGION_SIZE or SHARED_TEXT_REGION_SIZE, so redefine them here and +// avoid a warning message when running tests. +#define GLOBAL_SHARED_TEXT_SEGMENT 0x90000000U +#define SHARED_DATA_REGION_SIZE 0x10000000 +#define SHARED_TEXT_REGION_SIZE 0x10000000 + int get_memory_info(unsigned long long *rss, unsigned long long *vsize) { From abfb25769fe06ded988fd5677896d3bb256a2d60 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 8 Nov 2024 08:42:37 +0000 Subject: [PATCH 4/9] build(deps): bump the github-actions group across 1 directory with 3 updates Bumps the github-actions group with 3 updates in the / directory: [actions/checkout](https://github.com/actions/checkout), [github/codeql-action](https://github.com/github/codeql-action) and [dagger/dagger-for-github](https://github.com/dagger/dagger-for-github). Updates `actions/checkout` from 4.2.0 to 4.2.2 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/d632683dd7b4114ad314bca15554477dd762a938...11bd71901bbe5b1630ceea73d27597364c9af683) Updates `github/codeql-action` from 3.26.10 to 3.27.0 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/e2b3eafc8d227b0241d48be5f425d47c2d750a13...662472033e021d55d94146f66f6058822b0b39fd) Updates `dagger/dagger-for-github` from 6.11.0 to 6.14.0 - [Release notes](https://github.com/dagger/dagger-for-github/releases) - [Commits](https://github.com/dagger/dagger-for-github/compare/fc945fa66fc7bfa72bc80f85b1a1ef4bd1d30cbb...ad6a4e308a42fb2cf9be8b060f9aba9d57d4c9aa) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions - dependency-name: dagger/dagger-for-github dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions ... Signed-off-by: dependabot[bot] --- .github/workflows/codeql-analysis.yml | 8 ++++---- .github/workflows/container_description.yml | 4 ++-- .github/workflows/dagger-golangci-lint.yml | 4 ++-- .github/workflows/go.yml | 8 ++++---- .github/workflows/golangci-lint.yml | 2 +- .github/workflows/update-go-versions.yml | 2 +- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 5f017fcbc..8e1e7771d 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -46,11 +46,11 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@e2b3eafc8d227b0241d48be5f425d47c2d750a13 # v3.26.10 + uses: github/codeql-action/init@662472033e021d55d94146f66f6058822b0b39fd # v3.27.0 with: languages: ${{ matrix.language }} # If you wish to specify custom queries, you can do so here or in a config file. @@ -61,7 +61,7 @@ jobs: # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). # If this step fails, then you should remove it and run the build manually (see below) - name: Autobuild - uses: github/codeql-action/autobuild@e2b3eafc8d227b0241d48be5f425d47c2d750a13 # v3.26.10 + uses: github/codeql-action/autobuild@662472033e021d55d94146f66f6058822b0b39fd # v3.27.0 # ℹī¸ Command-line programs to run using the OS shell. # 📚 https://git.io/JvXDl @@ -75,4 +75,4 @@ jobs: # make release - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@e2b3eafc8d227b0241d48be5f425d47c2d750a13 # v3.26.10 + uses: github/codeql-action/analyze@662472033e021d55d94146f66f6058822b0b39fd # v3.27.0 diff --git a/.github/workflows/container_description.yml b/.github/workflows/container_description.yml index 144859486..dcca16ff3 100644 --- a/.github/workflows/container_description.yml +++ b/.github/workflows/container_description.yml @@ -18,7 +18,7 @@ jobs: if: github.repository_owner == 'prometheus' || github.repository_owner == 'prometheus-community' # Don't run this workflow on forks. steps: - name: git checkout - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Set docker hub repo name run: echo "DOCKER_REPO_NAME=$(make docker-repo-name)" >> $GITHUB_ENV - name: Push README to Dockerhub @@ -40,7 +40,7 @@ jobs: if: github.repository_owner == 'prometheus' || github.repository_owner == 'prometheus-community' # Don't run this workflow on forks. steps: - name: git checkout - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Set quay.io org name run: echo "DOCKER_REPO=$(echo quay.io/${GITHUB_REPOSITORY_OWNER} | tr -d '-')" >> $GITHUB_ENV - name: Set quay.io repo name diff --git a/.github/workflows/dagger-golangci-lint.yml b/.github/workflows/dagger-golangci-lint.yml index 1e50b91f0..4dc3340f3 100644 --- a/.github/workflows/dagger-golangci-lint.yml +++ b/.github/workflows/dagger-golangci-lint.yml @@ -23,9 +23,9 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Lint - uses: dagger/dagger-for-github@fc945fa66fc7bfa72bc80f85b1a1ef4bd1d30cbb # v6.11.0 + uses: dagger/dagger-for-github@ad6a4e308a42fb2cf9be8b060f9aba9d57d4c9aa # v6.14.0 with: version: "latest" verb: call diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index d2cc68930..4d21600db 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -24,7 +24,7 @@ jobs: supported_versions: ${{ steps.matrix.outputs.supported_versions }} steps: - name: Checkout code - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Read supported_go_versions.txt id: matrix run: | @@ -43,17 +43,17 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Run tests and check license - uses: dagger/dagger-for-github@fc945fa66fc7bfa72bc80f85b1a1ef4bd1d30cbb # v6.11.0 + uses: dagger/dagger-for-github@ad6a4e308a42fb2cf9be8b060f9aba9d57d4c9aa # v6.14.0 with: version: "latest" verb: call args: -vvv --src . make --go-version ${{matrix.go_version}} --args 'check_license test' - name: Run style and unused - uses: dagger/dagger-for-github@fc945fa66fc7bfa72bc80f85b1a1ef4bd1d30cbb # v6.11.0 + uses: dagger/dagger-for-github@ad6a4e308a42fb2cf9be8b060f9aba9d57d4c9aa # v6.14.0 if: ${{ matrix.go_version == '1.21' }} with: version: "latest" diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 7183091ac..7af9bba77 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -24,7 +24,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Install Go uses: actions/setup-go@41dfa10bad2bb2ae585af6ee5bb4d7d973ad74ed # v5.1.0 with: diff --git a/.github/workflows/update-go-versions.yml b/.github/workflows/update-go-versions.yml index 32b7fab82..cd0c53a99 100644 --- a/.github/workflows/update-go-versions.yml +++ b/.github/workflows/update-go-versions.yml @@ -13,7 +13,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Execute bash script run: bash update-go-version.bash From fcfad5c0b904b6213504116af3f2d78e9f0e0454 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Fri, 8 Nov 2024 09:54:31 +0100 Subject: [PATCH 5/9] [chore]: enable perfsprint linter (#1676) --- .golangci.yml | 12 ++++ api/prometheus/v1/api_test.go | 68 +++++++++---------- examples/exemplars/main.go | 4 +- examples/random/main.go | 4 +- prometheus/counter_test.go | 5 +- prometheus/internal/difflib.go | 3 +- prometheus/process_collector_cgo_darwin.go | 4 +- prometheus/promhttp/http_test.go | 2 +- prometheus/registry_test.go | 3 +- prometheus/testutil/promlint/promlint_test.go | 5 +- .../validations/duplicate_validations.go | 4 +- prometheus/value_test.go | 3 +- prometheus/vec_test.go | 5 +- 13 files changed, 67 insertions(+), 55 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d2580cccc..fb5d208eb 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -25,6 +25,7 @@ linters: - ineffassign - misspell - nolintlint + - perfsprint - predeclared - revive - staticcheck @@ -66,6 +67,17 @@ linters-settings: local-prefixes: github.com/prometheus/client_golang gofumpt: extra-rules: true + perfsprint: + # Optimizes even if it requires an int or uint type cast. + int-conversion: true + # Optimizes into `err.Error()` even if it is only equivalent for non-nil errors. + err-error: true + # Optimizes `fmt.Errorf`. + errorf: true + # Optimizes `fmt.Sprintf` with only one argument. + sprintf1: true + # Optimizes into strings concatenation. + strconcat: true revive: rules: # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unused-parameter diff --git a/api/prometheus/v1/api_test.go b/api/prometheus/v1/api_test.go index 01ddf45ca..bacf6a096 100644 --- a/api/prometheus/v1/api_test.go +++ b/api/prometheus/v1/api_test.go @@ -16,13 +16,13 @@ package v1 import ( "context" "errors" - "fmt" "io" "math" "net/http" "net/http/httptest" "net/url" "reflect" + "strconv" "strings" "testing" "time" @@ -260,7 +260,7 @@ func TestAPIs(t *testing.T) { }, { do: doQuery("2", testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/query", @@ -336,7 +336,7 @@ func TestAPIs(t *testing.T) { End: testTime, Step: 1 * time.Minute, }, WithTimeout(5*time.Second)), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/query_range", @@ -361,14 +361,14 @@ func TestAPIs(t *testing.T) { { do: doLabelNames(nil, testTime.Add(-100*time.Hour), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/labels", err: errors.New("some error"), }, { do: doLabelNames(nil, testTime.Add(-100*time.Hour), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), inWarnings: []string{"a"}, reqMethod: "POST", reqPath: "/api/v1/labels", @@ -400,14 +400,14 @@ func TestAPIs(t *testing.T) { { do: doLabelValues(nil, "mylabel", testTime.Add(-100*time.Hour), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "GET", reqPath: "/api/v1/label/mylabel/values", err: errors.New("some error"), }, { do: doLabelValues(nil, "mylabel", testTime.Add(-100*time.Hour), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), inWarnings: []string{"a"}, reqMethod: "GET", reqPath: "/api/v1/label/mylabel/values", @@ -464,7 +464,7 @@ func TestAPIs(t *testing.T) { { do: doSeries("up", testTime.Add(-time.Minute), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/series", err: errors.New("some error"), @@ -472,7 +472,7 @@ func TestAPIs(t *testing.T) { // Series with error and warning. { do: doSeries("up", testTime.Add(-time.Minute), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), inWarnings: []string{"a"}, reqMethod: "POST", reqPath: "/api/v1/series", @@ -493,7 +493,7 @@ func TestAPIs(t *testing.T) { { do: doSnapshot(true), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/admin/tsdb/snapshot", err: errors.New("some error"), @@ -507,7 +507,7 @@ func TestAPIs(t *testing.T) { { do: doCleanTombstones(), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/admin/tsdb/clean_tombstones", err: errors.New("some error"), @@ -528,7 +528,7 @@ func TestAPIs(t *testing.T) { { do: doDeleteSeries("up", testTime.Add(-time.Minute), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/admin/tsdb/delete_series", err: errors.New("some error"), @@ -550,8 +550,8 @@ func TestAPIs(t *testing.T) { do: doConfig(), reqMethod: "GET", reqPath: "/api/v1/status/config", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -578,16 +578,16 @@ func TestAPIs(t *testing.T) { do: doFlags(), reqMethod: "GET", reqPath: "/api/v1/status/flags", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { do: doBuildinfo(), reqMethod: "GET", reqPath: "/api/v1/status/buildinfo", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -616,8 +616,8 @@ func TestAPIs(t *testing.T) { do: doRuntimeinfo(), reqMethod: "GET", reqPath: "/api/v1/status/runtimeinfo", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -684,8 +684,8 @@ func TestAPIs(t *testing.T) { do: doAlertManagers(), reqMethod: "GET", reqPath: "/api/v1/alertmanagers", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -891,8 +891,8 @@ func TestAPIs(t *testing.T) { do: doRules(), reqMethod: "GET", reqPath: "/api/v1/rules", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -971,8 +971,8 @@ func TestAPIs(t *testing.T) { do: doTargets(), reqMethod: "GET", reqPath: "/api/v1/targets", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -1005,7 +1005,7 @@ func TestAPIs(t *testing.T) { { do: doTargetsMetadata("{job=\"prometheus\"}", "go_goroutines", "1"), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "GET", reqPath: "/api/v1/targets/metadata", err: errors.New("some error"), @@ -1037,7 +1037,7 @@ func TestAPIs(t *testing.T) { { do: doMetadata("", "1"), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "GET", reqPath: "/api/v1/metadata", err: errors.New("some error"), @@ -1047,8 +1047,8 @@ func TestAPIs(t *testing.T) { do: doTSDB(), reqMethod: "GET", reqPath: "/api/v1/status/tsdb", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -1127,8 +1127,8 @@ func TestAPIs(t *testing.T) { do: doWalReply(), reqMethod: "GET", reqPath: "/api/v1/status/walreplay", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -1212,7 +1212,7 @@ func TestAPIs(t *testing.T) { tests = append(tests, queryTests...) for i, test := range tests { - t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + t.Run(strconv.Itoa(i), func(t *testing.T) { tc.curTest = test res, warnings, err := test.do() @@ -1430,7 +1430,7 @@ func TestAPIClientDo(t *testing.T) { } for i, test := range tests { - t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + t.Run(strconv.Itoa(i), func(t *testing.T) { tc.ch <- test _, body, warnings, err := client.Do(context.Background(), tc.req) diff --git a/examples/exemplars/main.go b/examples/exemplars/main.go index 798c2dfd0..618224a7b 100644 --- a/examples/exemplars/main.go +++ b/examples/exemplars/main.go @@ -17,10 +17,10 @@ package main import ( - "fmt" "log" "math/rand" "net/http" + "strconv" "time" "github.com/prometheus/client_golang/prometheus" @@ -50,7 +50,7 @@ func main() { // Record fictional latency. now := time.Now() requestDurations.(prometheus.ExemplarObserver).ObserveWithExemplar( - time.Since(now).Seconds(), prometheus.Labels{"dummyID": fmt.Sprint(rand.Intn(100000))}, + time.Since(now).Seconds(), prometheus.Labels{"dummyID": strconv.Itoa(rand.Intn(100000))}, ) time.Sleep(600 * time.Millisecond) } diff --git a/examples/random/main.go b/examples/random/main.go index b4f6280c7..9192e94c2 100644 --- a/examples/random/main.go +++ b/examples/random/main.go @@ -18,11 +18,11 @@ package main import ( "flag" - "fmt" "log" "math" "math/rand" "net/http" + "strconv" "time" "github.com/prometheus/client_golang/prometheus" @@ -116,7 +116,7 @@ func main() { // the ExemplarObserver interface and thus don't need to // check the outcome of the type assertion. m.rpcDurationsHistogram.(prometheus.ExemplarObserver).ObserveWithExemplar( - v, prometheus.Labels{"dummyID": fmt.Sprint(rand.Intn(100000))}, + v, prometheus.Labels{"dummyID": strconv.Itoa(rand.Intn(100000))}, ) time.Sleep(time.Duration(75*oscillationFactor()) * time.Millisecond) } diff --git a/prometheus/counter_test.go b/prometheus/counter_test.go index 3686199b5..29d94bf0a 100644 --- a/prometheus/counter_test.go +++ b/prometheus/counter_test.go @@ -14,7 +14,6 @@ package prometheus import ( - "fmt" "math" "strings" "testing" @@ -120,10 +119,10 @@ func TestCounterVecGetMetricWithInvalidLabelValues(t *testing.T) { expectPanic(t, func() { counterVec.WithLabelValues(labelValues...) - }, fmt.Sprintf("WithLabelValues: expected panic because: %s", test.desc)) + }, "WithLabelValues: expected panic because: "+test.desc) expectPanic(t, func() { counterVec.With(test.labels) - }, fmt.Sprintf("WithLabelValues: expected panic because: %s", test.desc)) + }, "WithLabelValues: expected panic because: "+test.desc) if _, err := counterVec.GetMetricWithLabelValues(labelValues...); err == nil { t.Errorf("GetMetricWithLabelValues: expected error because: %s", test.desc) diff --git a/prometheus/internal/difflib.go b/prometheus/internal/difflib.go index ba09bddb4..8b016355a 100644 --- a/prometheus/internal/difflib.go +++ b/prometheus/internal/difflib.go @@ -22,6 +22,7 @@ import ( "bytes" "fmt" "io" + "strconv" "strings" ) @@ -524,7 +525,7 @@ func formatRangeUnified(start, stop int) string { beginning := start + 1 // lines start numbering with one length := stop - start if length == 1 { - return fmt.Sprintf("%d", beginning) + return strconv.Itoa(beginning) } if length == 0 { beginning-- // empty ranges begin at line just before the range diff --git a/prometheus/process_collector_cgo_darwin.go b/prometheus/process_collector_cgo_darwin.go index 6f48e5845..fc4c24713 100644 --- a/prometheus/process_collector_cgo_darwin.go +++ b/prometheus/process_collector_cgo_darwin.go @@ -22,9 +22,7 @@ import "C" import "fmt" func getMemory() (*memoryInfo, error) { - var ( - rss, vsize C.ulonglong - ) + var rss, vsize C.ulonglong if err := C.get_memory_info(&rss, &vsize); err != 0 { return nil, fmt.Errorf("task_info() failed with 0x%x", int(err)) diff --git a/prometheus/promhttp/http_test.go b/prometheus/promhttp/http_test.go index 3ad2d1da8..c3a433c1c 100644 --- a/prometheus/promhttp/http_test.go +++ b/prometheus/promhttp/http_test.go @@ -98,7 +98,7 @@ func readCompressedBody(r io.Reader, comp Compression) (string, error) { got, err := io.ReadAll(reader) return string(got), err } - return "", fmt.Errorf("Unsupported compression") + return "", errors.New("Unsupported compression") } func TestHandlerErrorHandling(t *testing.T) { diff --git a/prometheus/registry_test.go b/prometheus/registry_test.go index 35a74d282..f6bca22b7 100644 --- a/prometheus/registry_test.go +++ b/prometheus/registry_test.go @@ -27,6 +27,7 @@ import ( "net/http" "net/http/httptest" "os" + "strconv" "sync" "testing" "time" @@ -1282,7 +1283,7 @@ func ExampleRegistry_grouping() { ConstLabels: prometheus.Labels{ // Generate a label unique to this worker so its metric doesn't // collide with the metrics from other workers. - "worker_id": fmt.Sprintf("%d", workerID), + "worker_id": strconv.Itoa(workerID), }, }) workerReg.MustRegister(workTime) diff --git a/prometheus/testutil/promlint/promlint_test.go b/prometheus/testutil/promlint/promlint_test.go index c60507c19..c6a4e27a7 100644 --- a/prometheus/testutil/promlint/promlint_test.go +++ b/prometheus/testutil/promlint/promlint_test.go @@ -14,6 +14,7 @@ package promlint_test import ( + "errors" "fmt" "reflect" "strings" @@ -733,7 +734,7 @@ request_duration_seconds{httpService="foo"} 10 func TestLintUnitAbbreviations(t *testing.T) { genTest := func(n string) test { return test{ - name: fmt.Sprintf("%s with abbreviated unit", n), + name: n + " with abbreviated unit", in: fmt.Sprintf(` # HELP %s Test metric. # TYPE %s gauge @@ -820,7 +821,7 @@ mc_something_total 10 prefixValidation := func(mf *dto.MetricFamily) []error { if !strings.HasPrefix(mf.GetName(), "memcached_") { - return []error{fmt.Errorf("expected metric name to start with 'memcached_'")} + return []error{errors.New("expected metric name to start with 'memcached_'")} } return nil } diff --git a/prometheus/testutil/promlint/validations/duplicate_validations.go b/prometheus/testutil/promlint/validations/duplicate_validations.go index fdc1e6239..68645ed0a 100644 --- a/prometheus/testutil/promlint/validations/duplicate_validations.go +++ b/prometheus/testutil/promlint/validations/duplicate_validations.go @@ -14,7 +14,7 @@ package validations import ( - "fmt" + "errors" "reflect" dto "github.com/prometheus/client_model/go" @@ -27,7 +27,7 @@ func LintDuplicateMetric(mf *dto.MetricFamily) []error { for i, m := range mf.Metric { for _, k := range mf.Metric[i+1:] { if reflect.DeepEqual(m.Label, k.Label) { - problems = append(problems, fmt.Errorf("metric not unique")) + problems = append(problems, errors.New("metric not unique")) break } } diff --git a/prometheus/value_test.go b/prometheus/value_test.go index 004c3bb35..23da6b217 100644 --- a/prometheus/value_test.go +++ b/prometheus/value_test.go @@ -14,7 +14,6 @@ package prometheus import ( - "fmt" "testing" "time" @@ -51,7 +50,7 @@ func TestNewConstMetricInvalidLabelValues(t *testing.T) { expectPanic(t, func() { MustNewConstMetric(metricDesc, CounterValue, 0.3, "\xFF") - }, fmt.Sprintf("WithLabelValues: expected panic because: %s", test.desc)) + }, "WithLabelValues: expected panic because: "+test.desc) if _, err := NewConstMetric(metricDesc, CounterValue, 0.3, "\xFF"); err == nil { t.Errorf("NewConstMetric: expected error because: %s", test.desc) diff --git a/prometheus/vec_test.go b/prometheus/vec_test.go index 89bd85429..fdd3abc36 100644 --- a/prometheus/vec_test.go +++ b/prometheus/vec_test.go @@ -16,6 +16,7 @@ package prometheus import ( "fmt" "reflect" + "strconv" "testing" dto "github.com/prometheus/client_model/go" @@ -291,7 +292,7 @@ func testMetricVec(t *testing.T, vec *GaugeVec) { expected := map[[2]string]int{} for i := 0; i < 1000; i++ { - pair[0], pair[1] = fmt.Sprint(i%4), fmt.Sprint(i%5) // Varying combinations multiples. + pair[0], pair[1] = strconv.Itoa(i%4), strconv.Itoa(i%5) // Varying combinations multiples. expected[pair]++ vec.WithLabelValues(pair[0], pair[1]).Inc() @@ -363,7 +364,7 @@ func testConstrainedMetricVec(t *testing.T, vec *GaugeVec, constrain func(string expected := map[[2]string]int{} for i := 0; i < 1000; i++ { - pair[0], pair[1] = fmt.Sprint(i%4), fmt.Sprint(i%5) // Varying combinations multiples. + pair[0], pair[1] = strconv.Itoa(i%4), strconv.Itoa(i%5) // Varying combinations multiples. expected[[2]string{pair[0], constrain(pair[1])}]++ vec.WithLabelValues(pair[0], pair[1]).Inc() From 400ee29a10f94e342dfca04783b3c2fc53a9021b Mon Sep 17 00:00:00 2001 From: PrometheusBot Date: Mon, 11 Nov 2024 15:09:29 +0100 Subject: [PATCH 6/9] Update common Prometheus files (#1679) Signed-off-by: prombot --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 7af9bba77..305146993 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -36,4 +36,4 @@ jobs: uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 # v6.1.1 with: args: --verbose - version: v1.60.2 + version: v1.61.0 From bab92a7743e22107b825cbba25cbbcb0a5e25b10 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 11 Nov 2024 15:19:33 +0100 Subject: [PATCH 7/9] chore: enable usestdlibvars linter (#1680) Signed-off-by: Matthieu MOREL --- .golangci.yml | 1 + prometheus/promhttp/http_test.go | 14 +++++++------- prometheus/promhttp/instrument_client_test.go | 4 ++-- prometheus/promhttp/instrument_server_test.go | 4 ++-- prometheus/registry_test.go | 2 +- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index fb5d208eb..f5a7f9dac 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -31,6 +31,7 @@ linters: - staticcheck - unconvert - unused + - usestdlibvars - wastedassign issues: diff --git a/prometheus/promhttp/http_test.go b/prometheus/promhttp/http_test.go index c3a433c1c..a763da469 100644 --- a/prometheus/promhttp/http_test.go +++ b/prometheus/promhttp/http_test.go @@ -131,7 +131,7 @@ func TestHandlerErrorHandling(t *testing.T) { logger := log.New(logBuf, "", 0) writer := httptest.NewRecorder() - request, _ := http.NewRequest("GET", "/", nil) + request, _ := http.NewRequest(http.MethodGet, "/", nil) request.Header.Add("Accept", "test/plain") mReg := &mockTransactionGatherer{g: reg} @@ -252,7 +252,7 @@ func TestInstrumentMetricHandler(t *testing.T) { // Do it again to test idempotency. InstrumentMetricHandler(reg, HandlerForTransactional(mReg, HandlerOpts{})) writer := httptest.NewRecorder() - request, _ := http.NewRequest("GET", "/", nil) + request, _ := http.NewRequest(http.MethodGet, "/", nil) request.Header.Add(acceptHeader, acceptTextPlain) handler.ServeHTTP(writer, request) @@ -311,7 +311,7 @@ func TestHandlerMaxRequestsInFlight(t *testing.T) { w1 := httptest.NewRecorder() w2 := httptest.NewRecorder() w3 := httptest.NewRecorder() - request, _ := http.NewRequest("GET", "/", nil) + request, _ := http.NewRequest(http.MethodGet, "/", nil) request.Header.Add(acceptHeader, acceptTextPlain) c := blockingCollector{Block: make(chan struct{}), CollectStarted: make(chan struct{}, 1)} @@ -348,7 +348,7 @@ func TestHandlerTimeout(t *testing.T) { handler := HandlerFor(reg, HandlerOpts{Timeout: time.Millisecond}) w := httptest.NewRecorder() - request, _ := http.NewRequest("GET", "/", nil) + request, _ := http.NewRequest(http.MethodGet, "/", nil) request.Header.Add("Accept", "test/plain") c := blockingCollector{Block: make(chan struct{}), CollectStarted: make(chan struct{}, 1)} @@ -372,7 +372,7 @@ func TestInstrumentMetricHandlerWithCompression(t *testing.T) { handler := InstrumentMetricHandler(reg, HandlerForTransactional(mReg, HandlerOpts{DisableCompression: false})) compression := Zstd writer := httptest.NewRecorder() - request, _ := http.NewRequest("GET", "/", nil) + request, _ := http.NewRequest(http.MethodGet, "/", nil) request.Header.Add(acceptHeader, acceptTextPlain) request.Header.Add(acceptEncodingHeader, string(compression)) @@ -533,7 +533,7 @@ func TestNegotiateEncodingWriter(t *testing.T) { } for _, test := range testCases { - request, _ := http.NewRequest("GET", "/", nil) + request, _ := http.NewRequest(http.MethodGet, "/", nil) request.Header.Add(acceptEncodingHeader, test.acceptEncoding) rr := httptest.NewRecorder() _, encodingHeader, _, err := negotiateEncodingWriter(request, rr, test.offeredCompressions) @@ -631,7 +631,7 @@ func BenchmarkCompression(b *testing.B) { b.Run(benchmark.name+"_"+size.name, func(b *testing.B) { for i := 0; i < b.N; i++ { writer := httptest.NewRecorder() - request, _ := http.NewRequest("GET", "/", nil) + request, _ := http.NewRequest(http.MethodGet, "/", nil) request.Header.Add(acceptEncodingHeader, benchmark.compressionType) handler.ServeHTTP(writer, request) } diff --git a/prometheus/promhttp/instrument_client_test.go b/prometheus/promhttp/instrument_client_test.go index ce7c4da54..56133499c 100644 --- a/prometheus/promhttp/instrument_client_test.go +++ b/prometheus/promhttp/instrument_client_test.go @@ -223,7 +223,7 @@ func TestClientMiddlewareAPI_WithRequestContext(t *testing.T) { })) defer backend.Close() - req, err := http.NewRequest("GET", backend.URL, nil) + req, err := http.NewRequest(http.MethodGet, backend.URL, nil) if err != nil { t.Fatalf("%v", err) } @@ -276,7 +276,7 @@ func TestClientMiddlewareAPIWithRequestContextTimeout(t *testing.T) { })) defer backend.Close() - req, err := http.NewRequest("GET", backend.URL, nil) + req, err := http.NewRequest(http.MethodGet, backend.URL, nil) if err != nil { t.Fatalf("%v", err) } diff --git a/prometheus/promhttp/instrument_server_test.go b/prometheus/promhttp/instrument_server_test.go index 8aef09f70..600f593db 100644 --- a/prometheus/promhttp/instrument_server_test.go +++ b/prometheus/promhttp/instrument_server_test.go @@ -418,7 +418,7 @@ func TestMiddlewareAPI(t *testing.T) { _, _ = w.Write([]byte("OK")) }) - r, _ := http.NewRequest("GET", "www.example.com", nil) + r, _ := http.NewRequest(http.MethodGet, "www.example.com", nil) w := httptest.NewRecorder() chain.ServeHTTP(w, r) @@ -432,7 +432,7 @@ func TestMiddlewareAPI_WithExemplars(t *testing.T) { _, _ = w.Write([]byte("OK")) }, WithExemplarFromContext(func(_ context.Context) prometheus.Labels { return exemplar })) - r, _ := http.NewRequest("GET", "www.example.com", nil) + r, _ := http.NewRequest(http.MethodGet, "www.example.com", nil) w := httptest.NewRecorder() chain.ServeHTTP(w, r) diff --git a/prometheus/registry_test.go b/prometheus/registry_test.go index f6bca22b7..b8505c7d2 100644 --- a/prometheus/registry_test.go +++ b/prometheus/registry_test.go @@ -714,7 +714,7 @@ collected metric "broken_metric" { label: label: Date: Mon, 11 Nov 2024 17:26:17 +0100 Subject: [PATCH 8/9] Add: exponential backoff for CAS operations on floats (#1661) * add: exponential backoff for CAS operations of floats Signed-off-by: Ivan Goncharov * add: some more benchmark use cases (higher contention) Signed-off-by: Ivan Goncharov * fmt: fumpted some files Signed-off-by: Ivan Goncharov * add: license header Signed-off-by: Ivan Goncharov * add: comment explaining origin of backoff constants Signed-off-by: Ivan Goncharov --------- Signed-off-by: Ivan Goncharov --- prometheus/atomic_update.go | 50 +++++++++ prometheus/atomic_update_test.go | 167 +++++++++++++++++++++++++++++++ prometheus/counter.go | 10 +- prometheus/gauge.go | 10 +- prometheus/histogram.go | 10 +- prometheus/summary.go | 25 ++--- 6 files changed, 236 insertions(+), 36 deletions(-) create mode 100644 prometheus/atomic_update.go create mode 100644 prometheus/atomic_update_test.go diff --git a/prometheus/atomic_update.go b/prometheus/atomic_update.go new file mode 100644 index 000000000..b65896a31 --- /dev/null +++ b/prometheus/atomic_update.go @@ -0,0 +1,50 @@ +// Copyright 2014 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package prometheus + +import ( + "math" + "sync/atomic" + "time" +) + +// atomicUpdateFloat atomically updates the float64 value pointed to by bits +// using the provided updateFunc, with an exponential backoff on contention. +func atomicUpdateFloat(bits *uint64, updateFunc func(float64) float64) { + const ( + // both numbers are derived from empirical observations + // documented in this PR: https://github.com/prometheus/client_golang/pull/1661 + maxBackoff = 320 * time.Millisecond + initialBackoff = 10 * time.Millisecond + ) + backoff := initialBackoff + + for { + loadedBits := atomic.LoadUint64(bits) + oldFloat := math.Float64frombits(loadedBits) + newFloat := updateFunc(oldFloat) + newBits := math.Float64bits(newFloat) + + if atomic.CompareAndSwapUint64(bits, loadedBits, newBits) { + break + } else { + // Exponential backoff with sleep and cap to avoid infinite wait + time.Sleep(backoff) + backoff *= 2 + if backoff > maxBackoff { + backoff = maxBackoff + } + } + } +} diff --git a/prometheus/atomic_update_test.go b/prometheus/atomic_update_test.go new file mode 100644 index 000000000..0233bc71f --- /dev/null +++ b/prometheus/atomic_update_test.go @@ -0,0 +1,167 @@ +// Copyright 2014 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package prometheus + +import ( + "math" + "sync" + "sync/atomic" + "testing" + "unsafe" +) + +var output float64 + +func TestAtomicUpdateFloat(t *testing.T) { + var val float64 = 0.0 + bits := (*uint64)(unsafe.Pointer(&val)) + var wg sync.WaitGroup + numGoroutines := 100000 + increment := 1.0 + + for i := 0; i < numGoroutines; i++ { + wg.Add(1) + go func() { + defer wg.Done() + atomicUpdateFloat(bits, func(f float64) float64 { + return f + increment + }) + }() + } + + wg.Wait() + expected := float64(numGoroutines) * increment + if val != expected { + t.Errorf("Expected %f, got %f", expected, val) + } +} + +// Benchmark for atomicUpdateFloat with single goroutine (no contention). +func BenchmarkAtomicUpdateFloat_SingleGoroutine(b *testing.B) { + var val float64 = 0.0 + bits := (*uint64)(unsafe.Pointer(&val)) + + for i := 0; i < b.N; i++ { + atomicUpdateFloat(bits, func(f float64) float64 { + return f + 1.0 + }) + } + + output = val +} + +// Benchmark for old implementation with single goroutine (no contention) -> to check overhead of backoff +func BenchmarkAtomicNoBackoff_SingleGoroutine(b *testing.B) { + var val float64 = 0.0 + bits := (*uint64)(unsafe.Pointer(&val)) + + for i := 0; i < b.N; i++ { + for { + loadedBits := atomic.LoadUint64(bits) + newBits := math.Float64bits(math.Float64frombits(loadedBits) + 1.0) + if atomic.CompareAndSwapUint64(bits, loadedBits, newBits) { + break + } + } + } + + output = val +} + +// Benchmark varying the number of goroutines. +func benchmarkAtomicUpdateFloatConcurrency(b *testing.B, numGoroutines int) { + var val float64 = 0.0 + bits := (*uint64)(unsafe.Pointer(&val)) + b.SetParallelism(numGoroutines) + + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + atomicUpdateFloat(bits, func(f float64) float64 { + return f + 1.0 + }) + } + }) + + output = val +} + +func benchmarkAtomicNoBackoffFloatConcurrency(b *testing.B, numGoroutines int) { + var val float64 = 0.0 + bits := (*uint64)(unsafe.Pointer(&val)) + b.SetParallelism(numGoroutines) + + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + for { + loadedBits := atomic.LoadUint64(bits) + newBits := math.Float64bits(math.Float64frombits(loadedBits) + 1.0) + if atomic.CompareAndSwapUint64(bits, loadedBits, newBits) { + break + } + } + } + }) + + output = val +} + +func BenchmarkAtomicUpdateFloat_1Goroutine(b *testing.B) { + benchmarkAtomicUpdateFloatConcurrency(b, 1) +} + +func BenchmarkAtomicNoBackoff_1Goroutine(b *testing.B) { + benchmarkAtomicNoBackoffFloatConcurrency(b, 1) +} + +func BenchmarkAtomicUpdateFloat_2Goroutines(b *testing.B) { + benchmarkAtomicUpdateFloatConcurrency(b, 2) +} + +func BenchmarkAtomicNoBackoff_2Goroutines(b *testing.B) { + benchmarkAtomicNoBackoffFloatConcurrency(b, 2) +} + +func BenchmarkAtomicUpdateFloat_4Goroutines(b *testing.B) { + benchmarkAtomicUpdateFloatConcurrency(b, 4) +} + +func BenchmarkAtomicNoBackoff_4Goroutines(b *testing.B) { + benchmarkAtomicNoBackoffFloatConcurrency(b, 4) +} + +func BenchmarkAtomicUpdateFloat_8Goroutines(b *testing.B) { + benchmarkAtomicUpdateFloatConcurrency(b, 8) +} + +func BenchmarkAtomicNoBackoff_8Goroutines(b *testing.B) { + benchmarkAtomicNoBackoffFloatConcurrency(b, 8) +} + +func BenchmarkAtomicUpdateFloat_16Goroutines(b *testing.B) { + benchmarkAtomicUpdateFloatConcurrency(b, 16) +} + +func BenchmarkAtomicNoBackoff_16Goroutines(b *testing.B) { + benchmarkAtomicNoBackoffFloatConcurrency(b, 16) +} + +func BenchmarkAtomicUpdateFloat_32Goroutines(b *testing.B) { + benchmarkAtomicUpdateFloatConcurrency(b, 32) +} + +func BenchmarkAtomicNoBackoff_32Goroutines(b *testing.B) { + benchmarkAtomicNoBackoffFloatConcurrency(b, 32) +} diff --git a/prometheus/counter.go b/prometheus/counter.go index 4ce84e7a8..2996aef6a 100644 --- a/prometheus/counter.go +++ b/prometheus/counter.go @@ -134,13 +134,9 @@ func (c *counter) Add(v float64) { return } - for { - oldBits := atomic.LoadUint64(&c.valBits) - newBits := math.Float64bits(math.Float64frombits(oldBits) + v) - if atomic.CompareAndSwapUint64(&c.valBits, oldBits, newBits) { - return - } - } + atomicUpdateFloat(&c.valBits, func(oldVal float64) float64 { + return oldVal + v + }) } func (c *counter) AddWithExemplar(v float64, e Labels) { diff --git a/prometheus/gauge.go b/prometheus/gauge.go index dd2eac940..aa1846365 100644 --- a/prometheus/gauge.go +++ b/prometheus/gauge.go @@ -120,13 +120,9 @@ func (g *gauge) Dec() { } func (g *gauge) Add(val float64) { - for { - oldBits := atomic.LoadUint64(&g.valBits) - newBits := math.Float64bits(math.Float64frombits(oldBits) + val) - if atomic.CompareAndSwapUint64(&g.valBits, oldBits, newBits) { - return - } - } + atomicUpdateFloat(&g.valBits, func(oldVal float64) float64 { + return oldVal + val + }) } func (g *gauge) Sub(val float64) { diff --git a/prometheus/histogram.go b/prometheus/histogram.go index c0b6ecd7b..6deff565d 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -1641,13 +1641,9 @@ func waitForCooldown(count uint64, counts *histogramCounts) { // atomicAddFloat adds the provided float atomically to another float // represented by the bit pattern the bits pointer is pointing to. func atomicAddFloat(bits *uint64, v float64) { - for { - loadedBits := atomic.LoadUint64(bits) - newBits := math.Float64bits(math.Float64frombits(loadedBits) + v) - if atomic.CompareAndSwapUint64(bits, loadedBits, newBits) { - break - } - } + atomicUpdateFloat(bits, func(oldVal float64) float64 { + return oldVal + v + }) } // atomicDecUint32 atomically decrements the uint32 p points to. See diff --git a/prometheus/summary.go b/prometheus/summary.go index ac5203c6f..76a9e12f4 100644 --- a/prometheus/summary.go +++ b/prometheus/summary.go @@ -471,13 +471,9 @@ func (s *noObjectivesSummary) Observe(v float64) { n := atomic.AddUint64(&s.countAndHotIdx, 1) hotCounts := s.counts[n>>63] - for { - oldBits := atomic.LoadUint64(&hotCounts.sumBits) - newBits := math.Float64bits(math.Float64frombits(oldBits) + v) - if atomic.CompareAndSwapUint64(&hotCounts.sumBits, oldBits, newBits) { - break - } - } + atomicUpdateFloat(&hotCounts.sumBits, func(oldVal float64) float64 { + return oldVal + v + }) // Increment count last as we take it as a signal that the observation // is complete. atomic.AddUint64(&hotCounts.count, 1) @@ -519,14 +515,13 @@ func (s *noObjectivesSummary) Write(out *dto.Metric) error { // Finally add all the cold counts to the new hot counts and reset the cold counts. atomic.AddUint64(&hotCounts.count, count) atomic.StoreUint64(&coldCounts.count, 0) - for { - oldBits := atomic.LoadUint64(&hotCounts.sumBits) - newBits := math.Float64bits(math.Float64frombits(oldBits) + sum.GetSampleSum()) - if atomic.CompareAndSwapUint64(&hotCounts.sumBits, oldBits, newBits) { - atomic.StoreUint64(&coldCounts.sumBits, 0) - break - } - } + + // Use atomicUpdateFloat to update hotCounts.sumBits atomically. + atomicUpdateFloat(&hotCounts.sumBits, func(oldVal float64) float64 { + return oldVal + sum.GetSampleSum() + }) + atomic.StoreUint64(&coldCounts.sumBits, 0) + return nil } From 13851e92875918e91882566432243aa007548314 Mon Sep 17 00:00:00 2001 From: PrometheusBot Date: Tue, 12 Nov 2024 21:02:43 +0100 Subject: [PATCH 9/9] Update common Prometheus files (#1683) Signed-off-by: prombot --- Makefile.common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.common b/Makefile.common index cbb5d8638..09e5bff85 100644 --- a/Makefile.common +++ b/Makefile.common @@ -61,7 +61,7 @@ PROMU_URL := https://github.com/prometheus/promu/releases/download/v$(PROMU_ SKIP_GOLANGCI_LINT := GOLANGCI_LINT := GOLANGCI_LINT_OPTS ?= -GOLANGCI_LINT_VERSION ?= v1.60.2 +GOLANGCI_LINT_VERSION ?= v1.61.0 # golangci-lint only supports linux, darwin and windows platforms on i386/amd64/arm64. # windows isn't included here because of the path separator being different. ifeq ($(GOHOSTOS),$(filter $(GOHOSTOS),linux darwin))