internal/trace: don't report regions on system goroutines

If a goroutine is started within a user region, internal/trace assigns
the child goroutine a nameless region for its entire lifetime which is
assosciated the same task as the parent's region.

This is not strictly necessary: a child goroutine is not necessarily
related to the task unless it performs some task operation (in which
case it will be associated with the task through the standard means).

However, it can be quite handy to see child goroutines within a region,
which may be child worker goroutines that you simply didn't perform task
operations on.

If the first GC occurs during a region, the GC worker goroutines will
also inherit a child region. We know for sure that these aren't related
to the task, so filter them out from the region list.

Note that we can't exclude system goroutines from setting activeRegions
in EvGoCreate handling, because we don't know the goroutine start
function name until the first EvGoStart.

Fixes #53784.

Change-Id: Ic83d84e23858a8400a76d1ae2f1418ef49951178
Reviewed-on: https://go-review.googlesource.com/c/go/+/416858
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>

diff --git a/src/cmd/trace/trace.go b/src/cmd/trace/trace.go
index 1cabc25cedc72901d6af75dca416b4b392f7e9c2..e6c4cca72e00601193727c7d686f18f2ff8467e0 100644 (file)
--- a/src/cmd/trace/trace.go
+++ b/src/cmd/trace/trace.go
@@ -571,7 +571,7 @@ func generateTrace(params *traceParams, consumer traceConsumer) error {
 
                        fname := stk[0].Fn
                        info.name = fmt.Sprintf("G%v %s", newG, fname)
-                       info.isSystemG = isSystemGoroutine(fname)
+                       info.isSystemG = trace.IsSystemGoroutine(fname)
 
                        ctx.gcount++
                        setGState(ev, newG, gDead, gRunnable)
@@ -1129,12 +1129,6 @@ func (ctx *traceContext) buildBranch(parent frameNode, stk []*trace.Frame) int {
        return ctx.buildBranch(node, stk)
 }
 
-func isSystemGoroutine(entryFn string) bool {
-       // This mimics runtime.isSystemGoroutine as closely as
-       // possible.
-       return entryFn != "runtime.main" && strings.HasPrefix(entryFn, "runtime.")
-}
-
 // firstTimestamp returns the timestamp of the first event record.
 func firstTimestamp() int64 {
        res, _ := parseTrace()

diff --git a/src/internal/trace/goroutines.go b/src/internal/trace/goroutines.go
index a5fda489bea7980610bd7e9d52e31dc5eab645df..5da90e0b6d83487c0d2bed337e80bd8855df345e 100644 (file)
--- a/src/internal/trace/goroutines.go
+++ b/src/internal/trace/goroutines.go
@@ -4,7 +4,10 @@
 
 package trace
 
-import "sort"
+import (
+       "sort"
+       "strings"
+)
 
 // GDesc contains statistics and execution details of a single goroutine.
 type GDesc struct {
@@ -126,10 +129,17 @@ func (g *GDesc) finalize(lastTs, activeGCStartTime int64, trigger *Event) {
        finalStat := g.snapshotStat(lastTs, activeGCStartTime)
 
        g.GExecutionStat = finalStat
-       for _, s := range g.activeRegions {
-               s.End = trigger
-               s.GExecutionStat = finalStat.sub(s.GExecutionStat)
-               g.Regions = append(g.Regions, s)
+
+       // System goroutines are never part of regions, even though they
+       // "inherit" a task due to creation (EvGoCreate) from within a region.
+       // This may happen e.g. if the first GC is triggered within a region,
+       // starting the GC worker goroutines.
+       if !IsSystemGoroutine(g.Name) {
+               for _, s := range g.activeRegions {
+                       s.End = trigger
+                       s.GExecutionStat = finalStat.sub(s.GExecutionStat)
+                       g.Regions = append(g.Regions, s)
+               }
        }
        *(g.gdesc) = gdesc{}
 }
@@ -158,10 +168,13 @@ func GoroutineStats(events []*Event) map[uint64]*GDesc {
                case EvGoCreate:
                        g := &GDesc{ID: ev.Args[0], CreationTime: ev.Ts, gdesc: new(gdesc)}
                        g.blockSchedTime = ev.Ts
-                       // When a goroutine is newly created, inherit the
-                       // task of the active region. For ease handling of
-                       // this case, we create a fake region description with
-                       // the task id.
+                       // When a goroutine is newly created, inherit the task
+                       // of the active region. For ease handling of this
+                       // case, we create a fake region description with the
+                       // task id. This isn't strictly necessary as this
+                       // goroutine may not be assosciated with the task, but
+                       // it can be convenient to see all children created
+                       // during a region.
                        if creatorG := gs[ev.G]; creatorG != nil && len(creatorG.gdesc.activeRegions) > 0 {
                                regions := creatorG.gdesc.activeRegions
                                s := regions[len(regions)-1]
@@ -336,3 +349,9 @@ func RelatedGoroutines(events []*Event, goid uint64) map[uint64]bool {
        gmap[0] = true // for GC events
        return gmap
 }
+
+func IsSystemGoroutine(entryFn string) bool {
+       // This mimics runtime.isSystemGoroutine as closely as
+       // possible.
+       return entryFn != "runtime.main" && strings.HasPrefix(entryFn, "runtime.")
+}

diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go
index 197683bc69353af74bd1b2078b32141d80961386..49147ff8381e3b45cea2f51cfc9792a3b90fcd08 100644 (file)
--- a/src/runtime/traceback.go
+++ b/src/runtime/traceback.go
@@ -1120,7 +1120,7 @@ func tracebackHexdump(stk stack, frame *stkframe, bad uintptr) {
 // system (that is, the finalizer goroutine) is considered a user
 // goroutine.
 func isSystemGoroutine(gp *g, fixed bool) bool {
-       // Keep this in sync with cmd/trace/trace.go:isSystemGoroutine.
+       // Keep this in sync with internal/trace.IsSystemGoroutine.
        f := findfunc(gp.startpc)
        if !f.valid() {
                return false