Do not call #class from #hash to imorove performance

[pocke]

This PR improves performance by removing self.class.hash method calls from hash methods.

Profiling

I profiled rbs validate command with the following code.

profiling code

Put the code in the root directory of this repository.

require 'stringio'

$LOAD_PATH << File.join(__dir__, "./lib")
require 'rbs'
require 'rbs/cli'

def run
  RBS::CLI.new(stdout: StringIO.new, stderr: StringIO.new).run(%w[validate --silent]) 
end

run # warm up

require 'stackprof'
StackProf.run(mode: :wall, out: "stackprof-out-#{Time.now.to_f}", raw: true) do
  10.times { run }
end

Result:

$ stackprof stackprof-out-1645283785.4565349
==================================
  Mode: wall(1000)
  Samples: 5067 (0.67% miss rate)
  GC: 1351 (26.66%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
       875  (17.3%)         875  (17.3%)     (marking)
       472   (9.3%)         472   (9.3%)     Kernel#class
       470   (9.3%)         470   (9.3%)     (sweeping)
       285   (5.6%)         285   (5.6%)     RBS::Namespace#initialize
       213   (4.2%)         213   (4.2%)     RBS::Substitution#empty?
       152   (3.0%)         152   (3.0%)     Kernel#hash

(snip)

Kernel#class takes 9.3% of the wallclock time. It's the heaviest method.

Benchmarking

Benchmarking Code

require 'benchmark/ips'
require 'stringio'

$LOAD_PATH << File.join(__dir__, "./lib")
require 'rbs'
require 'rbs/cli'

def run
  RBS::CLI.new(stdout: StringIO.new, stderr: StringIO.new).run(%w[validate --silent]) 
end

Benchmark.ips do |x|
  x.time = 20
  x.report('validate') { run }
end

before

Warming up --------------------------------------
            validate     1.000  i/100ms
Calculating -------------------------------------
            validate      1.990  (± 0.0%) i/s -     40.000  in  20.231517s

after

Warming up --------------------------------------
            validate     1.000  i/100ms
Calculating -------------------------------------
            validate      2.128  (± 0.0%) i/s -     43.000  in  20.267035s

It improves ips from 1.990 to 2.128, 1.069x faster. 🚀

Removing self.class.hash is safe

It's safe.
By this change, hash method will return the same value between different class objects. It does not break the rule of hash and eql? methods.

Generates an Integer hash value for this object. This function must have the property that a.eql?(b) implies a.hash == b.hash.

https://docs.ruby-lang.org/en/3.1/Object.html#method-i-hash

I guess hash keys classes for one hash object are the same class in many cases, so the collision will occur rarely.

By the way, some self.class.hash method calls still remain. For example: lib/rbs/types.rb
These method calls are not the bottleneck. And their implementations are too simple, so if self.class.hash is removed they introduces collisions easily. So I didn't remove the self.class.hash.

[soutaro]

I'm good to merge this change, but still not sure if this small benchmark improvement changes user's experience.

[pocke]

Yes, it is a small improvement. Maybe it will change the user experience if the environment is larger, but I'm not sure also.