internal/trace: refactor Stack.Frames to return iter.Seq

The Frames function is almost an iter.Seq, except for its bool return
value.

Since none of the callers in the Go tree rely on the bool, we can remove
it. However, doing so might still obscure the intended usage as an iterator.

This refactor changes the API to return iter.Seq, making the intended
usage explicit. Refactoring the existing callers to take advantage of
the new interface will be done in a follow-up CL.

Change-Id: I03e4d6d762910e418cc37d59a6c519eb7f39b3b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/608855
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>
This commit is contained in:
Felix Geisendörfer 2024-08-27 20:46:33 +02:00 committed by Michael Knyszek
parent cfbd2e7b40
commit 89a5a60da6
9 changed files with 29 additions and 27 deletions

View File

@ -364,7 +364,7 @@ func (gs *gState[R]) rangeEnd(ts trace.Time, name string, stack trace.Stack, ctx
func lastFunc(s trace.Stack) string {
var last trace.StackFrame
s.Frames(func(f trace.StackFrame) bool {
s.Frames()(func(f trace.StackFrame) bool {
last = f
return true
})

View File

@ -307,7 +307,7 @@ func (m *stackMap) profile() []traceviewer.ProfileRecord {
for stack, record := range m.stacks {
rec := *record
i := 0
stack.Frames(func(frame trace.StackFrame) bool {
stack.Frames()(func(frame trace.StackFrame) bool {
rec.Stack = append(rec.Stack, &trace.Frame{
PC: frame.PC,
Fn: frame.Func,
@ -327,7 +327,7 @@ 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 {
stack.Frames()(func(frame trace.StackFrame) bool {
pcs[i] = frame.PC
i++
return i < len(pcs)

View File

@ -72,7 +72,7 @@ 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 {
r.Start.Stack().Frames()(func(f trace.StackFrame) bool {
frame = f
return false
})

View File

@ -15,7 +15,7 @@ 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 {
stk.Frames()(func(f trace.StackFrame) bool {
frames = append(frames, &trace.Frame{
PC: f.PC,
Fn: f.Func,

View File

@ -6,6 +6,7 @@ package trace
import (
"fmt"
"iter"
"math"
"strings"
"time"
@ -265,24 +266,25 @@ type Stack struct {
}
// Frames is an iterator over the frames in a Stack.
func (s Stack) Frames(yield func(f StackFrame) bool) bool {
if s.id == 0 {
return true
}
stk := s.table.stacks.mustGet(s.id)
for _, pc := range stk.pcs {
f := s.table.pcs[pc]
sf := StackFrame{
PC: f.pc,
Func: s.table.strings.mustGet(f.funcID),
File: s.table.strings.mustGet(f.fileID),
Line: f.line,
func (s Stack) Frames() iter.Seq[StackFrame] {
return func(yield func(StackFrame) bool) {
if s.id == 0 {
return
}
if !yield(sf) {
return false
stk := s.table.stacks.mustGet(s.id)
for _, pc := range stk.pcs {
f := s.table.pcs[pc]
sf := StackFrame{
PC: f.pc,
Func: s.table.strings.mustGet(f.funcID),
File: s.table.strings.mustGet(f.fileID),
Line: f.line,
}
if !yield(sf) {
return
}
}
}
return true
}
// NoStack is a sentinel value that can be compared against any Stack value, indicating
@ -796,7 +798,7 @@ func (e Event) String() string {
if s.Stack != NoStack {
fmt.Fprintln(&sb)
fmt.Fprintln(&sb, "TransitionStack=")
s.Stack.Frames(func(f StackFrame) bool {
s.Stack.Frames()(func(f StackFrame) bool {
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
@ -809,7 +811,7 @@ func (e Event) String() string {
if stk := e.Stack(); stk != NoStack {
fmt.Fprintln(&sb)
fmt.Fprintln(&sb, "Stack=")
stk.Frames(func(f StackFrame) bool {
stk.Frames()(func(f StackFrame) bool {
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

View File

@ -394,7 +394,7 @@ func (s *Summarizer) Event(ev *Event) {
if stk != NoStack {
var frame StackFrame
var ok bool
stk.Frames(func(f StackFrame) bool {
stk.Frames()(func(f StackFrame) bool {
frame = f
ok = true
return true

View File

@ -351,7 +351,7 @@ 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 {
stk.Frames()(func(f trace.StackFrame) bool {
if i == 0 {
// Allow for one fully zero stack.
//

View File

@ -148,7 +148,7 @@ func TestTraceCPUProfile(t *testing.T) {
if hogRegion != nil && ev.Goroutine() == hogRegion.Goroutine() {
traceSamples++
var fns []string
ev.Stack().Frames(func(frame trace.StackFrame) bool {
ev.Stack().Frames()(func(frame trace.StackFrame) bool {
if frame.Func != "runtime.goexit" {
fns = append(fns, fmt.Sprintf("%s:%d", frame.Func, frame.Line))
}
@ -438,7 +438,7 @@ func TestTraceStacks(t *testing.T) {
stackMatches := func(stk trace.Stack, frames []frame) bool {
i := 0
match := true
stk.Frames(func(f trace.StackFrame) bool {
stk.Frames()(func(f trace.StackFrame) bool {
if f.Func != frames[i].fn {
match = false
return false

View File

@ -94,7 +94,7 @@ 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 {
e.Stack().Frames()(func(f trace.StackFrame) bool {
file := strings.TrimPrefix(f.File, runtime.GOROOT())
fmt.Fprintf(&buf, "%s\n\t%s:%d\n", f.Func, file, f.Line)
return true