Make to_json twice as fast as it used to be

[shuheiktgw]

I found the current implementation of to_json can be slow since it defines to_json method dynamically first time the method is called. So I use class_eval instead and change to define the method when class itself is evaluated. As a result to_json method bacame two times faster (from 0.4819999448955059 ms to 0.2079999540001154 ms) and the class became much simpler!

Below is the test code I used to check the performance.

require 'spec_helper'

describe FastJsonapi::MultiToJson do
  include_context 'movie class'

  before(:all) { GC.disable }
  after(:all) { GC.enable }

  describe 'self.to_json' do
    it do
      second = Benchmark.measure { FastJsonapi::MultiToJson.to_json movie }.real * 1000
      print 'to_json: '
      puts "#{second} ms"
    end
  end
end

[shishirmk]

@shuheiktgw Thanks for the pull request @bf4 do you mind taking a look into this?

[bf4]

@shishirmk I think there's a lot to discuss in this PR.

Firstly, is the argument that the first time the method is called is twice as fast? If so, I don't think that's a useful benchmark. Most applications will live to have the method called more than that.

In any case, I think it may be better to tell end users they can put a call to_json in an initializer to eagerly evaluate the method after all the relevant gems are required.

For what it's worth, this pattern comes from a perf PR to ActiveModelSerializers that still holds up rails-api/active_model_serializers#233

The rest of the changes in this PR need a discussion of cost benefit analysis, including taking into account that the original PR was merged without a test suite, but was tested otherwise. The various methods removed in this PR are critical to configuring and debugging the behavior. I would use it in #135

[bf4]

On my machine, a MBP running Ruby 2.4.1 I run the bench against master and get about 0.28ms each run and against make_to_json_fast about 0.11 ms each run.

But, if I change the benchmark to run to_json just once, I get the same 0.03 ms on each branch.

require 'spec_helper'

describe FastJsonapi::MultiToJson do
  include_context 'movie class'

  before(:all) { GC.disable }
  after(:all) { GC.enable }

  describe 'self.to_json' do
    it do
+ FastJsonapi::MultiToJson.to_json movie
      second = Benchmark.measure { FastJsonapi::MultiToJson.to_json movie }.real * 1000
      print 'to_json: '
      puts "#{second} ms"
    end
  end
end

Let's rewrite the benchmark to use Benchmark.bmbm

Sometimes benchmark results are skewed because code executed earlier encounters different garbage collection overheads than that run later. bmbm attempts to minimize this effect by running the tests twice, the first time as a rehearsal in order to get the runtime environment stable, the second time for real. GC.start is executed before the start of each of the real timings; the cost of this is not included in the timings. In reality, though, there’s only so much that bmbm can do, and the results are not guaranteed to be isolated from garbage collection and other effects.

require 'spec_helper'

describe FastJsonapi::MultiToJson do
  include_context 'movie class'

  describe 'self.to_json' do
    it do
      n = 1_000
      Benchmark.bmbm do |x|
        x.report('to_json') {
          i = 0
          while i < n
            FastJsonapi::MultiToJson.to_json movie
            i += 1
          end
        }
      end
    end
  end
end

Here are the outputs against each branch. I'm not going to label which branch is which 'cause they're the same :)

Rehearsal -------------------------------------------
to_json   0.020000   0.000000   0.020000 (  0.032862)
---------------------------------- total: 0.020000sec

              user     system      total        real
to_json   0.020000   0.000000   0.020000 (  0.021913)

📝 $ be rspec spec/bench_spec.rb
Rehearsal -------------------------------------------
to_json   0.030000   0.010000   0.040000 (  0.029570)
---------------------------------- total: 0.040000sec

              user     system      total        real
to_json   0.020000   0.000000   0.020000 (  0.022821)

Or with benchmark-ips

ruby-2.4.1 [make_to_json_fast L|✚ 1…1⚑ 1]
📝 $ be rspec spec/bench_spec.rb
Warming up --------------------------------------
             to_json     3.951k i/100ms
Calculating -------------------------------------
             to_json     41.156k (± 4.3%) i/s -    209.403k in   5.097581s
.
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.

📝 $ be rspec spec/bench_spec.rb
Warming up --------------------------------------
             to_json     4.010k i/100ms
Calculating -------------------------------------
             to_json     41.973k (± 3.6%) i/s -    212.530k in   5.070251s

code is

require 'spec_helper'
require 'benchmark/ips'

describe FastJsonapi::MultiToJson do
  include_context 'movie class'

  describe 'self.to_json' do
    it do
      Benchmark.ips do |x|
        x.report('to_json') do |n|
          i = 0
          while i < n
            FastJsonapi::MultiToJson.to_json movie
            i += 1
          end
        end
      end
    end
  end
end

[bf4]

@shuheiktgw I commented above

[shuheiktgw]

@bf4 Thank you so much for your comments! You are absolutely right if you take a benchmark multiple time the result will be the same since the difference appears only for the first time that to_json is called.

Even if this PR does not improve performance, I personally believe that I can simply to_json method by using class_eval. What would you say if I make back some logging features I removed?

[bf4]

I don’t see the benefit of removing the logger or the current behavior of how the methods are defined. Is it aesthetic? B mobile phone

…

On Apr 22, 2018, at 4:43 AM, Shuhei Kitagawa ***@***.***> wrote: @bf4 Thank you so much for your comments! You are absolutely right if you take a benchmark multiple time the result will be the same since the difference appears only for the first time that to_json is called. Even if this PR does not improve performance, I personally believe that I can simply to_json method by using class_eval. What would you think if I make back some logging features I removed? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[bf4]

@shuheiktgw Given this discussion, I think you should rename the PR to describe what you want to do with it.