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.

Sign up for GitHub

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

Jump to bottom

runtime/pprof: line directive filename changes results in incorrect and nondeterministic filenames in pprof samples #56135

Open
prattmic opened this issue Oct 10, 2022 · 1 comment
Open

runtime/pprof: line directive filename changes results in incorrect and nondeterministic filenames in pprof samples #56135

prattmic opened this issue Oct 10, 2022 · 1 comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Unplanned

Comments

@prattmic
Copy link
Member 

package main

import (
        "os"
        "runtime/pprof"
        "time"
)

var a, b, c, d int

// noinline doesn't matter, just makes pprof -raw easier to read.
//go:noinline
func add() {
//line a.go:10
        a++
//line b.go:20
        b++
//line c.go:30
        c++
//line d.go:40
        d++
//line line.go:23
}

func main() {
        f, err := os.Create("/tmp/cpu.pprof")
        if err != nil {
                panic(err)
        }
        defer f.Close()

        pprof.StartCPUProfile(f)
        defer pprof.StopCPUProfile()

        start := time.Now()
        for time.Since(start) < 1*time.Second {
                for i := 0; i < 1000; i++ {
                        add()
                }
        }
}

When running this program, all samples in add() will have the same filename, regardless of location. e.g.,

$ pprof -raw /tmp/cpu.pprof 
PeriodType: cpu nanoseconds
Period: 10000000                
Time: 2022-10-10 16:04:43.735472003 -0400 EDT
Duration: 1.10
Samples:                   
samples/count cpu/nanoseconds                  
         10  100000000: 1 2                
         17  170000000: 3 2  
         21  210000000: 4 5 2 
          5   50000000: 6 5 2 
         20  200000000: 7 5 2 
          9   90000000: 8 2 
          4   40000000: 9 2 
          7   70000000: 10 5 2 
          5   50000000: 11 5 2 
          1   10000000: 12 13 14 2 
          1   10000000: 15 2 
Locations
     1: 0x4a09b4 M=1 main.main line.go:38 s=0
     2: 0x435526 M=1 runtime.main /usr/lib/google-golang/src/runtime/proc.go:250 s=0
     3: 0x4a09be M=1 main.main line.go:37 s=0
     4: 0x4a08ae M=1 main.add c.go:30 s=0
     5: 0x4a09b8 M=1 main.main line.go:38 s=0
     6: 0x4a08a0 M=1 main.add c.go:10 s=0
     7: 0x4a08bc M=1 main.add c.go:23 s=0
     8: 0x4a09af M=1 main.main line.go:37 s=0
     9: 0x4a09c1 M=1 main.main line.go:37 s=0
    10: 0x4a08a7 M=1 main.add c.go:20 s=0
    11: 0x4a08b5 M=1 main.add c.go:40 s=0
    12: 0x451e33 M=1 runtime.nanotime /usr/lib/google-golang/src/runtime/time_nofake.go:19 s=0
    13: 0x467ef3 M=1 time.Since /usr/lib/google-golang/src/time/time.go:894 s=0
    14: 0x4a0978 M=1 main.main line.go:36 s=0
    15: 0x4a09c9 M=1 main.main line.go:37 s=0
Mappings
1: 0x400000/0x4a1000/0x0 /tmp/go-build1127057430/b001/exe/line  [FN]

Notice that all Locations in main.add report c.go, even though all of the increments are represented (line numbers 10, 20, 30, 40 are a, b, c, d, respectively).

This is primarily a deficiency in the pprof format itself [1], which encodes the filename at the Function level, thus assuming that a function is entirely in a single file. This is true in the Go language, but //line directives allow specifying multiple fake filenames per function.

This is also nondeterministic because the Go runtime sets Function.filename to the filename of the first frame containing that function. So multiple runs produce profiles with different filenames for add.

This is an unfortunate situation, but I don't think it is currently important enough to do anything about (which would likely involve changing pprof to support multiple filenames per function. (Add an optional filename field to Line, which overrides the value in Function?))

[1] In theory we could use a different Function for samples with different filenames. I suspect this would break some of the grouping/aggregation functionality in pprof though.

cc @golang/runtime @mdempsky @cherrymui
@prattmic prattmic added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 10, 2022
@prattmic prattmic added this to the Backlog milestone Oct 10, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Oct 10, 2022
@gopherbot
Copy link

Change https://go.dev/cl/438255 mentions this issue: runtime/pprof: set Function.start_line field

@mknyszek mknyszek modified the milestones: Backlog, Unplanned Oct 12, 2022
gopherbot pushed a commit that referenced this issue Oct 14, 2022
@prattmic
runtime/pprof: set Function.start_line field
a401468 
Now that we plumb the start line to the runtime, we can include in pprof
files. Since runtime.Frame.startLine is not (currently) exported, we
need a runtime helper to get the value.

For #55022.
Updates #56135.

Change-Id: Ifc5b68a7b7170fd7895e4099deb24df7977b22ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/438255
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Austin Clements <austin@google.com>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
@prattmic @romaindoumenc
runtime/pprof: set Function.start_line field
4ef48e7 
Now that we plumb the start line to the runtime, we can include in pprof
files. Since runtime.Frame.startLine is not (currently) exported, we
need a runtime helper to get the value.

For golang#55022.
Updates golang#56135.

Change-Id: Ifc5b68a7b7170fd7895e4099deb24df7977b22ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/438255
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
3 participants
@mknyszek @prattmic @gopherbot