Ban the usage of `.keys.first` and `.values.first`
Description of the proposal
Today we seem to sometime use .keys.first
and .values.first
which is bad in a couple of ways:
- It has to iterate all keys: this increases the pressure on L1/L2 cache,
- It clones all keys/values into memory,
We should likely provide a helpers for Hash
: Hash#first_key
, Hash#first_value
.
hash = 10000.times.map { |k| [k,k] }.to_h;
[42] pry(main)> puts Benchmark.measure { 10000.times { hash.keys.first } }
0.690280 0.052384 0.742664 ( 0.743206)
=> nil
[43] pry(main)> puts MemoryProfiler.report { 10000.times { hash.keys.first } }.total_allocated_memsize
Gitlab::Metrics::Samplers::InfluxSampler: can't set length of shared string, stopping
800408465
=> nil
[44] pry(main)> puts Benchmark.measure { 10000.times { hash.first.first } }
0.004036 0.000003 0.004039 ( 0.004039)
=> nil
[45] pry(main)> puts MemoryProfiler.report { 10000.times { hash.first.first } }.total_allocated_memsize
400000
=> nil
hash = 10000.times.map { |k| [k,k] }.to_h;
[46] pry(main)> puts Benchmark.measure { 10000.times { hash.values.first } }
0.518665 0.015941 0.534606 ( 0.534605)
=> nil
[47] pry(main)> puts MemoryProfiler.report { 10000.times { hash.values.first } }.total_allocated_memsize
800400000
=> nil
[48] pry(main)> puts Benchmark.measure { 10000.times { hash.first.second } }
0.003865 0.000000 0.003865 ( 0.003864)
=> nil
[49] pry(main)> puts MemoryProfiler.report { 10000.times { hash.first.second } }.total_allocated_memsize
400000
=> 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