fix: drop type params from Go function names

pkg/distributor/distributor.go

@@ -295,6 +295,9 @@ func (d *Distributor) PushParsed(ctx context.Context, req *distributormodel.Push
 				d.metrics.receivedCompressedBytes.WithLabelValues(profName, tenantID).Observe(float64(len(raw.RawProfile)))
 			}
 			p := raw.Profile
+			if profLanguage == "go" {
+				p.Profile = pprof.FixGoProfile(p.Profile)
+			}
 			decompressedSize := p.SizeVT()
 			d.metrics.receivedDecompressedBytes.WithLabelValues(profName, tenantID).Observe(float64(decompressedSize))
 			d.metrics.receivedSamples.WithLabelValues(profName, tenantID).Observe(float64(len(p.Sample)))

pkg/phlaredb/symdb/resolver_pprof_test.go

@@ -144,7 +144,7 @@ func Test_Pprof_subtree(t *testing.T) {
 func Test_Resolver_pprof_options(t *testing.T) {
 	s := newMemSuite(t, [][]string{{"testdata/profile.pb.gz"}})
 	samples := s.indexed[0][0].Samples
-	const samplesTotal = 572
+	const samplesTotal = 561
 
 	var sc PartitionStats
 	s.db.partitions[0].WriteStats(&sc)

pkg/pprof/fix_go_profile.go

@@ -0,0 +1,39 @@
+package pprof
+
+import (
+	profilev1 "github.com/grafana/pyroscope/api/gen/proto/go/google/v1"
+)
+
+// FixGoProfile removes type parameters from Go function names.
+//
+// In go 1.21 and above, function names include type parameters,
+// however, due to the bug, a function name can include any
+// of the type instances regardless of the call site. Thus, e.g.,
+// x[T1].foo and x[T2].foo can't be distinguished in a profile.
+// This leads to incorrect profiles and incorrect flame graphs,
+// and hugely increases cardinality of stack traces.
+//
+// FixGoProfile will change x[T1].foo and x[T2].foo to x[...].foo
+// and normalize the profile, if type parameters are present in
+// the profile. Otherwise, the profile returned unchanged.
+func FixGoProfile(p *profilev1.Profile) *profilev1.Profile {
+	var n int
+	for i, s := range p.StringTable {
+		c := DropGoTypeParameters(s)
+		if c != s {
+			p.StringTable[i] = c
+			n++
+		}
+	}
+	if n == 0 {
+		return p
+	}
+	// Merging with self effectively normalizes the profile:
+	// it removed duplicates, establishes order of labels,
+	// and allocates monotonic identifiers.
+	var m ProfileMerge
+	// We safely ignore the error as the only case when it can
+	// happen is when merged profiles are of different types.
+	_ = m.MergeNoClone(p)
+	return m.Profile()
+}

pkg/pprof/fix_go_profile_test.go

@@ -0,0 +1,36 @@
+package pprof
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_FixGoProfile(t *testing.T) {
+	p, err := OpenFile("testdata/heap_go1.21.pprof")
+	require.NoError(t, err)
+
+	f := FixGoProfile(p.Profile)
+	s := make(map[string]struct{})
+	for _, x := range f.StringTable {
+		if _, ok := s[x]; !ok {
+			s[x] = struct{}{}
+		} else {
+			t.Fatal("duplicate string found")
+		}
+	}
+
+	t.Logf(" * Sample:   %6d -> %-6d", len(p.Sample), len(f.Sample))
+	t.Logf(" * Location: %6d -> %-6d", len(p.Location), len(f.Location))
+	t.Logf(" * Function: %6d -> %-6d", len(p.Function), len(f.Function))
+	t.Logf(" * Strings:  %6d -> %-6d", len(p.StringTable), len(f.StringTable))
+	// fix_test.go:24:  * Sample:     6785 -> 3797
+	// fix_test.go:25:  * Location:   4848 -> 4680
+	// fix_test.go:26:  * Function:   2801 -> 2724
+	// fix_test.go:27:  * Strings:    3536 -> 3458
+	assert.Equal(t, 2988, len(p.Sample)-len(f.Sample))
+	assert.Equal(t, 168, len(p.Location)-len(f.Location))
+	assert.Equal(t, 77, len(p.Function)-len(f.Function))
+	assert.Equal(t, 78, len(p.StringTable)-len(f.StringTable))
+}

pkg/model/fmt.go → pkg/pprof/fmt.go

@@ -1,4 +1,4 @@
-package model
+package pprof
 
 import (
 	"regexp"

pkg/model/fmt_test.go → pkg/pprof/fmt_test.go

@@ -1,4 +1,4 @@
-package model
+package pprof
 
 import (
 	"bufio"

pkg/pprof/merge.go

@@ -20,23 +20,38 @@ type ProfileMerge struct {
 	sampleTable   RewriteTable[SampleKey, *profilev1.Sample, *profilev1.Sample]
 }
 
-// Merge adds p to the profile merge.
-// Profile is modified in place but not retained by the function.
+// Merge adds p to the profile merge, cloning new objects.
+// Profile p is modified in place but not retained by the function.
 func (m *ProfileMerge) Merge(p *profilev1.Profile) error {
+	return m.merge(p, true)
+}
+
+// MergeNoClone adds p to the profile merge, borrowing objects.
+// Profile p is modified in place and retained by the function.
+func (m *ProfileMerge) MergeNoClone(p *profilev1.Profile) error {
+	return m.merge(p, false)
+}
+
+func (m *ProfileMerge) merge(p *profilev1.Profile, clone bool) error {
 	if p == nil || len(p.StringTable) < 2 {
 		return nil
 	}
 	ConvertIDsToIndices(p)
 	if m.profile == nil {
-		m.init(p)
-		return nil
+		m.init(p, clone)
 	}
 
 	// We rewrite strings first in order to compare
 	// sample types and period type.
 	m.tmp = slices.GrowLen(m.tmp, len(p.StringTable))
 	m.stringTable.Index(m.tmp, p.StringTable)
 	RewriteStrings(p, m.tmp)
+	if len(m.profile.StringTable) == 0 {
+		// Right after initialisation we need to make
+		// sure that the string identifiers are normalized
+		// among profiles.
+		RewriteStrings(m.profile, m.tmp)
+	}
 
 	if err := combineHeaders(m.profile, p); err != nil {
 		return err
@@ -93,58 +108,63 @@ func (m *ProfileMerge) Profile() *profilev1.Profile {
 	return m.profile
 }
 
-func (m *ProfileMerge) init(x *profilev1.Profile) {
+func (m *ProfileMerge) init(x *profilev1.Profile, clone bool) {
 	factor := 2
 	m.stringTable = NewRewriteTable(
 		factor*len(x.StringTable),
 		func(s string) string { return s },
 		func(s string) string { return s },
 	)
 
-	m.functionTable = NewRewriteTable[FunctionKey, *profilev1.Function, *profilev1.Function](
-		factor*len(x.Function), GetFunctionKey, cloneVT[*profilev1.Function])
+	if clone {
+		m.functionTable = NewRewriteTable[FunctionKey, *profilev1.Function, *profilev1.Function](
+			factor*len(x.Function), GetFunctionKey, cloneVT[*profilev1.Function])
+
+		m.mappingTable = NewRewriteTable[MappingKey, *profilev1.Mapping, *profilev1.Mapping](
+			factor*len(x.Mapping), GetMappingKey, cloneVT[*profilev1.Mapping])
+
+		m.locationTable = NewRewriteTable[LocationKey, *profilev1.Location, *profilev1.Location](
+			factor*len(x.Location), GetLocationKey, cloneVT[*profilev1.Location])
 
-	m.mappingTable = NewRewriteTable[MappingKey, *profilev1.Mapping, *profilev1.Mapping](
-		factor*len(x.Mapping), GetMappingKey, cloneVT[*profilev1.Mapping])
+		m.sampleTable = NewRewriteTable[SampleKey, *profilev1.Sample, *profilev1.Sample](
+			factor*len(x.Sample), GetSampleKey, func(sample *profilev1.Sample) *profilev1.Sample {
+				c := sample.CloneVT()
+				slices.Clear(c.Value)
+				return c
+			})
+	} else {
+		m.functionTable = NewRewriteTable[FunctionKey, *profilev1.Function, *profilev1.Function](
+			factor*len(x.Function), GetFunctionKey, noClone[*profilev1.Function])
 
-	m.locationTable = NewRewriteTable[LocationKey, *profilev1.Location, *profilev1.Location](
-		factor*len(x.Location), GetLocationKey, cloneVT[*profilev1.Location])
+		m.mappingTable = NewRewriteTable[MappingKey, *profilev1.Mapping, *profilev1.Mapping](
+			factor*len(x.Mapping), GetMappingKey, noClone[*profilev1.Mapping])
 
-	m.sampleTable = NewRewriteTable[SampleKey, *profilev1.Sample, *profilev1.Sample](
-		factor*len(x.Sample), GetSampleKey, func(sample *profilev1.Sample) *profilev1.Sample {
-			c := sample.CloneVT()
-			slices.Clear(c.Value)
-			return c
-		})
+		m.locationTable = NewRewriteTable[LocationKey, *profilev1.Location, *profilev1.Location](
+			factor*len(x.Location), GetLocationKey, noClone[*profilev1.Location])
+
+		m.sampleTable = NewRewriteTable[SampleKey, *profilev1.Sample, *profilev1.Sample](
+			factor*len(x.Sample), GetSampleKey, noClone[*profilev1.Sample])
+	}
 
 	m.profile = &profilev1.Profile{
-		SampleType:        make([]*profilev1.ValueType, len(x.SampleType)),
-		DropFrames:        x.DropFrames,
-		KeepFrames:        x.KeepFrames,
-		TimeNanos:         x.TimeNanos,
-		DurationNanos:     x.DurationNanos,
+		SampleType: make([]*profilev1.ValueType, len(x.SampleType)),
+		DropFrames: x.DropFrames,
+		KeepFrames: x.KeepFrames,
+		TimeNanos:  x.TimeNanos,
+		// Profile durations are summed up, therefore
+		// we skip the field at initialization.
+		// DurationNanos:  x.DurationNanos,
 		PeriodType:        x.PeriodType.CloneVT(),
 		Period:            x.Period,
 		DefaultSampleType: x.DefaultSampleType,
 	}
 	for i, st := range x.SampleType {
 		m.profile.SampleType[i] = st.CloneVT()
 	}
-
-	m.sampleTable.Append(x.Sample)
-	m.locationTable.Append(x.Location)
-	m.functionTable.Append(x.Function)
-	m.mappingTable.Append(x.Mapping)
-	m.stringTable.Append(x.StringTable)
-
-	for i, s := range x.Sample {
-		dst := m.sampleTable.s[i].Value
-		for j, v := range s.Value {
-			dst[j] += v
-		}
-	}
 }
 
+func noClone[T any](t T) T { return t }
+
 func cloneVT[T interface{ CloneVT() T }](t T) T { return t.CloneVT() }
 
 // combineHeaders checks that all profiles can be merged and returns