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

Use frozen_string_literal: true #220

Merged
merged 2 commits into from
Aug 24, 2023

Conversation

technicalpickles
Copy link
Contributor

This can help reduce memory, by making string literals frozen. That means they are only allocated once in the file, instead of each time the code executes.

Noticed this while starting to profile for #200

This can help reduce memory, by making string literals frozen. That
means they are only allocated once in the file, instead of each time the
code executes.
@technicalpickles
Copy link
Contributor Author

I generated this using rubocop -A --only Style/FrozenStringLiteralComment,Layout/EmptyLineAfterMagicComment lib/ initially. I'd recommend setting rubocop or similar at some point to help enforce it in the future.

@ArturT
Copy link
Member

ArturT commented Aug 22, 2023

@technicalpickles Thanks for the proposed PR. I added this to review in our backlog.

Do you have stats showing how much memory reduction you see in your test suite? More or less?

@technicalpickles
Copy link
Contributor Author

I was able to use memory_profile before and after. Including the comparison below, but basically this allocates 4.8% less memory for the benchmarking I was doing.

Summary

Before

Total allocated: 5_264_380_999 bytes (46_645_350 objects)
Total retained:  490_574_213 bytes (3_490_258 objects)

After

Total allocated: 5_009_393_273 bytes (44_483_039 objects)
Total retained:  489_417_475 bytes (3_484_878 objects)

254,987,726 byte reduction of allocated memory (4.8%). 2,162,311 reduction of allocated objects (4.6%)

1,156,738 byte reduction of retained memory (0.2%). 5380 reduction of retained objects (0.1%).

memory allocation

by gem

before

46_175_051  knapsack_pro-5.3.5

after

42_921_828  knapsack_pro-ruby/lib

3253223 reduction of allocated memory (7%).

by file

before

25_501_613  /Users/josh.nichols/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/knapsack_pro-5.3.5/lib/knapsack_pro/logger_wrapper.rb
19_857_496  /Users/josh.nichols/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/knapsack_pro-5.3.5/lib/knapsack_pro/test_case_mergers/rspec_merger.rb

after

25_498_953  /Users/josh.nichols/workspace/knapsack_pro-ruby/lib/knapsack_pro/logger_wrapper.rb
17_183_176  /Users/josh.nichols/workspace/knapsack_pro-ruby/lib/knapsack_pro/test_case_mergers/rspec_merger.rb

object allocation

by gem

before

287_868  knapsack_pro-5.3.5

after

210_915  knapsack_pro-ruby/lib

76953 reduction of allocated objects (26%)

by file

before

164138  /Users/josh.nichols/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/knapsack_pro-5.3.5/lib/knapsack_pro/test_case_mergers/rspec_merger.rb
111540  /Users/josh.nichols/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/knapsack_pro-5.3.5/lib/knapsack_pro/logger_wrapper.rb

after

111525  /Users/josh.nichols/workspace/knapsack_pro-ruby/lib/knapsack_pro/logger_wrapper.rb
97280  /Users/josh.nichols/workspace/knapsack_pro-ruby/lib/knapsack_pro/test_case_mergers/rspec_merger.rb

@ArturT
Copy link
Member

ArturT commented Aug 22, 2023

@technicalpickles Thanks for the stats.

@ArturT ArturT merged commit bcd9ab2 into KnapsackPro:master Aug 24, 2023
@technicalpickles technicalpickles deleted the frozen-string-literal branch August 24, 2023 13:09
@ArturT
Copy link
Member

ArturT commented Aug 24, 2023

@technicalpickles We released the knapsack_pro 5.6.0 version. Please update it.

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.

2 participants