Skip to content

Conversation

@ericproulx
Copy link
Contributor

@ericproulx ericproulx commented Mar 1, 2025

#2529 change the @setup variable to an [] but the code is using += when adding a new step. This creates a new [] everytime and its not the best in terms on memory footprint. This PR replaced the += by << to not create new arrays.

I had to remove a test since having duplicates is expected. It would have failed in #2529 if the test had been like this since @setup is not the same object.

it 'does not unnecessarily retain duplicate setup blocks' do
  subject.logger
  first_setup = subject.instance_variable_get(:@setup)
  subject.logger
  last_setup = subject.instance_variable_get(:@setup)
  expect(first_setup.size).to eq(last_setup.size)
end

Running benchmark/remounting.rb shows the efficiency. It shows the difference between a Set and an Array but we still see a difference between << and +=
This is currently

Calculating -------------------------------------
 setup size: 1000
         using Array    19.623M memsize (   226.006k retained)
                       116.426k objects (     2.123k retained)
                        50.000  strings (     7.000  retained)
 setup size: 1
           using Set    11.680M memsize (     4.024k retained)
                       117.004k objects (    16.000  retained)
                         5.000  strings (     1.000  retained)

Comparison:
           using Set:   11680280 allocated
         using Array:   19623445 allocated - 1.68x more

This is with the PR

Calculating -------------------------------------
 setup size: 1000
         using Array    11.815M memsize (   229.806k retained)
                       112.426k objects (     2.123k retained)
                        50.000  strings (     7.000  retained)
 setup size: 1
           using Set    11.680M memsize (     4.024k retained)
                       111.004k objects (    16.000  retained)
                         5.000  strings (     1.000  retained)

Comparison:
           using Set:   11680280 allocated
         using Array:   11814541 allocated - 1.01x more

Some refactor
Remove wrongful spec
@ericproulx ericproulx marked this pull request as ready for review March 1, 2025 17:26
@ericproulx ericproulx requested a review from dblock March 1, 2025 17:29
@dblock dblock merged commit d956e44 into ruby-grape:master Mar 2, 2025
63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants