Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazily render mock diff output on successful match #8

Merged
merged 4 commits into from
Jun 25, 2024

Conversation

mikeauclair
Copy link

@mikeauclair mikeauclair commented Jun 25, 2024

Summary

  • For mocks on functions that take complex types as arg, Sprintf-ing the actual value can be costly. Switch successful matches to use thunks to lazily render their output, and fully avoid rendering their output when there are no differences since we return a constant string in that case anyways

Changes

  • Switch to a string builder for accumulating output for better performance even when we do render

Motivation

  • We heavily use mocks at DevotedHealth, and use them frequently in gRPC contexts where we're mocking RPCs that take <Whatever>Request structs that are costly to %v via Sprintf. Sprintf showed up as a tangible contributor to runtime performance when profiling some of our heavier tests. The below ~5s cpu time spent on Sprintf goes away completely here in one of our exemplar tests
Screenshot 2024-06-25 at 12 16 35 PM

Related issues

@mikeauclair mikeauclair changed the title wip Lazily render mock diff output on successful match Jun 25, 2024
@@ -923,56 +923,73 @@ func (args Arguments) Is(objects ...interface{}) bool {
return true
}

type outputRenderer interface{}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testify supports go 1.17+ so we'd need to wait for that version to drop before cutting this over to generics

@mikeauclair mikeauclair marked this pull request as ready for review June 25, 2024 16:18
@mikeauclair mikeauclair merged commit 0579227 into master Jun 25, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants