testing: reduce memory allocation in Helper

Store the PC instead of the string name of the function, and defer
that conversion until we need it.

Helper is still relatively expensive in CPU time (few hundred ns),
but memory allocation is now constant for a test rather than linear in
the number of times Helper is called.

benchstat:
name        old time/op    new time/op    delta
TBHelper-4    1.30µs ±27%    0.53µs ± 1%   -59.03%  (p=0.008 n=5+5)

name        old alloc/op   new alloc/op   delta
TBHelper-4      216B ± 0%        0B       -100.00%  (p=0.008 n=5+5)

name        old allocs/op  new allocs/op  delta
TBHelper-4      2.00 ± 0%      0.00       -100.00%  (p=0.008 n=5+5)

Change-Id: I6565feb491513815e1058637d086b0374fa94e19
GitHub-Last-Rev: c2329cf225
GitHub-Pull-Request: golang/go#38834
Reviewed-on: https://go-review.googlesource.com/c/go/+/231717
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>

src/testing/testing.go

@@ -384,17 +384,18 @@ const maxStackLen = 50
 // common holds the elements common between T and B and
 // captures common methods such as Errorf.
 type common struct {
-	mu          sync.RWMutex        // guards this group of fields
+	mu          sync.RWMutex         // guards this group of fields
-	output      []byte              // Output generated by test or benchmark.
+	output      []byte               // Output generated by test or benchmark.
-	w           io.Writer           // For flushToParent.
+	w           io.Writer            // For flushToParent.
-	ran         bool                // Test or benchmark (or one of its subtests) was executed.
+	ran         bool                 // Test or benchmark (or one of its subtests) was executed.
-	failed      bool                // Test or benchmark has failed.
+	failed      bool                 // Test or benchmark has failed.
-	skipped     bool                // Test of benchmark has been skipped.
+	skipped     bool                 // Test of benchmark has been skipped.
-	done        bool                // Test is finished and all subtests have completed.
+	done        bool                 // Test is finished and all subtests have completed.
-	helpers     map[string]struct{} // functions to be skipped when writing file/line info
+	helperPCs   map[uintptr]struct{} // functions to be skipped when writing file/line info
-	cleanups    []func()            // optional functions to be called at the end of the test
+	helperNames map[string]struct{}  // helperPCs converted to function names
-	cleanupName string              // Name of the cleanup function.
+	cleanups    []func()             // optional functions to be called at the end of the test
-	cleanupPc   []uintptr           // The stack trace at the point where Cleanup was called.
+	cleanupName string               // Name of the cleanup function.
+	cleanupPc   []uintptr            // The stack trace at the point where Cleanup was called.
 
 	chatty     *chattyPrinter // A copy of chattyPrinter, if the chatty flag is set.
 	bench      bool           // Whether the current test is a benchmark.
@@ -509,7 +510,7 @@ func (c *common) frameSkip(skip int) runtime.Frame {
 			}
 			return prevFrame
 		}
-		if _, ok := c.helpers[frame.Function]; !ok {
+		if _, ok := c.helperNames[frame.Function]; !ok {
 			// Found a frame that wasn't inside a helper function.
 			return frame
 		}
@@ -521,6 +522,14 @@ func (c *common) frameSkip(skip int) runtime.Frame {
 // and inserts the final newline if needed and indentation spaces for formatting.
 // This function must be called with c.mu held.
 func (c *common) decorate(s string, skip int) string {
+	// If more helper PCs have been added since we last did the conversion
+	if c.helperNames == nil {
+		c.helperNames = make(map[string]struct{})
+		for pc := range c.helperPCs {
+			c.helperNames[pcToName(pc)] = struct{}{}
+		}
+	}
+
 	frame := c.frameSkip(skip)
 	file := frame.File
 	line := frame.Line
@@ -853,10 +862,19 @@ func (c *common) Skipped() bool {
 func (c *common) Helper() {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	if c.helpers == nil {
+	if c.helperPCs == nil {
-		c.helpers = make(map[string]struct{})
+		c.helperPCs = make(map[uintptr]struct{})
+	}
+	// repeating code from callerName here to save walking a stack frame
+	var pc [1]uintptr
+	n := runtime.Callers(2, pc[:]) // skip runtime.Callers + Helper
+	if n == 0 {
+		panic("testing: zero callers found")
+	}
+	if _, found := c.helperPCs[pc[0]]; !found {
+		c.helperPCs[pc[0]] = struct{}{}
+		c.helperNames = nil // map will be recreated next time it is needed
 	}
-	c.helpers[callerName(1)] = struct{}{}
 }
 
 // Cleanup registers a function to be called when the test and all its
@@ -995,13 +1013,17 @@ func (c *common) runCleanup(ph panicHandling) (panicVal interface{}) {
 // callerName gives the function name (qualified with a package path)
 // for the caller after skip frames (where 0 means the current function).
 func callerName(skip int) string {
-	// Make room for the skip PC.
 	var pc [1]uintptr
 	n := runtime.Callers(skip+2, pc[:]) // skip + runtime.Callers + callerName
 	if n == 0 {
 		panic("testing: zero callers found")
 	}
-	frames := runtime.CallersFrames(pc[:n])
+	return pcToName(pc[0])
+}
+
+func pcToName(pc uintptr) string {
+	pcs := []uintptr{pc}
+	frames := runtime.CallersFrames(pcs)
 	frame, _ := frames.Next()
 	return frame.Function
 }