Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/pprof: discrepency between pprof list and weblist #27473

Closed
ardan-bkennedy opened this issue Sep 3, 2018 · 6 comments
Closed

cmd/pprof: discrepency between pprof list and weblist #27473

ardan-bkennedy opened this issue Sep 3, 2018 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@ardan-bkennedy
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.11 darwin/amd64

Does this issue reproduce with the latest release?

YES

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/bill/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/bill/code/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/p6/l5y36lvx3zvdc3gtfh0g_43w0000gn/T/go-build393058492=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Using this code:
https://github.com/ardanlabs/gotraining/tree/master/topics/go/profiling/memcpu

I ran this set of commands:

$ go test -gcflags "-m -m" -run none -bench . -benchtime 3s -benchmem -memprofile p.out  
$ go tool pprof p.out  
(pprof) weblist algOne  

What did you expect to see?

I saw the weblist view was different (and more accurate) then the list view

weblist view

Type: alloc_space
Time: Sep 3, 2018 at 10:39am (PDT)
Total: 325.53MB
github.com/ardanlabs/gotraining/topics/go/profiling/memcpu.algOne
/Users/bill/code/go/src/github.com/ardanlabs/gotraining/topics/go/profiling/memcpu/stream.go

  Total:    325.53MB   325.53MB (flat, cum)   100%
     78            .          .            
     79            .          .           // algOne is one way to solve the problem. 
     80            .          .           func algOne(data []byte, find []byte, repl []byte, output *bytes.Buffer) { 
     81            .          .            
     82            .          .           	// Use a bytes Buffer to provide a stream to process. 
     83     311.03MB   311.03MB           	input := bytes.NewBuffer(data) 
     84            .          .            
     85            .          .           	// The number of bytes we are looking for. 
     86            .          .           	size := len(find) 
     87            .          .            
     88            .          .           	// Declare the buffers we need to process the stream. 
     89      14.50MB    14.50MB           	buf := make([]byte, size) 
     90            .          .           	end := size - 1 
     91            .          .            
     92            .          .           	// Read in an initial number of bytes we need to get started. 
     93            .          .           	if n, err := io.ReadFull(input, buf[:end]); err != nil { 
     94            .          .           		output.Write(buf[:n]) 

list view .

ROUTINE ======================== github.com/ardanlabs/gotraining/topics/go/profiling/memcpu.algOne in /Users/bill/code/go/src/github.com/ardanlabs/gotraining/topics/go/profiling/memcpu/stream.go
   14.50MB   325.53MB (flat, cum)   100% of Total
         .          .     78:
         .          .     79:// algOne is one way to solve the problem.
         .          .     80:func algOne(data []byte, find []byte, repl []byte, output *bytes.Buffer) {
         .          .     81:
         .          .     82:	// Use a bytes Buffer to provide a stream to process.
         .   311.03MB     83:	input := bytes.NewBuffer(data)
         .          .     84:
         .          .     85:	// The number of bytes we are looking for.
         .          .     86:	size := len(find)
         .          .     87:
         .          .     88:	// Declare the buffers we need to process the stream.
   14.50MB    14.50MB     89:	buf := make([]byte, size)
         .          .     90:	end := size - 1

What did you see instead?

Notice the weblist view is showing the allocation is flat, which is correct since the call to NewBuffer is inlined. Maybe more important the views are different. Please keep the allocation flat on the report, because it is more accurate with what the compiler chose to do.

@ardan-bkennedy ardan-bkennedy changed the title Discrepency between pprof list and weblist cmd/go/pprof: Discrepency between pprof list and weblist Sep 3, 2018
@agnivade agnivade changed the title cmd/go/pprof: Discrepency between pprof list and weblist cmd/pprof: discrepency between pprof list and weblist Sep 3, 2018
@agnivade agnivade added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 3, 2018
@agnivade agnivade added this to the Go1.12 milestone Sep 3, 2018
@agnivade
Copy link
Contributor

agnivade commented Sep 3, 2018

/cc @hyangah

@hyangah
Copy link
Contributor

hyangah commented Sep 4, 2018

@ardan-bkennedy Thanks for the detailed report. This is a bug in the upstream pprof code. Can you file the issue in https://github.com/google/pprof/issues so it can be tracked and fixed in the upstream?

@ardan-bkennedy
Copy link
Author

I can if you leave the allocation flat, LOLOLOL

I'm doing it now.

@dgryski
Copy link
Contributor

dgryski commented Sep 5, 2018

Upstream issue is google/pprof#415

@agnivade
Copy link
Contributor

@hyangah - Now since the bug is filed upstream, is it necessary to keep this open ? Not sure about the policy on this.

@hyangah
Copy link
Contributor

hyangah commented Sep 19, 2018

Yes, let's close this.

@hyangah hyangah closed this as completed Sep 19, 2018
@golang golang locked and limited conversation to collaborators Sep 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants