internal/trace/v2: store stacks as PCs, not frames

Most stacks share some frames, especially prefixes, and deduplicating
them can save significant amounts of memory.

This will be especially true when we convert traces from the old to the
new format. Here, all stacks exist in a single generation and will be
live together.

For busy traces, such as one of running Staticcheck on std, with CPU
profiling enabled, this change saves ~400 MiB of memory.

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

diff --git a/src/internal/trace/v2/base.go b/src/internal/trace/v2/base.go
index 57e580290263b61e1e70a010662fe2d96941ce67..e2ba09362b95d1f29c5fe4e646981757e3c2a658 100644 (file)
--- a/src/internal/trace/v2/base.go
+++ b/src/internal/trace/v2/base.go
@@ -45,6 +45,7 @@ type evTable struct {
        freq    frequency
        strings dataTable[stringID, string]
        stacks  dataTable[stackID, stack]
+       pcs     map[uint64]frame
 
        // extraStrings are strings that get generated during
        // parsing but haven't come directly from the trace, so
@@ -241,12 +242,12 @@ func (s cpuSample) asEvent(table *evTable) Event {
 
 // stack represents a goroutine stack sample.
 type stack struct {
-       frames []frame
+       pcs []uint64
 }
 
 func (s stack) String() string {
        var sb strings.Builder
-       for _, frame := range s.frames {
+       for _, frame := range s.pcs {
                fmt.Fprintf(&sb, "\t%#v\n", frame)
        }
        return sb.String()

diff --git a/src/internal/trace/v2/event.go b/src/internal/trace/v2/event.go
index 763313c3326943b5432b83b7330ccceda87a9c8c..ec5e27e57a9e90375ff1f21aab58ab49cb791078 100644 (file)
--- a/src/internal/trace/v2/event.go
+++ b/src/internal/trace/v2/event.go
@@ -264,7 +264,8 @@ func (s Stack) Frames(yield func(f StackFrame) bool) bool {
                return true
        }
        stk := s.table.stacks.mustGet(s.id)
-       for _, f := range stk.frames {
+       for _, pc := range stk.pcs {
+               f := s.table.pcs[pc]
                sf := StackFrame{
                        PC:   f.pc,
                        Func: s.table.strings.mustGet(f.funcID),

diff --git a/src/internal/trace/v2/generation.go b/src/internal/trace/v2/generation.go
index 4cdf76e21c06f95463fbff4d819a4f837f23ceac..da31585266571803d2b7de1ddd4fa96d644455e6 100644 (file)
--- a/src/internal/trace/v2/generation.go
+++ b/src/internal/trace/v2/generation.go
@@ -43,7 +43,9 @@ type spilledBatch struct {
 // batch read of the next generation, if any.
 func readGeneration(r *bufio.Reader, spill *spilledBatch) (*generation, *spilledBatch, error) {
        g := &generation{
-               evTable: new(evTable),
+               evTable: &evTable{
+                       pcs: make(map[uint64]frame),
+               },
                batches: make(map[ThreadID][]batch),
        }
        // Process the spilled batch.
@@ -106,7 +108,7 @@ func readGeneration(r *bufio.Reader, spill *spilledBatch) (*generation, *spilled
        g.strings.compactify()
 
        // Validate stacks.
-       if err := validateStackStrings(&g.stacks, &g.strings); err != nil {
+       if err := validateStackStrings(&g.stacks, &g.strings, g.pcs); err != nil {
                return nil, nil, err
        }
 
@@ -130,7 +132,7 @@ func processBatch(g *generation, b batch) error {
                        return err
                }
        case b.isStacksBatch():
-               if err := addStacks(&g.stacks, b); err != nil {
+               if err := addStacks(&g.stacks, g.pcs, b); err != nil {
                        return err
                }
        case b.isCPUSamplesBatch():
@@ -156,11 +158,20 @@ func processBatch(g *generation, b batch) error {
 
 // validateStackStrings makes sure all the string references in
 // the stack table are present in the string table.
-func validateStackStrings(stacks *dataTable[stackID, stack], strings *dataTable[stringID, string]) error {
+func validateStackStrings(
+       stacks *dataTable[stackID, stack],
+       strings *dataTable[stringID, string],
+       frames map[uint64]frame,
+) error {
        var err error
        stacks.forEach(func(id stackID, stk stack) bool {
-               for _, frame := range stk.frames {
-                       _, ok := strings.get(frame.funcID)
+               for _, pc := range stk.pcs {
+                       frame, ok := frames[pc]
+                       if !ok {
+                               err = fmt.Errorf("found unknown pc %x for stack %d", pc, id)
+                               return false
+                       }
+                       _, ok = strings.get(frame.funcID)
                        if !ok {
                                err = fmt.Errorf("found invalid func string ID %d for stack %d", frame.funcID, id)
                                return false
@@ -237,7 +248,7 @@ func addStrings(stringTable *dataTable[stringID, string], b batch) error {
 // addStacks takes a batch whose first byte is an EvStacks event
 // (indicating that the batch contains only stacks) and adds each
 // string contained therein to the provided stacks map.
-func addStacks(stackTable *dataTable[stackID, stack], b batch) error {
+func addStacks(stackTable *dataTable[stackID, stack], pcs map[uint64]frame, b batch) error {
        if !b.isStacksBatch() {
                return fmt.Errorf("internal error: addStacks called on non-stacks batch")
        }
@@ -273,7 +284,7 @@ func addStacks(stackTable *dataTable[stackID, stack], b batch) error {
                }
 
                // Each frame consists of 4 fields: pc, funcID (string), fileID (string), line.
-               frames := make([]frame, 0, nFrames)
+               frames := make([]uint64, 0, nFrames)
                for i := uint64(0); i < nFrames; i++ {
                        // Read the frame data.
                        pc, err := binary.ReadUvarint(r)
@@ -292,16 +303,20 @@ func addStacks(stackTable *dataTable[stackID, stack], b batch) error {
                        if err != nil {
                                return fmt.Errorf("reading frame %d's line for stack %d: %w", i+1, id, err)
                        }
-                       frames = append(frames, frame{
-                               pc:     pc,
-                               funcID: stringID(funcID),
-                               fileID: stringID(fileID),
-                               line:   line,
-                       })
+                       frames = append(frames, pc)
+
+                       if _, ok := pcs[pc]; !ok {
+                               pcs[pc] = frame{
+                                       pc:     pc,
+                                       funcID: stringID(funcID),
+                                       fileID: stringID(fileID),
+                                       line:   line,
+                               }
+                       }
                }
 
                // Add the stack to the map.
-               if err := stackTable.insert(stackID(id), stack{frames: frames}); err != nil {
+               if err := stackTable.insert(stackID(id), stack{pcs: frames}); err != nil {
                        return err
                }
        }