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

2 participants