Case when splat

[rrosenblum]

This is part 1 of memory performance improvements.

There is a slight risk in the auto-correct. If more than one when condition could apply to the same input, and the earlier condition is used as a short circuiting, then auto-correct could change the code to do the wrong thing. I do not think this is a large concern. If somebody thinks otherwise, I will gladly disable auto-correct by default. I also wasn't sure if I should document this risk inside of the description, class, or not at all.

Example

case foo
when 2
  puts 'special'
when *[2, 4, 6]
  puts 'even'
end

# auto-corrected
case foo
when *[2, 4, 6]
  puts 'even'
when 2
  puts 'special'
end

While running memory_profiler on RuboCop. The sample for the data came from running RuboCop against lib/rubocop/cop/performance, lib/rubocop/cop/rails, and lib/rubocop/cop/lint. I could not sample running against the entire project because of the time and resources needed to analyze that much memory.

Total memory consumption dropped by about 2%
The memory allocation for variable_force.rb dropped by 30%.
There was little impact on locatable.rb, and safe_assignment.rb didn't fall into the top 50 most memory consuming files.

[jonas054]

It's strange to use an example where rearranging the whens has changed the semantics. So remove 3 from the array.

[rrosenblum]

Good catch. I didn't mean to have overlap in the example.

[jonas054]

If the splat operator is being applied to an array literal you could also just change

when *[1, 2, 3]

to

when 1, 2, 3

Perhaps that's a better way to auto-correct in that particular case, provided that it gives the same improvement regarding memory allocation.

[rrosenblum]

Good idea about auto-correcting splat on array literal to be a list without the array. I will see what this does to memory usage. Assuming that it does not have the same issue that using splat does, I will make that correction.

[rrosenblum]

I was able to test case when with replacing splat array with its expanded form. As expected, there was a noticeable drop in memory allocation. There is an increase in the retained memory, but that is negligible compared to the drop in allocated memory. If presented with the opportunity, refactoring out the splat is the best option.

variable_force.rb
master
  allocated - 2822517
  retained  - 120
reorder (not maintaining relative defined order)
  allocated - 1995877
  retained  - 120
no splat
  allocated - 770733
  retained  - 352

locatable.rb - allocated
  master - 3140792
  reorder (not maintaining relative defined order) - 3158712
  no splat - 2503656

[rrosenblum]

I have updated the cop to maintain relative condition order and correct splat array literal to remove the splat and square brackets. There is potential for increased performance by rearranging the splat when conditions, but it is impossible to RuboCop to automatically handle those conditions.

[rrosenblum]

The code has been modified and rebased. Let me know if I need to tweak anything else.

[jonas054]

👍 LGTM

[bbatsov]

manor -> manner

[rrosenblum]

updated