-
Notifications
You must be signed in to change notification settings - Fork 68
Memory usage optimization / avoid sending unneeded data to parallel processing #408
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
Conversation
Done to avoid copying potentially hige inputs and functions to the processes when all they need to do is crunch some silly small numbers. HUGE performance gain while reducing memory usage a lot for benchmarks with bigger data in use. Also use Task.async_stream instead of our good old `Parallel.map` as with the dawn of `inputs` I have seen people do 30+ scenarios (10 benchmarks with 3 inputs will do that to you), where if we're running on a 4 core system we might be doing too much in parallel also potentially skyrocketing memory consumption.
This includes a slight workaround to preserve the names of inputs as people may be relying on those. Doesn't feel great but... somehow reasonable?
|
Mental notes:
|
|
🥳 |
Now, that scenario is finally fast/fastest in that lil benchmark: ``` Name ips average deviation median 99th % Formatter.output 248.71 4.02 ms ±4.58% 4.03 ms 4.59 ms sequential_output 248.38 4.03 ms ±3.16% 4.00 ms 4.53 ms format & write 247.77 4.04 ms ±6.76% 4.02 ms 4.35 ms Comparison: Formatter.output 248.71 sequential_output 248.38 - 1.00x slower +0.00539 ms format & write 247.77 - 1.00x slower +0.0153 ms ```
|
I definitely should get exguard working again... the amount of force pushes due to minor credo things are embarrassing ._. |
|
👋 Hey @hrzndhrn @NickNeck as you have some of the most Benchee plugins I'd appreciate it if you took a look here, mainly from the PoV on impact for plugins. If you want to review the PR as a whole of course I'm happy too 😁 TLDR;
Thanks! |
|
@PragTob thanks for the hint. Thanks for Benchee and all your work. I will take a look during the weekend. |
|
@NickNeck thanks for all your great OSS work and also specifically of creating Benchee plugins, it was designed to facillitate and allow this but seeing all the things people do warms my heart! 💚 |
NickNeck
left a comment
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.
LGTM 👍 . I think the necessary changes for plugins are not too hard and the benefit it is worth.
|
@NickNeck thanks a lot! 💚 |
This reverts commit dfa6987. It turned out that the parallel processing wasn't the issue with memory consumption we faced but instead copying data to processes. See: * #408 * #414 * https://pragtob.wordpress.com/2023/12/18/careful-what-data-you-send-or-how-to-tank-your-performance-with-task-async/
This reverts commit dfa6987. It turned out that the parallel processing wasn't the issue with memory consumption we faced but instead copying data to processes. See: * #408 * #414 * https://pragtob.wordpress.com/2023/12/18/careful-what-data-you-send-or-how-to-tank-your-performance-with-task-async/

Should combat memory consumption issues caused by sending too much data to processes.
edit: To highlight the severeness of this:
:saveto compare performance across elixir version, before this PR each individual save file was 200+MB. With this MR, it's 1MB (as it removes the data from formatters and:saveis a formatter under the hood). The file size isn't the major thing, but it illustrates how much memory we save and how much time copying memory we avoid.I wanna write up all of this in like 2 blog posts but as per usual my speed is abysmal 😁