Skip to content
This repository was archived by the owner on Jun 27, 2023. It is now read-only.

gomock: fix matcher's degenerate quadratic path.#103

Merged
balshetzer merged 1 commit intogolang:masterfrom
matttproud:fix/quadratic-reporting-cost
Aug 22, 2017
Merged

gomock: fix matcher's degenerate quadratic path.#103
balshetzer merged 1 commit intogolang:masterfrom
matttproud:fix/quadratic-reporting-cost

Conversation

@matttproud
Copy link
Copy Markdown
Contributor

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

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
@balshetzer balshetzer merged commit 18f6dd1 into golang:master Aug 22, 2017
@matttproud matttproud deleted the fix/quadratic-reporting-cost branch August 25, 2017 12:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants