-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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: list command only respecting no inlining output #35143
Comments
On tip today I noticed the weblist view was taking into account inlining. This was great to see. The console list command was not. Not sure this was intended or not? |
We fixed inlined function info encoding in the generated profile data recently. I expected that fixed this bug (but left this open to verify before closing) but, based on your lat comment, I guess something is still broken. What do you mean by the console list command? This is what I got now. $ go tool pprof p.out File: memcpu.test Type: alloc_space Time: Nov 12, 2019 at 10:23pm (EST) Entering interactive mode (type "help" for commands, "o" for options) (pprof) list lgOne Total: 127.01MB ROUTINE ======================== github.com/ardanlabs/gotraining/topics/go/profiling/memcpu.algOne in /tmp/gotraining/topics/go/profiling/memcpu/stream.go 13MB 127.01MB (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. . 114.01MB 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. 13MB 13MB 89: 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]) |
The allocation on line 83 should be flat. Now on tip when using |
|
can you try the (pprof) noinlines (pprof) list algOne Total: 127.01MB ROUTINE ======================== github.com/ardanlabs/gotraining/topics/go/profiling/memcpu.algOne in /tmp/gotraining/topics/go/profiling/memcpu/stream.go 127.01MB 127.01MB (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. 114.01MB 114.01MB 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. 13MB 13MB 89: 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]) |
I will try but this is confusing. It's a flat allocation because of inlining. Using the -noinlines flag to see inlining seems odd? Using the -noinlines switch is giving me the view with the inlining being taken into account. Which is great! At least I can now turn that on when I want to see the inlining optimization represented! |
Is it ok to close this bug? ps. reading the referenced google/pprof bug, we had the exactly same discussion about the difference between weblist and list. |
Yes. THANK YOU for making this switch toggle the two different views. I really appreciate the effort. But we aware that |
The correct answer for the bytes.NewBuffer call is 0 flat, 114MB cum. The
allocation is *inside* bytes.NewBuffer.
(In contrast to what we show today, which is IMO wrong.)
The intent of all of these inlining fixes is to make it so the output
doesn't depend on whether inlining happened or not.
It should appear as you read the source. You should see the 114MB flat line
if you list bytes.NewBuffer.
…On Tue, Nov 12, 2019 at 8:04 PM William Kennedy ***@***.***> wrote:
I will try but this is confusing. It's a flat allocation because of
inlining. Using the -noinlines flag to see inlining seems odd?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#35143?email_source=notifications&email_token=ABUSAIBMGMSEN64IHYXZEO3QTN4DXA5CNFSM4JE2XAFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOED4ZU4A#issuecomment-553228912>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABUSAIE4L3ETSTZLJGIL343QTN4DXANCNFSM4JE2XAFA>
.
|
@randall77 The allocation is caused by the lines calling into io.ReadFull. The escape analysis report will show line 113 as the cause. If the inlining optimization is taken into account, then line 113 is the cause. If the inline optimization does not take place, then it is line 83. if I am trying to remove this allocation I need to deal with the io.Read calls, not the call to NewBuffer. |
Before closing this bug, I want to clarify: @ardan-bkennedy We didn't make any pprof command line tool change. The pprof command line tool features are maintained by the upstream pprof team. We fixed a bug in runtime/pprof that caused missing inline info in generated profiles (regression undetected in the last couple of release cycles). For pprof feature improvement requests, please file requests to upstream. @randall77 We still show 0 flat, 114MB cum by default - which I also think more appropriate as the default. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
With 1.13 and tip
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
Using this code
https://github.com/ardanlabs/gotraining/tree/master/topics/go/profiling/memcpu
I ran the following command
What did you expect to see?
What did you see instead?
Why?
The list command only shows output not taking into account the inling optimization. I need to see the list output with inlining. There is a command flag called
-noinlines
but that doesn't help me since it is the default mode.I have reported this several times in the past. There has been some discussion about flags. I think this has been lost. I could not find any open issue related to this.
The text was updated successfully, but these errors were encountered: