-
-
Notifications
You must be signed in to change notification settings - Fork 357
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
Reduce recorder memory usage #1479
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvement, thank you
Hello, @pirj. Any chance we're shipping this soon? |
Can you construct a benchmark for this? It looks harmles but I'm also confused why it saves memory, its essentially the same code except for using blocks rather than lambdas |
@JonRowe hello! I got to this part of the code by debugging and profiling my own test suite. The description includes a benchmark of it. |
@xjunior if you check the benchmarks folder you'll see the sort of thing we like to add with these changes to document them for prosperity (and allow us to re-check if it changes at a later date) this helps us to avoid churn because something works "better on my machine" |
@JonRowe I added some. It doesn't show the same results I'm showing above. I believe this is because mine is running on a bigger sample, with rails, rspec-rails and others. The results I pushed show a 30% difference on memory allocation on this specific example. |
Before this change in my test suite: ```sh allocated memory by gem ----------------------------------- 450.63 MB rspec-mocks-3.11.0 250.91 MB activesupport-5.2.8.1 247.35 MB activerecord-5.2.8.1 ``` After this change in my test suite: ```sh allocated memory by gem ----------------------------------- 250.92 MB activesupport-5.2.8.1 247.36 MB activerecord-5.2.8.1 198.83 MB rspec-mocks-3.11.0 ```
@JonRowe WDYT? I'm very much in favour of merging this. |
This has been released in 3.12.7 |
Before this change in my test suite:
After this change in my test suite: