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

/cc @gitlab-org/maintainers/rails-backend

Assignee Loading
Time tracking Loading