From d17f08684718399a3bfd2dcaaea9a8882ddef735 Mon Sep 17 00:00:00 2001 From: "Matt T. Proud" Date: Tue, 22 Aug 2017 23:26:49 +0200 Subject: [PATCH] gomock: fix matcher's degenerate quadratic path. I noticed several tests timing out when attempting to integrate HEAD into our mainline branch. The reason was the accidental introduction of a quadratic call path in callSet.FindMatch, which performed string concatenation in a tight loop. This was corroborated by PProf reporting 23 seconds out of 30 seconds being spent in runtime.concatstrings, which backs onto runtime.memmove. The host I instrumented this on was not the same as the ones for which the data is shown below. The error accumulation routine has been replaced with bytes.Buffer, which exponentially grows its underlying buffer to amortize the cost. After converting a prototype changelist to this pull request you see here, the total runtime of this test that demonstrated the defect fell from 80 seconds to 15. This was just one run on a noisy cluster. This is change introduced the defect: https://github.com/golang/mock/blame/4187d4d04aa043124750c9250259ceafdc5f7380/gomock/callset.go#L68 --- gomock/callset.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/gomock/callset.go b/gomock/callset.go index a88a172f..7f3f6d1d 100644 --- a/gomock/callset.go +++ b/gomock/callset.go @@ -15,6 +15,7 @@ package gomock import ( + "bytes" "errors" "fmt" ) @@ -65,22 +66,22 @@ func (cs callSet) FindMatch(receiver interface{}, method string, args []interfac // Search through the unordered set of calls expected on a method on a // receiver. - callsErrors := "" + var callsErrors bytes.Buffer for _, call := range calls { // A call should not normally still be here if exhausted, // but it can happen if, for instance, .Times(0) was used. // Pretend the call doesn't match. if call.exhausted() { - callsErrors += "\nThe call was exhausted." + callsErrors.WriteString("\nThe call was exhausted.") continue } err := call.matches(args) if err != nil { - callsErrors += "\n" + err.Error() + fmt.Fprintf(&callsErrors, "\n%v", err) } else { return call, nil } } - return nil, fmt.Errorf(callsErrors) + return nil, fmt.Errorf(callsErrors.String()) }