Ban the usage of `.keys.include?` and `.values.include?`
Description of the proposal
Today we seem to sometime use .keys.include? which is bad in a couple of ways:
- Performs
O(n)search, where we have efficient.has_key?, - It clones all keys into separate array.
We should prefer to use .has_key? and .has_value?.
hash = 10000.times.map { |k| [k,k] }.to_h;
[35] pry(main)> puts Benchmark.measure { 10000.times { hash.keys.include?(10000) } }
1.014176 0.039619 1.053795 ( 1.057158)
=> nil
[36] pry(main)> puts MemoryProfiler.report { 10000.times { hash.keys.include?(10000) } }.total_allocated_memsize
800408317
=> nil
[37] pry(main)> puts Benchmark.measure { 10000.times { hash.has_key?(10000) } }
0.000679 0.000045 0.000724 ( 0.000719)
=> nil
[38] pry(main)> puts MemoryProfiler.report { 10000.times { hash.has_key?(10000) } }.total_allocated_memsize
0
=> nil-
Mention the proposal in the next backend weekly call and the #backend channel to encourage contribution -
Proceed with the proposal once 50% of the maintainers have weighed in, and 80% of the votes are 👍 -
Once approved, mention it again in the next backend weekly call and the #backend channel
Related to https://gitlab.com/gitlab-org/gitlab-ce/issues/64924
Edited by Kamil Trzciński