ruby-protobuf / protobuf

A pure ruby implementation of Google's Protocol Buffers
https://github.com/ruby-protobuf
MIT License
463 stars 101 forks source link

Bdewitt/remove helpers #391

Closed abrandoned closed 5 years ago

abrandoned commented 5 years ago

Quite a few optimizations for MRI and JRuby; started with too large a cut (including some protobuf java helpers for varint encoding) but paired back to just including optimizations in the first PR. Will be starting another gem to include the varint changes for java (which yields another significant increase in perf for encode/decode)

@film42 @quixoten @brianstien @liveh2o

Primary optimizations were removing the need to do runtime checking of map?/repeated? on field objects by writing the access methods set/set_field/value_from_values directly onto the field object; also sped up the access of each get_field call by using a constant dynamically defined on inherited object, JRuby has much faster access to Constants than class method lookups; also removed superfluous lookups of fields and respond_to calls to be replaced by methods on the field object

included the benchmark I ran as varint_prof.rb but will probably move it into a rake task so we can use it for future optimization; will also squash all the commits for a final merge but keeping them in the branch for now as changes are discussed

Benchmark/ips if encode/decode/to_hash on a protobuf message with MRI and JRuby below

JRuby -- approx 5x faster to_hash 2.5x faster encode 1.5x faster decode

# BEFORE PR
JRUBY_OPTS="--server --disable:did_you_mean" bundle exec ruby ./varint_prof.rb
Warming up --------------------------------------
             to_hash    32.455k i/100ms
              encode    15.644k i/100ms
              decode    18.291k i/100ms
Calculating -------------------------------------
             to_hash    394.430k (± 1.5%) i/s -      7.886M in  19.999530s
              encode    175.397k (± 1.8%) i/s -      3.520M in  20.075313s
              decode    209.607k (± 2.2%) i/s -      4.207M in  20.081145s

# AFTER PR applied
JRUBY_OPTS="--server --disable:did_you_mean" bundle exec ruby ./varint_prof.rb 
Warming up --------------------------------------
             to_hash   102.823k i/100ms
              encode    33.450k i/100ms
              decode    26.993k i/100ms
Calculating -------------------------------------
             to_hash      1.934M (± 4.1%) i/s -     38.661M in  20.034220s
              encode    443.646k (± 3.6%) i/s -      8.864M in  20.011563s
              decode    351.769k (± 2.2%) i/s -      7.045M in  20.038938s

MRI -- approx 3x faster to_hash 1.3x faster encode 1.2x faster decode

# BEFORE
bundle exec ruby ./varint_prof.rb
Warming up --------------------------------------
             to_hash    36.539k i/100ms
              encode    11.728k i/100ms
              decode    10.526k i/100ms
Calculating -------------------------------------
             to_hash    419.455k (± 0.7%) i/s -      8.404M in  20.036460s
              encode    120.492k (± 2.8%) i/s -      2.416M in  20.068034s
              decode    109.387k (± 0.7%) i/s -      2.189M in  20.016147s

# AFTER PR applied
bundle exec ruby ./varint_prof.rb 
Warming up --------------------------------------
             to_hash   105.286k i/100ms
              encode    15.569k i/100ms
              decode    12.675k i/100ms
Calculating -------------------------------------
             to_hash      1.402M (± 0.8%) i/s -     28.111M in  20.058605s
              encode    167.251k (± 1.2%) i/s -      3.347M in  20.016641s
              decode    134.016k (± 0.8%) i/s -      2.687M in  20.052077s
liveh2o commented 5 years ago

Overall, this looks good. I especially appreciate getting rid of that crazy set_field method with all of the nested conditionals. 💯

liveh2o commented 5 years ago

Any idea what's up with all the failing specs?

film42 commented 5 years ago

I've read through this PR several times now and it looks good to me. If specs are passing I think we're good to cut a pre-release. Nice work, @abrandoned !