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: c2329cf225dab6de7309af3583daa011bafb9a62
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>

1 files changed, 39 insertions, 17 deletions

diff --git a/src/testing/testing.go b/src/testing/testing.go
index a44c0a0749..d4b108a183 100644
--- a/src/testing/testing.go
+++ b/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
-	output      []byte              // Output generated by test or benchmark.
-	w           io.Writer           // For flushToParent.
-	ran         bool                // Test or benchmark (or one of its subtests) was executed.
-	failed      bool                // Test or benchmark has failed.
-	skipped     bool                // Test of benchmark has been skipped.
-	done        bool                // Test is finished and all subtests have completed.
-	helpers     map[string]struct{} // functions to be skipped when writing file/line info
-	cleanups    []func()            // optional functions to be called at the end of the test
-	cleanupName string              // Name of the cleanup function.
-	cleanupPc   []uintptr           // The stack trace at the point where Cleanup was called.
+	mu          sync.RWMutex         // guards this group of fields
+	output      []byte               // Output generated by test or benchmark.
+	w           io.Writer            // For flushToParent.
+	ran         bool                 // Test or benchmark (or one of its subtests) was executed.
+	failed      bool                 // Test or benchmark has failed.
+	skipped     bool                 // Test of benchmark has been skipped.
+	done        bool                 // Test is finished and all subtests have completed.
+	helperPCs   map[uintptr]struct{} // functions to be skipped when writing file/line info
+	helperNames map[string]struct{}  // helperPCs converted to function names
+	cleanups    []func()             // optional functions to be called at the end of the test
+	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 {
-		c.helpers = make(map[string]struct{})
+	if c.helperPCs == nil {
+		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
 }