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

Reduce allocations when installing gems with bundler #6977

Conversation

segiddins
Copy link
Member

==> memprof.after.txt <==
Total allocated: 1.13 MB (2352 objects)
Total retained:  10.08 kB (78 objects)

==> memprof.before.txt <==
Total allocated: 46.27 MB (38439 objects)
Total retained:  9.94 kB (75 objects)

Yes, we were allocating 45MB of arrays in dependencies_installed?,
it was accidentally cubic.

What was the end-user or developer problem that led to this PR?

What is your fix for the problem, implemented in this PR?

Make sure the following tasks are checked

Copy link
Member

@martinemde martinemde left a comment

Choose a reason for hiding this comment

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

Nested, nested temporary array creation, neat.

Just need to update specs testing the changed interface.

@martinemde martinemde force-pushed the segiddins/reduce-allocations-when-installing-gems-with-bundler branch from 17cc837 to b36336a Compare October 12, 2023 01:32
@segiddins segiddins force-pushed the segiddins/reduce-allocations-when-installing-gems-with-bundler branch 2 times, most recently from 18ef2ff to 25f4878 Compare October 19, 2023 18:14
```
==> memprof.after.txt <==
Total allocated: 1.13 MB (2352 objects)
Total retained:  10.08 kB (78 objects)

==> memprof.before.txt <==
Total allocated: 46.27 MB (38439 objects)
Total retained:  9.94 kB (75 objects)
```

Yes, we were allocating 45MB of arrays in `dependencies_installed?`,
it was accidentally cubic.
@deivid-rodriguez deivid-rodriguez force-pushed the segiddins/reduce-allocations-when-installing-gems-with-bundler branch from 25f4878 to 13ab874 Compare November 23, 2023 21:15
@segiddins segiddins merged commit 07cbfd8 into master Nov 26, 2023
66 checks passed
@segiddins segiddins deleted the segiddins/reduce-allocations-when-installing-gems-with-bundler branch November 26, 2023 22:07
@deivid-rodriguez
Copy link
Member

Awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants