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 parsing lockfile #6976

Merged

Conversation

segiddins
Copy link
Member

==> memprof.after.txt <==
Total allocated: 673.08 kB (7644 objects)
Total retained:  107.35 kB (1018 objects)

==> memprof.before.txt <==
Total allocated: 739.12 kB (9140 objects)
Total retained:  138.61 kB (1695 objects)

Savings will scale by the number of lines in the lockfile

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.

Feel free to merge anyway if you don't think the @state change is worth it.

bundler/lib/bundler/lockfile_parser.rb Outdated Show resolved Hide resolved
@@ -149,11 +160,11 @@ def parse_dependency(line)
return unless line =~ NAME_VERSION
spaces = $1
return unless spaces.size == 2
name = $2
name = -$2
Copy link
Member

Choose a reason for hiding this comment

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

Cute. I'm surprised this isn't used more often, enough that it's common practice. First I've seen it and had to look it up.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think I've seen this before, but I failed when I tried to look it up. What is it?

Copy link
Member

Choose a reason for hiding this comment

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

Unary minus on String. It freezes the string returns a frozen version of the string. Unary + thaws.

Copy link
Member

Choose a reason for hiding this comment

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

Huh, how about that. I guess I should have checked the API docs! I did finally find a writeup: https://metaredux.com/posts/2019/05/10/weird-ruby-positive-and-negative-strings.html

Copy link
Member Author

Choose a reason for hiding this comment

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

@- is an alias for #dedup, which not only returns a frozen string, but will deduplicate it, leading to reduced retained memory usage

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: The unary operator is -@.

bundler/lib/bundler/lockfile_parser.rb Show resolved Hide resolved
bundler/lib/bundler/lockfile_parser.rb Outdated Show resolved Hide resolved
```
==> memprof.after.txt <==
Total allocated: 673.08 kB (7644 objects)
Total retained:  107.35 kB (1018 objects)

==> memprof.before.txt <==
Total allocated: 739.12 kB (9140 objects)
Total retained:  138.61 kB (1695 objects)
```

Savings will scale by the number of lines in the lockfile
@segiddins segiddins force-pushed the segiddins/reduce-allocations-when-parsing-lockfile branch from 8c829df to f6abf44 Compare September 21, 2023 18:04
@segiddins segiddins merged commit 6a2fa57 into master Sep 21, 2023
91 of 92 checks passed
@segiddins segiddins deleted the segiddins/reduce-allocations-when-parsing-lockfile branch September 21, 2023 19:53
deivid-rodriguez pushed a commit that referenced this pull request Oct 13, 2023
…hen-parsing-lockfile

Reduce allocations when parsing lockfile

(cherry picked from commit 6a2fa57)
deivid-rodriguez pushed a commit that referenced this pull request Oct 13, 2023
…hen-parsing-lockfile

Reduce allocations when parsing lockfile

(cherry picked from commit 6a2fa57)
deivid-rodriguez pushed a commit that referenced this pull request Oct 13, 2023
…hen-parsing-lockfile

Reduce allocations when parsing lockfile

(cherry picked from commit 6a2fa57)
deivid-rodriguez pushed a commit that referenced this pull request Oct 16, 2023
…hen-parsing-lockfile

Reduce allocations when parsing lockfile

(cherry picked from commit 6a2fa57)
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

5 participants