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

Optimize parsing option args in load #342

Merged
merged 1 commit into from May 6, 2023

Conversation

Watson1978
Copy link
Contributor

It looked up one by one to make sure that the supported options were included in passed options. This approach of looking up the passed hash each time has an overhead.

This patch will extract key and value from the passed options using rb_hash_foreach() and check if they are supported or not.

before after result
load 640.171k 679.986k 1.062x

Environment

  • MacBook Pro (M1, 2021)
  • macOS 13.3.1
  • Apple M1
  • clang 14.0.3
  • Ruby 3.2.2

Before

Warming up --------------------------------------
                load    63.538k i/100ms
Calculating -------------------------------------
                load    640.171k (± 0.2%) i/s -      3.240M in   5.061851s

After

Warming up --------------------------------------
                load    67.690k i/100ms
Calculating -------------------------------------
                load    679.986k (± 0.3%) i/s -      3.452M in   5.076904s

Test code

require 'bundler/inline'
gemfile do
  source 'https://rubygems.org'
  gem 'benchmark-ips'
  gem 'ox'
end

xml = %{
<?xml version="1.0"?>
<Park.Animal>
  <type>mutant</type>
  <friends type="Hash">
    <i>5</i>
    <Park.Animal>
      <type>dog</type>
    </Park.Animal>
  </friends>
</Park.Animal>
}

Benchmark.ips do |x|
  x.report('load') { Ox.load(xml, mode: :generic) }
end

It looked up one by one to make sure that the supported options were included in passed options.
This approach of looking up the passed hash each time has an overhead.

This patch will extract key and value from the passed options using `rb_hash_foreach()` and check if they are supported or not.

−            | before   | after    | result
--            | --       | --       | --
load          | 640.171k | 679.986k | 1.062x

### Environment
- MacBook Pro (M1, 2021)
- macOS 13.3.1
- Apple M1
- clang 14.0.3
- Ruby 3.2.2

### Before
```
Warming up --------------------------------------
                load    63.538k i/100ms
Calculating -------------------------------------
                load    640.171k (± 0.2%) i/s -      3.240M in   5.061851s
```

### After
```
Warming up --------------------------------------
                load    67.690k i/100ms
Calculating -------------------------------------
                load    679.986k (± 0.3%) i/s -      3.452M in   5.076904s
```

### Test code
```
require 'bundler/inline'
gemfile do
  source 'https://rubygems.org'
  gem 'benchmark-ips'
  gem 'ox'
end

xml = %{
<?xml version="1.0"?>
<Park.Animal>
  <type>mutant</type>
  <friends type="Hash">
    <i>5</i>
    <Park.Animal>
      <type>dog</type>
    </Park.Animal>
  </friends>
</Park.Animal>
}

Benchmark.ips do |x|
  x.report('load') { Ox.load(xml, mode: :generic) }
end
```
@Watson1978
Copy link
Contributor Author

This is similar way with ohler55/oj#683

@ohler55 ohler55 merged commit 83eca82 into ohler55:develop May 6, 2023
16 checks passed
@Watson1978 Watson1978 deleted the load-options branch May 7, 2023 05:31
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