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

Use RSTRING_PTR() to retrieve C string pointer #684

Merged
merged 1 commit into from Aug 8, 2021

Conversation

Watson1978
Copy link
Collaborator

rb_string_value_ptr() attempts implicit type conversion with #to_str method.

char *
rb_string_value_ptr(volatile VALUE *ptr)
{
    VALUE str = rb_string_value(ptr);
    return RSTRING_PTR(str);
}

Refer: https://github.com/ruby/ruby/blob/f81964568f954495ad9a517066bd241f5db22059/string.c#L2319-L2324

However, we have already convert to string using #to_s method or check the type.
Therefore, This patch will call RSTRING_PTR() directly to reduce overhead.

before after result
Oj.load 419.273k 421.858k -
Oj.dump 1.233M 1.303M 1.06x

Environment

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

Before

Warming up --------------------------------------
             Oj.load    42.892k i/100ms
             Oj.dump   124.603k i/100ms
Calculating -------------------------------------
             Oj.load    419.273k (± 0.6%) i/s -      2.102M in   5.012916s
             Oj.dump      1.233M (± 0.5%) i/s -      6.230M in   5.051849s

After

Warming up --------------------------------------
             Oj.load    42.986k i/100ms
             Oj.dump   132.017k i/100ms
Calculating -------------------------------------
             Oj.load    421.858k (± 0.4%) i/s -      2.149M in   5.094940s
             Oj.dump      1.303M (± 0.8%) i/s -      6.601M in   5.068088s

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) }

  data = Oj.load(json)
  x.report('Oj.dump') { Oj.dump(data) }
end

`rb_string_value_ptr()` attempts implicit type conversion with `#to_str` method.

```c
char *
rb_string_value_ptr(volatile VALUE *ptr)
{
    VALUE str = rb_string_value(ptr);
    return RSTRING_PTR(str);
}
```
Refer: https://github.com/ruby/ruby/blob/f81964568f954495ad9a517066bd241f5db22059/string.c#L2319-L2324

However, we have already convert to string using `#to_s` method or check the type.
Therefore, This patch will call `RSTRING_PTR()` directly to reduce overhead.

−               | before   | after    | result
--               | --       | --       | --
Oj.load          | 419.273k | 421.858k | -
Oj.dump          | 1.233M   | 1.303M   | 1.06x

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

### Before
```
Warming up --------------------------------------
             Oj.load    42.892k i/100ms
             Oj.dump   124.603k i/100ms
Calculating -------------------------------------
             Oj.load    419.273k (± 0.6%) i/s -      2.102M in   5.012916s
             Oj.dump      1.233M (± 0.5%) i/s -      6.230M in   5.051849s
```

### After
```
Warming up --------------------------------------
             Oj.load    42.986k i/100ms
             Oj.dump   132.017k i/100ms
Calculating -------------------------------------
             Oj.load    421.858k (± 0.4%) i/s -      2.149M in   5.094940s
             Oj.dump      1.303M (± 0.8%) i/s -      6.601M in   5.068088s
```

### 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) }

  data = Oj.load(json)
  x.report('Oj.dump') { Oj.dump(data) }
end
```
@ohler55 ohler55 merged commit 4fdd25d into ohler55:develop Aug 8, 2021
@Watson1978 Watson1978 deleted the string-ptr branch August 8, 2021 15:45
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