cmd/trace,internal/trace,runtime: refactor to access frames via range over func

Change-Id: Id0be0eb35ae8560bd5338ec296a086aaf4617db0
Reviewed-on: https://go-review.googlesource.com/c/go/+/608856
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>

diff --git a/src/cmd/trace/gstate.go b/src/cmd/trace/gstate.go
index 76c58073b33e853952d84eb46f66e9eb43e4a190..ea501ef57d561e4dafc4d839a7c4e457202652cf 100644 (file)
--- a/src/cmd/trace/gstate.go
+++ b/src/cmd/trace/gstate.go
@@ -362,11 +362,9 @@ func (gs *gState[R]) rangeEnd(ts trace.Time, name string, stack trace.Stack, ctx
        delete(gs.activeRanges, name)
 }
 
-func lastFunc(s trace.Stack) string {
-       var last trace.StackFrame
-       s.Frames()(func(f trace.StackFrame) bool {
-               last = f
-               return true
-       })
-       return last.Func
+func lastFunc(s trace.Stack) (fn string) {
+       for frame := range s.Frames() {
+               fn = frame.Func
+       }
+       return
 }

diff --git a/src/cmd/trace/pprof.go b/src/cmd/trace/pprof.go
index 856b97b75fed9d6d847ed99046d26f8e733084e2..d27dfa7aa331001a65d1c3d76ba0c78b973dd610 100644 (file)
--- a/src/cmd/trace/pprof.go
+++ b/src/cmd/trace/pprof.go
@@ -306,19 +306,19 @@ func (m *stackMap) profile() []traceviewer.ProfileRecord {
        prof := make([]traceviewer.ProfileRecord, 0, len(m.stacks))
        for stack, record := range m.stacks {
                rec := *record
-               i := 0
-               stack.Frames()(func(frame trace.StackFrame) bool {
+               for i, frame := range slices.Collect(stack.Frames()) {
                        rec.Stack = append(rec.Stack, &trace.Frame{
                                PC:   frame.PC,
                                Fn:   frame.Func,
                                File: frame.File,
                                Line: int(frame.Line),
                        })
-                       i++
                        // Cut this off at pprofMaxStack because that's as far
                        // as our deduplication goes.
-                       return i < pprofMaxStack
-               })
+                       if i >= pprofMaxStack {
+                               break
+                       }
+               }
                prof = append(prof, rec)
        }
        return prof
@@ -326,10 +326,10 @@ func (m *stackMap) profile() []traceviewer.ProfileRecord {
 
 // pcsForStack extracts the first pprofMaxStack PCs from stack into pcs.
 func pcsForStack(stack trace.Stack, pcs *[pprofMaxStack]uint64) {
-       i := 0
-       stack.Frames()(func(frame trace.StackFrame) bool {
+       for i, frame := range slices.Collect(stack.Frames()) {
                pcs[i] = frame.PC
-               i++
-               return i < len(pcs)
-       })
+               if i >= len(pcs) {
+                       break
+               }
+       }
 }

diff --git a/src/cmd/trace/regions.go b/src/cmd/trace/regions.go
index cae38355a58019af943d7a74b2cf501e80d9d1e5..4073b6b07d472153a2d191cec1b0cf2a28f173a5 100644 (file)
--- a/src/cmd/trace/regions.go
+++ b/src/cmd/trace/regions.go
@@ -72,10 +72,9 @@ func fingerprintRegion(r *trace.UserRegionSummary) regionFingerprint {
 func regionTopStackFrame(r *trace.UserRegionSummary) trace.StackFrame {
        var frame trace.StackFrame
        if r.Start != nil && r.Start.Stack() != trace.NoStack {
-               r.Start.Stack().Frames()(func(f trace.StackFrame) bool {
+               for f := range r.Start.Stack().Frames() {
                        frame = f
-                       return false
-               })
+               }
        }
        return frame
 }

diff --git a/src/cmd/trace/viewer.go b/src/cmd/trace/viewer.go
index c367c7d6366b5fe1614f1debb978f30e5d9ce8f5..6ce74b75b8dbbad40c7fac07dc5844236f3b80ea 100644 (file)
--- a/src/cmd/trace/viewer.go
+++ b/src/cmd/trace/viewer.go
@@ -15,15 +15,14 @@ import (
 // used to store the frames to reduce allocations.
 func viewerFrames(stk trace.Stack) []*trace.Frame {
        var frames []*trace.Frame
-       stk.Frames()(func(f trace.StackFrame) bool {
+       for f := range stk.Frames() {
                frames = append(frames, &trace.Frame{
                        PC:   f.PC,
                        Fn:   f.Func,
                        File: f.File,
                        Line: int(f.Line),
                })
-               return true
-       })
+       }
        return frames
 }
 

diff --git a/src/internal/trace/event.go b/src/internal/trace/event.go
index 90effce65336aff667583eeed4770fccba083e83..a5c5aec2f8a8babb5d62c46a3b6f92a2aeb5d60f 100644 (file)
--- a/src/internal/trace/event.go
+++ b/src/internal/trace/event.go
@@ -798,11 +798,10 @@ func (e Event) String() string {
                if s.Stack != NoStack {
                        fmt.Fprintln(&sb)
                        fmt.Fprintln(&sb, "TransitionStack=")
-                       s.Stack.Frames()(func(f StackFrame) bool {
+                       for f := range s.Stack.Frames() {
                                fmt.Fprintf(&sb, "\t%s @ 0x%x\n", f.Func, f.PC)
                                fmt.Fprintf(&sb, "\t\t%s:%d\n", f.File, f.Line)
-                               return true
-                       })
+                       }
                }
        case EventExperimental:
                r := e.Experimental()
@@ -811,11 +810,10 @@ func (e Event) String() string {
        if stk := e.Stack(); stk != NoStack {
                fmt.Fprintln(&sb)
                fmt.Fprintln(&sb, "Stack=")
-               stk.Frames()(func(f StackFrame) bool {
+               for f := range stk.Frames() {
                        fmt.Fprintf(&sb, "\t%s @ 0x%x\n", f.Func, f.PC)
                        fmt.Fprintf(&sb, "\t\t%s:%d\n", f.File, f.Line)
-                       return true
-               })
+               }
        }
        return sb.String()
 }

diff --git a/src/internal/trace/summary.go b/src/internal/trace/summary.go
index 1cd506ac5a7cf6b6697014e378132144f076350e..f31439feb8ff0b1efce565bc5470e2ccf144e232 100644 (file)
--- a/src/internal/trace/summary.go
+++ b/src/internal/trace/summary.go
@@ -390,24 +390,14 @@ func (s *Summarizer) Event(ev *Event) {
                        // This root frame will be identical for all transitions on this
                        // goroutine, because it represents its immutable start point.
                        if g.Name == "" {
-                               stk := st.Stack
-                               if stk != NoStack {
-                                       var frame StackFrame
-                                       var ok bool
-                                       stk.Frames()(func(f StackFrame) bool {
-                                               frame = f
-                                               ok = true
-                                               return true
-                                       })
-                                       if ok {
-                                               // NB: this PC won't actually be consistent for
-                                               // goroutines which existed at the start of the
-                                               // trace. The UI doesn't use it directly; this
-                                               // mainly serves as an indication that we
-                                               // actually saw a call stack for the goroutine
-                                               g.PC = frame.PC
-                                               g.Name = frame.Func
-                                       }
+                               for frame := range st.Stack.Frames() {
+                                       // NB: this PC won't actually be consistent for
+                                       // goroutines which existed at the start of the
+                                       // trace. The UI doesn't use it directly; this
+                                       // mainly serves as an indication that we
+                                       // actually saw a call stack for the goroutine
+                                       g.PC = frame.PC
+                                       g.Name = frame.Func
                                }
                        }
 

diff --git a/src/internal/trace/testtrace/validation.go b/src/internal/trace/testtrace/validation.go
index 42a561f1f033d09101021eec0dcca578786162d5..59ff19e6106e9b319e5864b8b6ba3f693f5f83ae 100644 (file)
--- a/src/internal/trace/testtrace/validation.go
+++ b/src/internal/trace/testtrace/validation.go
@@ -350,20 +350,17 @@ func (v *Validator) getOrCreateThread(e *errAccumulator, ev trace.Event, m trace
 
 func checkStack(e *errAccumulator, stk trace.Stack) {
        // Check for non-empty values, but we also check for crashes due to incorrect validation.
-       i := 0
-       stk.Frames()(func(f trace.StackFrame) bool {
+       for i, f := range slices.Collect(stk.Frames()) {
                if i == 0 {
                        // Allow for one fully zero stack.
                        //
                        // TODO(mknyszek): Investigate why that happens.
-                       return true
+                       continue
                }
                if f.Func == "" || f.File == "" || f.PC == 0 || f.Line == 0 {
                        e.Errorf("invalid stack frame %#v: missing information", f)
                }
-               i++
-               return true
-       })
+       }
 }
 
 type errAccumulator struct {

diff --git a/src/internal/trace/trace_test.go b/src/internal/trace/trace_test.go
index dcf9d05fb4e2b886a472708034a6979774bb40d0..facac47eef06049f2d87f3b9d07088f427c17699 100644 (file)
--- a/src/internal/trace/trace_test.go
+++ b/src/internal/trace/trace_test.go
@@ -16,6 +16,7 @@ import (
        "os"
        "path/filepath"
        "runtime"
+       "slices"
        "strings"
        "testing"
 )
@@ -148,12 +149,11 @@ func TestTraceCPUProfile(t *testing.T) {
                                if hogRegion != nil && ev.Goroutine() == hogRegion.Goroutine() {
                                        traceSamples++
                                        var fns []string
-                                       ev.Stack().Frames()(func(frame trace.StackFrame) bool {
+                                       for frame := range ev.Stack().Frames() {
                                                if frame.Func != "runtime.goexit" {
                                                        fns = append(fns, fmt.Sprintf("%s:%d", frame.Func, frame.Line))
                                                }
-                                               return true
-                                       })
+                                       }
                                        stack := strings.Join(fns, "|")
                                        traceStacks[stack]++
                                }
@@ -436,21 +436,15 @@ func TestTraceStacks(t *testing.T) {
                        }...)
                }
                stackMatches := func(stk trace.Stack, frames []frame) bool {
-                       i := 0
-                       match := true
-                       stk.Frames()(func(f trace.StackFrame) bool {
+                       for i, f := range slices.Collect(stk.Frames()) {
                                if f.Func != frames[i].fn {
-                                       match = false
                                        return false
                                }
                                if line := uint64(frames[i].line); line != 0 && line != f.Line {
-                                       match = false
                                        return false
                                }
-                               i++
-                               return true
-                       })
-                       return match
+                       }
+                       return true
                }
                r, err := trace.NewReader(bytes.NewReader(tb))
                if err != nil {

diff --git a/src/runtime/trace_cgo_test.go b/src/runtime/trace_cgo_test.go
index 28b6f7833ee3eacab2271e4a6c70b0dbde0f57a1..871698f8b4886b7c3eaf729d1acac7bef9136b06 100644 (file)
--- a/src/runtime/trace_cgo_test.go
+++ b/src/runtime/trace_cgo_test.go
@@ -94,10 +94,9 @@ func mustFindLogV2(t *testing.T, trc io.Reader, category string) trace.Event {
 // dumpStack returns e.Stack() as a string.
 func dumpStackV2(e *trace.Event) string {
        var buf bytes.Buffer
-       e.Stack().Frames()(func(f trace.StackFrame) bool {
+       for f := range e.Stack().Frames() {
                file := strings.TrimPrefix(f.File, runtime.GOROOT())
                fmt.Fprintf(&buf, "%s\n\t%s:%d\n", f.Func, file, f.Line)
-               return true
-       })
+       }
        return buf.String()
 }