KnapsackPro / knapsack_pro-ruby

Knapsack Pro gem splits tests across parallel CI nodes and makes sure that tests run in optimal time
https://knapsackpro.com
MIT License
131 stars 27 forks source link

Use frozen_string_literal: true #220

Closed technicalpickles closed 1 year ago

technicalpickles commented 1 year ago

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 https://github.com/KnapsackPro/knapsack_pro-ruby/issues/200

technicalpickles commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@technicalpickles Thanks for the stats.

ArturT commented 1 year ago

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