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: forces choice between "inuse" and "alloc" heap profiles #16892

Closed
rhysh opened this issue Aug 26, 2016 · 5 comments
Closed

cmd/pprof: forces choice between "inuse" and "alloc" heap profiles #16892

rhysh opened this issue Aug 26, 2016 · 5 comments
Milestone

Comments

@rhysh
Copy link
Contributor

@rhysh rhysh commented Aug 26, 2016

Hello! I've got a bug with go tool pprof's proto format that forces a choice between "inuse" and "alloc" modes (assuming "inuse" by default), which means I've got to capture two separate profiles or use the standalone google/pprof tool.

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

    $ go version
    go version devel +8f3c8a3 Fri Aug 26 20:06:58 2016 +0000 darwin/amd64
    $ go1.7 version
    go version go1.7 darwin/amd64
    
  2. What operating system and processor architecture are you using (go env)?

    darwin/amd64, though I've also observed this on linux/amd64.

  3. What did you do?

    First, I run a trivial http server with pprof enabled. Then I point go tool pprof at it, asking the command to save a gzipped-protobuf-formatted heap profile. I then try to read that profile in various modes: inuse_space and alloc_space

  4. What did you expect to see?

    I expected a single saved heap profile to be readable in all four modes (inuse_space, inuse_objects, alloc_space, alloc_objects). I expected this because the other pprof tool can do this, and because the text file served from /debug/pprof/heap can be interpreted in all four modes.

  5. What did you see instead?

    A heap profile saved with go tool pprof -proto -inuse_space -output ./file http://... cannot be read with go tool pprof -alloc_space ./file, and vice versa.

main.go:

package main

import (
    "net/http"
    _ "net/http/pprof"
)

var sink []byte

func main() {
    for i := 0; i < 10; i++ {
        sink = make([]byte, 1<<20)
    }
    panic(http.ListenAndServe("127.0.0.1:8080", nil))
}

test.sh:

#!/usr/bin/env bash

go version

go build -o ./server ./main.go
./server &
server="$!"
sleep 1

for tool in "go tool pprof" "pprof"; do
    for record_style in "" "-inuse_space" "-alloc_space"; do
        $tool $record_style -proto -symbolize=remote -output ./heap.pb.gz http://127.0.0.1:8080/debug/pprof/heap &>/dev/null
        for view_style in "-inuse_space" "-alloc_space"; do
            $tool $view_style -top ./heap.pb.gz &>/dev/null || printf "failed tool=%q record=%q view=%q\n" "$tool" "$record_style" "$view_style"
        done
        rm ./heap.pb.gz
    done
done

kill -KILL -- "$server"
rm ./server
$ ./test.sh 
go version devel +8f3c8a3 Fri Aug 26 20:06:58 2016 +0000 darwin/amd64
failed tool=go\ tool\ pprof record='' view=-alloc_space
failed tool=go\ tool\ pprof record=-inuse_space view=-alloc_space
failed tool=go\ tool\ pprof record=-alloc_space view=-inuse_space
$ PATH="$HOME/go1.7/bin:$PATH" ./test.sh 
go version go1.7 darwin/amd64
failed tool=go\ tool\ pprof record='' view=-alloc_space
failed tool=go\ tool\ pprof record=-inuse_space view=-alloc_space
failed tool=go\ tool\ pprof record=-alloc_space view=-inuse_space
@aclements
Copy link
Member

@aclements aclements commented Aug 30, 2016

@rauls5382
Copy link
Contributor

@rauls5382 rauls5382 commented Sep 3, 2016

I agree this is a problem. Earlier versions of pprof would select between alloc/inuse before saving the profile on a proto. That was later changed to save all the data in the proto.

go tool pprof is based on an older version of pprof, which does not have that fix. At some point we will refresh the tool to use the new version. I already have that in my queue under #16184

In the meantime, I recommend using the standalone pprof to collect the profiles.

@quentinmit
Copy link
Contributor

@quentinmit quentinmit commented Sep 6, 2016

Closing as dupe of #16184

@quentinmit quentinmit closed this Sep 6, 2016
@rsc
Copy link
Contributor

@rsc rsc commented Nov 1, 2016

@rauls5382, we are probably not going to sync upstream pprof into pprof for Go 1.8 - too late. But we may still get to generating the binary pprof format by default from Go's profile handlers. Was the bug that discarded data in the legacy->binary conversion or was it still in binary->binary saving? If it's just in legacy->binary, then we can ignore it assuming we get direct binary generation working. If it's in the binary->binary saving too then we might want to grab that specific change. Thanks.

@rsc
Copy link
Contributor

@rsc rsc commented Nov 3, 2016

Let's call this a dup of #17764.

It looks like the initial checkin to github.com/google/pprof has a very different copy of profile/legacy_profile.go, so I can't find out when the fix in question was committed or what it was. That said, let's just work on getting the binary form generated, which will solve this problem.

@rsc rsc closed this Nov 3, 2016
@golang golang locked and limited conversation to collaborators Nov 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.