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>

3 files changed, 30 insertions, 17 deletions

diff --git a/src/cmd/trace/trace.go b/src/cmd/trace/trace.go
index 1cabc25ced..e6c4cca72e 100644
--- 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 a5fda489be..5da90e0b6d 100644
--- 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 197683bc69..49147ff838 100644
--- 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