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.load with symbol_keys mode #681

Merged
merged 1 commit into from Aug 6, 2021

Conversation

Watson1978
Copy link
Collaborator

Convert from String to Symbol, seems that it has some overhead.
This patch will generate Symbol via rb_intern3() and ID2SYM().

The same approach seems to be favored by other gems.
(https://github.com/brianmario/mysql2/blob/ca883e1a359a10f48ac5d0ce649827b424110cd7/ext/mysql2/result.c#L171-L172)

before after result
Oj.load 339.194k 387.595k 1.14x

Environment

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

Before

Warming up --------------------------------------
             Oj.load    33.731k i/100ms
Calculating -------------------------------------
             Oj.load    339.194k (± 0.9%) i/s -      1.720M in   5.072086s

After

Warming up --------------------------------------
             Oj.load    39.129k i/100ms
Calculating -------------------------------------
             Oj.load    387.595k (± 1.0%) i/s -      1.956M in   5.048159s

Test code

require 'benchmark/ips'
require 'oj'

json =<<-EOF
{
  "$id": "https://example.com/person.schema.json",
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "title": "Person",
  "type": "object",
  "properties": {
    "firstName": {
      "type": "string",
      "description": "The person's first name."
    },
    "lastName": {
      "type": "string",
      "description": "The person's last name."
    },
    "age": {
      "description": "Age in years which must be equal to or greater than zero.",
      "type": "integer",
      "minimum": 0
    }
  }
}
EOF

Benchmark.ips do |x|
  x.report('Oj.load') { Oj.load(json, symbol_keys: true) }
end

Convert from String to Symbol, seems that it has some overhead.
This patch will generate Symbol via `rb_intern3()` and `ID2SYM()`.

The same approach seems to be favored by other gems.
(https://github.com/brianmario/mysql2/blob/ca883e1a359a10f48ac5d0ce649827b424110cd7/ext/mysql2/result.c#L171-L172)

−               | before   | after    | result
--               | --       | --       | --
Oj.load          | 339.194k | 387.595k | 1.14x

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

### Before
```
Warming up --------------------------------------
             Oj.load    33.731k i/100ms
Calculating -------------------------------------
             Oj.load    339.194k (± 0.9%) i/s -      1.720M in   5.072086s
```

### After
```
Warming up --------------------------------------
             Oj.load    39.129k i/100ms
Calculating -------------------------------------
             Oj.load    387.595k (± 1.0%) i/s -      1.956M in   5.048159s
```

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

json =<<-EOF
{
  "$id": "https://example.com/person.schema.json",
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "title": "Person",
  "type": "object",
  "properties": {
    "firstName": {
      "type": "string",
      "description": "The person's first name."
    },
    "lastName": {
      "type": "string",
      "description": "The person's last name."
    },
    "age": {
      "description": "Age in years which must be equal to or greater than zero.",
      "type": "integer",
      "minimum": 0
    }
  }
}
EOF

Benchmark.ips do |x|
  x.report('Oj.load') { Oj.load(json, symbol_keys: true) }
end
```
@ohler55 ohler55 merged commit 62aea59 into ohler55:develop Aug 6, 2021
@ohler55
Copy link
Owner

ohler55 commented Aug 6, 2021

Very nice, thank you.

@Watson1978 Watson1978 deleted the symbol branch August 6, 2021 12:17
@Watson1978
Copy link
Collaborator Author

Thank you for your quick response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants