Skip to content

Reduce array alloc on mount #2543

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

Merged
merged 3 commits into from
Mar 2, 2025

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