Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance of Oj.dump with compat/rails mode #675

Merged
merged 1 commit into from Aug 2, 2021

Conversation

@Watson1978
Copy link
Collaborator

@Watson1978 Watson1978 commented Jul 26, 2021

This patch introduces oj_hash_has_key() (same as rb_hash_has_key)
to reduce rb_funcall() calling because it has a overhead.

This patch will improve Oj.dump performance as following.

before after result
Oj.dump 1.949M 1.966M
Oj.dump (compat) 850.154k 1.198M 1.41x
Oj.dump (rails) 657.383k 840.051k 1.28x

Environment

  • MacBook Air (M1, 2020)
  • macOS 12.0 beta 3
  • Apple M1
  • Ruby 3.0.2

Before

Warming up --------------------------------------
             Oj.dump   198.379k i/100ms
    Oj.dump (compat)    86.466k i/100ms
     Oj.dump (rails)    66.760k i/100ms
Calculating -------------------------------------
             Oj.dump      1.949M (± 0.3%) i/s -      9.919M in   5.088487s
    Oj.dump (compat)    850.154k (± 0.3%) i/s -      4.323M in   5.085367s
     Oj.dump (rails)    657.383k (± 0.4%) i/s -      3.338M in   5.077802s

After

Warming up --------------------------------------
             Oj.dump   198.297k i/100ms
    Oj.dump (compat)   120.402k i/100ms
     Oj.dump (rails)    84.204k i/100ms
Calculating -------------------------------------
             Oj.dump      1.966M (± 0.3%) i/s -      9.915M in   5.044305s
    Oj.dump (compat)      1.198M (± 0.2%) i/s -      6.020M in   5.026524s
     Oj.dump (rails)    840.051k (± 0.1%) i/s -      4.210M in   5.011848s

Test code

require 'benchmark/ips'
require 'oj'

data = {
  'short_string': 'a' * 50,
  'long_string': 'b' * 255,
  'utf8_string': 'あいうえお' * 10
}

Benchmark.ips do |x|
  x.report('Oj.dump') { Oj.dump(data) }
  x.report('Oj.dump (compat)') { Oj.dump(data, mode: :compat) }
  x.report('Oj.dump (rails)') { Oj.dump(data, mode: :rails) }
end
@Watson1978 Watson1978 changed the title Improve performance of Oj.dumpwith compat/rails mode Improve performance of Oj.dump with compat/rails mode Jul 26, 2021
@Watson1978
Copy link
Collaborator Author

@Watson1978 Watson1978 commented Jul 26, 2021

I retrieved a benchmark result on Intel Mac.

before after result
Oj.dump 1.435M 1.453M
Oj.dump (compat) 558.205k 897.370k 1.61x
Oj.dump (rails) 428.819k 655.485k 1.53x

Env

  • MacBook Pro (16-inch, 2019)
  • macOS BigSur 11.5
  • CPU 2.4 GHz 8cores Intel Core i9
  • Ruby 3.0.2

Before

Warming up --------------------------------------
             Oj.dump   139.896k i/100ms
    Oj.dump (compat)    55.788k i/100ms
     Oj.dump (rails)    43.009k i/100ms
Calculating -------------------------------------
             Oj.dump      1.435M (± 2.3%) i/s -      7.275M in   5.072783s
    Oj.dump (compat)    558.205k (± 2.5%) i/s -      2.789M in   5.000364s
     Oj.dump (rails)    428.819k (± 4.7%) i/s -      2.150M in   5.026970s

After

Warming up --------------------------------------
             Oj.dump   142.370k i/100ms
    Oj.dump (compat)    89.373k i/100ms
     Oj.dump (rails)    66.401k i/100ms
Calculating -------------------------------------
             Oj.dump      1.453M (± 3.4%) i/s -      7.261M in   5.001952s
    Oj.dump (compat)    897.370k (± 1.9%) i/s -      4.558M in   5.081196s
     Oj.dump (rails)    655.485k (± 2.3%) i/s -      3.320M in   5.067834s

This patch introduces `oj_hash_has_key()` (same as rb_hash_has_key)
to reduce `rb_funcall()` calling because it has a overhead.

This patch will improve `Oj.dump` performance as following.

−               | before   | after    | result
--               | --       | --       | --
Oj.dump          | 1.949M   | 1.966M   | −
Oj.dump (compat) | 850.154k | 1.198M   | 1.41x
Oj.dump (rails)  | 657.383k | 840.051k | 1.28x

### Environment
- MacBook Air (M1, 2020)
- macOS 12.0 beta 3
- Apple M1
- Ruby 3.0.2

### Before
```
Warming up --------------------------------------
             Oj.dump   198.379k i/100ms
    Oj.dump (compat)    86.466k i/100ms
     Oj.dump (rails)    66.760k i/100ms
Calculating -------------------------------------
             Oj.dump      1.949M (± 0.3%) i/s -      9.919M in   5.088487s
    Oj.dump (compat)    850.154k (± 0.3%) i/s -      4.323M in   5.085367s
     Oj.dump (rails)    657.383k (± 0.4%) i/s -      3.338M in   5.077802s
```

### After
```
Warming up --------------------------------------
             Oj.dump   198.297k i/100ms
    Oj.dump (compat)   120.402k i/100ms
     Oj.dump (rails)    84.204k i/100ms
Calculating -------------------------------------
             Oj.dump      1.966M (± 0.3%) i/s -      9.915M in   5.044305s
    Oj.dump (compat)      1.198M (± 0.2%) i/s -      6.020M in   5.026524s
     Oj.dump (rails)    840.051k (± 0.1%) i/s -      4.210M in   5.011848s
```

### Test code
```ruby
require 'benchmark/ips'
require 'oj'

data = {
  'short_string': 'a' * 50,
  'long_string': 'b' * 255,
  'utf8_string': 'あいうえお' * 10
}

Benchmark.ips do |x|
  x.report('Oj.dump') { Oj.dump(data) }
  x.report('Oj.dump (compat)') { Oj.dump(data, mode: :compat) }
  x.report('Oj.dump (rails)') { Oj.dump(data, mode: :rails) }
end
```
@Watson1978
Copy link
Collaborator Author

@Watson1978 Watson1978 commented Aug 2, 2021

@ohler55 I would appreciate any feedback about this pull request. Thanks.

@ohler55
Copy link
Owner

@ohler55 ohler55 commented Aug 2, 2021

Looks very reasonable to me. Thank you.

@ohler55 ohler55 merged commit 7ed86e3 into ohler55:develop Aug 2, 2021
29 of 31 checks passed
@ohler55
Copy link
Owner

@ohler55 ohler55 commented Aug 2, 2021

I would not expect a significant change on non-trivial objects. For small it does help.

@Watson1978 Watson1978 deleted the hash_has_key branch Aug 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants