fix(godeltaprof): fix mutex tests on gotip (#108)

Fixes #103

This PR fixes an issue when a heap/mutex profile has a record with equal stack (by accumulating values for duplicate stacks before delta computation).
The issue was found in tests for mutex(golang started to produce two profile records for the same mutex ( see the linked issue for more info))
I'm not sure if it is possible to trigger the issue for heap.

The PR turned up big because of refactored test helpers for reuse. I can split PR into 2 or 3 smaller ones if needed.

This PR also drops support for go16, go17 as I wanted to use generics in prof/map.go

goos: darwin
goarch: arm64
pkg: github.com/grafana/pyroscope-go/godeltaprof/compat
                                      │   old.txt   │               new.txt                │
                                      │   sec/op    │    sec/op      vs base               │
HeapCompression-8                       691.1µ ± 0%    729.8µ ± 65%   +5.59% (p=0.002 n=6)
MutexCompression/ScalerMutexProfile-8   640.6µ ± 1%    693.8µ ±  9%   +8.30% (p=0.002 n=6)
MutexCompression/ScalerBlockProfile-8   643.2µ ± 1%    690.3µ ±  1%   +7.33% (p=0.002 n=6)
HeapDelta-8                             76.26µ ± 1%   113.92µ ±  2%  +49.38% (p=0.002 n=6)
MutexDelta/ScalerMutexProfile-8         48.98µ ± 1%    81.11µ ±  2%  +65.59% (p=0.002 n=6)
MutexDelta/ScalerBlockProfile-8         49.03µ ± 1%    81.14µ ±  1%  +65.49% (p=0.002 n=6)
geomean                                 193.3µ         253.0µ        +30.87%

                                      │   old.txt    │               new.txt               │
                                      │     B/op     │     B/op      vs base               │
HeapCompression-8                       974.1Ki ± 0%   973.9Ki ± 0%   -0.03% (p=0.002 n=6)
MutexCompression/ScalerMutexProfile-8   974.6Ki ± 0%   974.3Ki ± 0%   -0.02% (p=0.002 n=6)
MutexCompression/ScalerBlockProfile-8   974.6Ki ± 0%   974.3Ki ± 0%   -0.03% (p=0.002 n=6)
HeapDelta-8                              333.00 ± 0%     53.00 ± 0%  -84.08% (p=0.002 n=6)
MutexDelta/ScalerMutexProfile-8          312.00 ± 0%     30.00 ± 0%  -90.38% (p=0.002 n=6)
MutexDelta/ScalerBlockProfile-8          312.00 ± 0%     30.00 ± 0%  -90.38% (p=0.002 n=6)
geomean                                 17.42Ki        5.874Ki       -66.28%

                                      │  old.txt   │              new.txt              │
                                      │ allocs/op  │ allocs/op   vs base               │
HeapCompression-8                       788.0 ± 0%   787.0 ± 0%   -0.13% (p=0.002 n=6)
MutexCompression/ScalerMutexProfile-8   788.0 ± 0%   787.0 ± 0%   -0.13% (p=0.002 n=6)
MutexCompression/ScalerBlockProfile-8   788.0 ± 0%   787.0 ± 0%   -0.13% (p=0.002 n=6)
HeapDelta-8                             2.000 ± 0%   1.000 ± 0%  -50.00% (p=0.002 n=6)
MutexDelta/ScalerMutexProfile-8         2.000 ± 0%   1.000 ± 0%  -50.00% (p=0.002 n=6)
MutexDelta/ScalerBlockProfile-8         2.000 ± 0%   1.000 ± 0%  -50.00% (p=0.002 n=6)
geomean                                 39.70        28.05       -29.33%

So it looks like the cost of deduplication is about 5% of overall time(including compression), which is not that bad.

.github/workflows/go.yml

@@ -12,7 +12,7 @@ jobs:
  strategy:
  fail-fast: false
  matrix:
- go: ['1.16', '1.17', '1.18', '1.19', '1.20', '1.21', '1.22', 'tip']
+ go: ['1.18', '1.19', '1.20', '1.21', '1.22', 'tip']
  steps:
  - name: Checkout
  uses: actions/checkout@v3
@@ -21,11 +21,7 @@ jobs:
  uses: actions/setup-go@v4
  with:
  go-version: ${{ matrix.go }}
- - name: Downgrade go.mod for go 1.17 and go 1.16
- if: matrix.go == '1.17' || matrix.go == '1.16'
- run: make go/mod_16_for_testing
  - name: Run go/mod
- if: matrix.go != '1.17' && matrix.go != '1.16'
  run: make go/mod && git diff --exit-code
  - name: Install Go stable
  if: matrix.go == 'tip'

Makefile

@@ -15,8 +15,3 @@ go/mod:
  cd godeltaprof/ && GO111MODULE=on go mod download
  cd godeltaprof/ && GO111MODULE=on go mod tidy
 
-.PHONY: go/mod_16_for_testing
-go/mod_16_for_testing:
- rm -rf godeltaprof/compat/go.mod godeltaprof/compat/go.sum godeltaprof/go.mod godeltaprof/go.sum go.work otelpyroscope/
- cat go.mod_go16_test.txt > go.mod
- go mod tidy

go.mod

@@ -6,4 +6,4 @@ replace github.com/grafana/pyroscope-go/godeltaprof => ./godeltaprof
 
 require github.com/grafana/pyroscope-go/godeltaprof v0.1.6
 
-require github.com/klauspost/compress v1.17.3 // indirect
+require github.com/klauspost/compress v1.17.8 // indirect

go.sum

@@ -1,2 +1,2 @@
-github.com/klauspost/compress v1.17.3 h1:qkRjuerhUU1EmXLYGkSH6EZL+vPSxIrYjLNAK4slzwA=
-github.com/klauspost/compress v1.17.3/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM=
+github.com/klauspost/compress v1.17.8 h1:YcnTYrq7MikUT7k0Yb5eceMmALQPYBW/Xltxn0NAMnU=
+github.com/klauspost/compress v1.17.8/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw=

go.work.sum

@@ -6,17 +6,12 @@ github.com/chromedp/sysutil v1.0.0 h1:+ZxhTpfpZlmchB58ih/LBHX52ky7w2VhQVKQMucy3I
 github.com/chromedp/sysutil v1.0.0/go.mod h1:kgWmDdq8fTzXYcKIBqIYvRRTnYb9aNS9moAV0xufSww=
 github.com/chzyer/readline v1.5.1 h1:upd/6fQk4src78LMRzh5vItIt361/o4uq553V8B5sGI=
 github.com/chzyer/readline v1.5.1/go.mod h1:Eh+b79XXUwfKfcPLepksvw2tcLE/Ct21YObkaSkeBlk=
-github.com/go-logr/logr v1.2.3 h1:2DntVwHkVopvECVRSlL5PSo9eG+cAkDCuckLubN+rq0=
-github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
 github.com/gobwas/httphead v0.1.0 h1:exrUm0f4YX0L7EBwZHuCF4GDp8aJfVeBrlLQrs6NqWU=
 github.com/gobwas/httphead v0.1.0/go.mod h1:O/RXo79gxV8G+RqlR/otEwx4Q36zl9rqC5u12GKvMCM=
 github.com/gobwas/pool v0.2.1 h1:xfeeEhW7pwmX8nuLVlqbzVc7udMDrwetjEv+TZIz1og=
 github.com/gobwas/pool v0.2.1/go.mod h1:q8bcK0KcYlCgd9e7WYLm9LpyS+YeLd8JVDW6WezmKEw=
 github.com/gobwas/ws v1.2.1 h1:F2aeBZrm2NDsc7vbovKrWSogd4wvfAxg0FQ89/iqOTk=
 github.com/gobwas/ws v1.2.1/go.mod h1:hRKAFb8wOxFROYNsT1bqfWnhX+b5MFeJM9r2ZSwg/KY=
-github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
-github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
-github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/ianlancetaylor/demangle v0.0.0-20230524184225-eabc099b10ab h1:BA4a7pe6ZTd9F8kXETBoijjFJ/ntaa//1wiH9BZu4zU=
 github.com/ianlancetaylor/demangle v0.0.0-20230524184225-eabc099b10ab/go.mod h1:gx7rwoVhcfuVKG5uya9Hs3Sxj7EIvldVofAWIUtGouw=
 github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
@@ -27,7 +22,6 @@ github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
 github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
 github.com/yuin/goldmark v1.4.13 h1:fVcFKWvrslecOb/tg+Cc05dkeYx540o0FuFt3nUVDoE=
 github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
-golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 h1:7I4JAnoQBe7ZtJcBaYHi5UtiO8tQHbUSXxL+pnGRANg=
 golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c=
 golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U=
 golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=

godeltaprof/compat/compression_test.go

@@ -2,33 +2,27 @@ package compat
 
 import (
  "io"
- "math/rand"
  "runtime"
  "testing"
-
- "github.com/grafana/pyroscope-go/godeltaprof/internal/pprof"
 )
 
 func BenchmarkHeapCompression(b *testing.B) {
- opt := &pprof.ProfileBuilderOptions{
- GenericsFrames: true,
- LazyMapping: true,
- }
- dh := new(pprof.DeltaHeapProfiler)
- fs := generateMemProfileRecords(512, 32, 239)
- rng := rand.NewSource(239)
- objSize := fs[0].AllocBytes / fs[0].AllocObjects
- nMutations := int(uint(rng.Int63())) % len(fs)
+ h := newHeapTestHelper()
+ fs := h.generateMemProfileRecords(512, 32)
+ h.rng.Seed(239)
+ nmutations := int(h.rng.Int63() % int64(len(fs)))
+
  b.ResetTimer()
  for i := 0; i < b.N; i++ {
- _ = WriteHeapProto(dh, opt, io.Discard, fs, int64(runtime.MemProfileRate))
- for j := 0; j < nMutations; j++ {
- idx := int(uint(rng.Int63())) % len(fs)
- fs[idx].AllocObjects += 1
- fs[idx].AllocBytes += objSize
- fs[idx].FreeObjects += 1
- fs[idx].FreeBytes += objSize
+ if i == 1000 {
+ v := h.rng.Int63()
+ if v != 7817861117094116717 {
+ b.Errorf("unexpected random value: %d. "+
+ "The bench should be deterministic for better comparision.", v)
+ }
  }
+ _ = WriteHeapProto(h.dp, h.opt, io.Discard, fs, int64(runtime.MemProfileRate))
+ h.mutate(nmutations, fs)
  }
 }
 
@@ -43,38 +37,25 @@ func BenchmarkMutexCompression(b *testing.B) {
  runtime.SetMutexProfileFraction(5)
  defer runtime.SetMutexProfileFraction(prevMutexProfileFraction)
 
- opt := &pprof.ProfileBuilderOptions{
- GenericsFrames: true,
- LazyMapping: true,
- }
- dh := new(pprof.DeltaMutexProfiler)
- fs := generateBlockProfileRecords(512, 32, 239)
- rng := rand.NewSource(239)
- nMutations := int(uint(rng.Int63())) % len(fs)
- oneBlockCycles := fs[0].Cycles / fs[0].Count
+ h := newMutexTestHelper()
+ h.scaler = scaler
+ fs := h.generateBlockProfileRecords(512, 32)
+ h.rng.Seed(239)
+ nmutations := int(h.rng.Int63() % int64(len(fs)))
  b.ResetTimer()
 
  for i := 0; i < b.N; i++ {
- _ = PrintCountCycleProfile(dh, opt, io.Discard, scaler, fs)
- for j := 0; j < nMutations; j++ {
- idx := int(uint(rng.Int63())) % len(fs)
- fs[idx].Count += 1
- fs[idx].Cycles += oneBlockCycles
+ if i == 1000 {
+ v := h.rng.Int63()
+ if v != 7817861117094116717 {
+ b.Errorf("unexpected random value: %d. "+
+ "The bench should be deterministic for better comparision.", v)
+ }
  }
+ _ = PrintCountCycleProfile(h.dp, h.opt, io.Discard, scaler, fs)
+ h.mutate(nmutations, fs)
  }
  })
 
  }
 }
-
-func WriteHeapProto(dp *pprof.DeltaHeapProfiler, opt *pprof.ProfileBuilderOptions, w io.Writer, p []runtime.MemProfileRecord, rate int64) error {
- stc := pprof.HeapProfileConfig(rate)
- b := pprof.NewProfileBuilder(w, opt, stc)
- return dp.WriteHeapProto(b, p, rate)
-}
-
-func PrintCountCycleProfile(d *pprof.DeltaMutexProfiler, opt *pprof.ProfileBuilderOptions, w io.Writer, scaler pprof.MutexProfileScaler, records []runtime.BlockProfileRecord) error {
- stc := pprof.MutexProfileConfig()
- b := pprof.NewProfileBuilder(w, opt, stc)
- return d.PrintCountCycleProfile(b, scaler, records)
-}