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

Add a folded stacks output format #669

Closed
wants to merge 2 commits into from
Closed

Conversation

mhansen
Copy link
Contributor

@mhansen mhansen commented Nov 8, 2021

Fixes #658

I'm not sure I've covered everything here; mailing it out early for feedback.

I based this exporter off printTraces, removing the extraneous bits like labels and unit formatting.

I copied the sample mean divisor logic from printTraces.

This uses the sample_index to choose which number to output.

I'm not sure the best way to deal with semicolons or newlines in names. I just removed them for now, although this might lead to colissions. I don't think there's a way to escape them in folded stacks format. Maybe I could just ignore them, and output a corrupted output. That's what a lot of other exporters do. Or say that any profile that contains them is invalid, and return an error. For now, just remove them.

Prior art:

This connects pprof with visualisation tools that accept the folded stack format: https://profilerpedia.markhansen.co.nz/formats/folded-stacks/

These were named 'source' because it initially only tested the
list/source output type. But 'dot' output isn't really a source.

This is just a cleanup preparing for the next commit
@google-cla google-cla bot added the cla: yes label Nov 8, 2021
@mhansen
Copy link
Contributor Author

mhansen commented Nov 8, 2021

Todo: I should add a test case for same stacks different labels and make sure they're aggregated together. I suspect that's not working right now, that instead it'll output duplicate stacks.

@mhansen
Copy link
Contributor Author

mhansen commented Nov 8, 2021

Particularly I'd love some feedback about if there's a different function or something I should use to print the name of the stack. For example, I'd expect -filefunctions to show the stack frame name and the file name, but it doesn't seem to with -format as coded up today.

// TODO: should we print more than just s.Name?
// NodeInfo.PrintableName() has a lot more.

// Remove semicolons and newlines.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a note on why?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Err, didn't mean to click "Add single comment", I'll have more comments!

if i > 0 {
fmt.Fprint(w, ";")
}
// TODO: should we print more than just s.Name?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think this needs more discussion. There is an implicit interplay with the granularity. See below. I would expect that we utilize that - i.e. use PrintableName() or NameComponents() directly defining how they should join. This is in particular important because graph.CreateNodes() respects the granularity and so if we don't encode all components in the string, then something like "-folded -filefunctions" may produce duplicate elements.

$ pprof -top -nodecount=5 -symbolize=none internal/report/testdata/sample.cpu
File: sample.bin
Type: cpu
Time: Sep 29, 2017 at 1:57pm (PDT)
Duration: 2s, Total samples = 1760ms (87.91%)
Showing nodes accounting for 1640ms, 93.18% of 1760ms total
Showing top 5 nodes out of 14
      flat  flat%   sum%        cum   cum%
    1280ms 72.73% 72.73%     1370ms 77.84%  runtime.mapiternext
     150ms  8.52% 81.25%     1760ms   100%  main.busyLoop
     110ms  6.25% 87.50%      110ms  6.25%  math.Abs (inline)
      60ms  3.41% 90.91%       70ms  3.98%  runtime.evacuate
      40ms  2.27% 93.18%       40ms  2.27%  runtime.add (inline)
$ pprof -top -nodecount=5 -symbolize=none -filefunctions internal/report/testdata/sample.cpu
File: sample.bin
Type: cpu
Time: Sep 29, 2017 at 1:57pm (PDT)
Duration: 2s, Total samples = 1760ms (87.91%)
Showing nodes accounting for 1640ms, 93.18% of 1760ms total
Showing top 5 nodes out of 14
      flat  flat%   sum%        cum   cum%
    1280ms 72.73% 72.73%     1370ms 77.84%  runtime.mapiternext /usr/lib/google-golang/src/runtime/hashmap.go
     150ms  8.52% 81.25%     1760ms   100%  main.busyLoop internal/report/testdata/sample/sample.go
     110ms  6.25% 87.50%      110ms  6.25%  math.Abs /usr/lib/google-golang/src/math/abs.go (inline)
      60ms  3.41% 90.91%       70ms  3.98%  runtime.evacuate /usr/lib/google-golang/src/runtime/hashmap.go
      40ms  2.27% 93.18%       40ms  2.27%  runtime.add /usr/lib/google-golang/src/runtime/stubs.go (inline)
$ pprof -top -nodecount=5 -symbolize=none -files internal/report/testdata/sample.cpu
File: sample.bin
Type: cpu
Time: Sep 29, 2017 at 1:57pm (PDT)
Duration: 2s, Total samples = 1760ms (87.91%)
Showing nodes accounting for 1740ms, 98.86% of 1760ms total
Showing top 5 nodes out of 8
      flat  flat%   sum%        cum   cum%
    1400ms 79.55% 79.55%     1450ms 82.39%  /usr/lib/google-golang/src/runtime/hashmap.go
     150ms  8.52% 88.07%     1760ms   100%  internal/report/testdata/sample/sample.go
     110ms  6.25% 94.32%      110ms  6.25%  /usr/lib/google-golang/src/math/abs.go (inline)
      40ms  2.27% 96.59%      130ms  7.39%  /usr/lib/google-golang/src/runtime/hashmap_fast.go
      40ms  2.27% 98.86%       40ms  2.27%  /usr/lib/google-golang/src/runtime/stubs.go (inline)
$ pprof -top -nodecount=5 -symbolize=none -addresses internal/report/testdata/sample.cpu
File: sample.bin
Type: cpu
Time: Sep 29, 2017 at 1:57pm (PDT)
Duration: 2s, Total samples = 1760ms (87.91%)
Showing nodes accounting for 350ms, 19.89% of 1760ms total
Showing top 5 nodes out of 96
      flat  flat%   sum%        cum   cum%
     100ms  5.68%  5.68%      100ms  5.68%  00000000004b4463 math.Abs /usr/lib/google-golang/src/math/abs.go:16
      70ms  3.98%  9.66%       70ms  3.98%  00000000004095d0 runtime.mapiternext /usr/lib/google-golang/src/runtime/hashmap.go:823
      70ms  3.98% 13.64%       70ms  3.98%  00000000004096a2 runtime.mapiternext /usr/lib/google-golang/src/runtime/hashmap.go:856
      60ms  3.41% 17.05%       60ms  3.41%  000000000040957a runtime.mapiternext /usr/lib/google-golang/src/runtime/hashmap.go:819
      50ms  2.84% 19.89%       50ms  2.84%  00000000004095b6 runtime.mapiternext /usr/lib/google-golang/src/runtime/hashmap.go:822

@@ -33,9 +33,12 @@ type testcase struct {
want string
}

func TestSource(t *testing.T) {
func TestTextReports(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have a separate test for the folded report? I find such combined meta-tests somewhat complicated... If this is to share setup / teardown code then maybe we can share it by other means?

@aalexand
Copy link
Collaborator

aalexand commented May 7, 2024

Looks like this PR stalled, closing it. Feel free to re-open if needed.

@aalexand aalexand closed this May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FR: Output Folded Stacks Format
2 participants