Skip to content

Commit

Permalink
Merge pull request #3 from DataDog/felix.geisendoerfer/PROF-9665-memo…
Browse files Browse the repository at this point in the history
…ry-increase-workaround

workaround upstream inlining issue
  • Loading branch information
felixge authored Apr 25, 2024
2 parents 5068587 + 7b657f2 commit 60fb0c2
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 1 deletion.
8 changes: 7 additions & 1 deletion LICENSE-3rdparty.csv
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,10 @@ N/A,github.com/mattn/go-isatty,MIT,Copyright (c) Yasuhiro MATSUMOTO <mattn.jp@gm
N/A,github.com/sourcegraph/conc,MIT,Copyright (c) 2023 Sourcegraph
N/A,go.uber.org/atomic,MIT,Copyright (c) 2016 Uber Technologies, Inc.
N/A,go.uber.org/multierr,MIT,Copyright (c) 2017-2021 Uber Technologies, Inc.
N/A,golang.org/x/sys,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved.
N/A,golang.org/x/sys,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved.
N/A,github.com/stretchr/testify,MIT,Copyright (c) 2012-2020 Mat Ryer, Tyler Bunnell and contributors.
N/A,github.com/davecgh/go-spew,ISC,Copyright (c) 2012-2016 Dave Collins <[email protected]>
N/A,github.com/kr/text,MIT,Copyright 2012 Keith Rarick
N/A,github.com/pmezard/go-difflib,BSD 3-Clause,Copyright (c) 2013, Patrick Mezard
N/A,gopkg.in/yaml.v3,MIT,Copyright (c) 2006-2010 Kirill Simonov + Copyright (c) 2006-2011 Kirill Simonov
N/A,gopkg.in/yaml.v3,Apache-2.0,Copyright (c) 2011-2019 Canonical Ltd
5 changes: 5 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,15 @@ require (
github.com/lmittmann/tint v1.0.4
github.com/mattn/go-isatty v0.0.20
github.com/sourcegraph/conc v0.3.0
github.com/stretchr/testify v1.8.1
)

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/kr/text v0.2.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
go.uber.org/atomic v1.7.0 // indirect
go.uber.org/multierr v1.9.0 // indirect
golang.org/x/sys v0.6.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
15 changes: 15 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,18 +1,29 @@
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/google/pprof v0.0.0-20240227163752-401108e1b7e7 h1:y3N7Bm7Y9/CtpiVkw/ZWj6lSlDF3F74SfKwfTCer72Q=
github.com/google/pprof v0.0.0-20240227163752-401108e1b7e7/go.mod h1:czg5+yv1E0ZGTi6S6vVK1mke0fV+FaUhNGcd6VRS9Ik=
github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0=
github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/lmittmann/tint v1.0.4 h1:LeYihpJ9hyGvE0w+K2okPTGUdVLfng1+nDNVR4vWISc=
github.com/lmittmann/tint v1.0.4/go.mod h1:HIS3gSy7qNwGCj+5oRjAutErFBl4BzdQP6cJZ0NfMwE=
github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8=
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo=
github.com/sourcegraph/conc v0.3.0/go.mod h1:Sdozi7LEKbFPqYX2/J+iBAM6HpqSLTASQIKqDmF7Mt0=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw=
Expand All @@ -21,5 +32,9 @@ go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI=
go.uber.org/multierr v1.9.0/go.mod h1:X2jQV1h+kxSjClGpnseKVIxpmcjrj7MNnI0bnlfKTVQ=
golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ=
golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo=
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
10 changes: 10 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ OPTIONS`
return err
}

// Apply no inline hack
if err := mergedProfile.ApplyNoInlineHack(); err != nil {
return err
}

// Writing pgo file to dst
n, err := mergedProfile.Write(dst)
if err != nil {
Expand Down Expand Up @@ -322,6 +327,11 @@ func (p *MergedProfile) Merge(id string, prof *profile.Profile) (err error) {
return
}

// ApplyNoInlineHack removes samples that lead to bad inlining decisions.
func (p *MergedProfile) ApplyNoInlineHack() error {
return ApplyNoInlineHack(p.profile)
}

// Write writes the merged profile to dst and returns the number of bytes
// written.
func (p *MergedProfile) Write(dst string) (int64, error) {
Expand Down
50 changes: 50 additions & 0 deletions noinline.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package main

import (
"fmt"

"github.com/google/pprof/profile"
)

const grpcProcessDataFunc = "google.golang.org/grpc/internal/transport.(*loopyWriter).processData"

// ApplyNoInlineHack removes problematic samples from the profile to avoid bad
// inlining decisions that can have a large memory impact.
// See https://github.com/golang/go/issues/65532 for details.
//
// TODO: Delete this once it's fixed upstream.
func ApplyNoInlineHack(prof *profile.Profile) error {
if err := removeLeafSamples(prof, []string{grpcProcessDataFunc}); err != nil {
return fmt.Errorf("noinline hack: %w", err)
}
return nil
}

func removeLeafSamples(prof *profile.Profile, funcs []string) error {
newSamples := make([]*profile.Sample, 0, len(prof.Sample))
for _, s := range prof.Sample {
leaf, ok := leafLine(s)
if ok && lineContainsAny(leaf, funcs) {
continue
}
newSamples = append(newSamples, s)
}
prof.Sample = newSamples
return nil
}

func leafLine(s *profile.Sample) (profile.Line, bool) {
if len(s.Location) == 0 || len(s.Location[0].Line) == 0 {
return profile.Line{}, false
}
return s.Location[0].Line[0], true
}

func lineContainsAny(leaf profile.Line, funcs []string) bool {
for _, fn := range funcs {
if leaf.Function.Name == fn {
return true
}
}
return false
}
36 changes: 36 additions & 0 deletions noinline_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package main

import (
"os"
"path/filepath"
"testing"

"github.com/google/pprof/profile"
"github.com/stretchr/testify/require"
)

func TestApplyNoInlineHack(t *testing.T) {
prof := loadTestProfile(t, "grpc-anon.pprof")
require.Equal(t, 17, leafSamples(prof, grpcProcessDataFunc))
require.NoError(t, ApplyNoInlineHack(prof))
require.Equal(t, 0, leafSamples(prof, grpcProcessDataFunc))
}

func loadTestProfile(t *testing.T, name string) *profile.Profile {
t.Helper()
data, err := os.ReadFile(filepath.Join("testdata", name))
require.NoError(t, err)
prof, err := profile.ParseData(data)
require.NoError(t, err)
return prof
}

func leafSamples(prof *profile.Profile, fn string) (count int) {
for _, s := range prof.Sample {
leaf, ok := leafLine(s)
if ok && leaf.Function.Name == fn {
count++
}
}
return count
}
3 changes: 3 additions & 0 deletions testdata/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```
pprofutils anon -whitelist='^google\.golang\.org/grpc' grpc-orig.pprof grpc-anon.pprof
```
Binary file added testdata/grpc-anon.pprof
Binary file not shown.

0 comments on commit 60fb0c2

Please sign in to comment.