Skip to content

Commit

Permalink
Use rb_sym2str() to retrieve String object (#687)
Browse files Browse the repository at this point in the history
When it retrieves String object using `rb_sym_to_s()`, it duplicate the internal String of Symbol.

```c
VALUE
rb_sym_to_s(VALUE sym)
{
    return str_new_shared(rb_cString, rb_sym2str(sym));
}
```
Refer: https://github.com/ruby/ruby/blob/6f6a84f2f3e82f9554618f080f9b66ded52884cc/string.c#L11136-L11140

There will be no need to duplicate it, since we haven't changed the retrieved String.
This patch will improve performance where it handles Symbols.

−               | before   | after    | result
--               | --       | --       | --
Oj.dump          | 807.090k | 1.173M   | 1.45x
Oj.dump (compat) | 760.372k | 1.080M   | 1.42x
Oj.dump (rails)  | 656.976k | 880.951k | 1.34x

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

### Before
```
Warming up --------------------------------------
             Oj.dump    80.910k i/100ms
    Oj.dump (compat)    75.874k i/100ms
     Oj.dump (rails)    65.977k i/100ms
Calculating -------------------------------------
             Oj.dump    807.090k (± 0.9%) i/s -      4.046M in   5.012848s
    Oj.dump (compat)    760.372k (± 0.8%) i/s -      3.870M in   5.089417s
     Oj.dump (rails)    656.976k (± 0.8%) i/s -      3.299M in   5.021613s
```

### After
```
Warming up --------------------------------------
             Oj.dump   119.895k i/100ms
    Oj.dump (compat)   107.509k i/100ms
     Oj.dump (rails)    87.631k i/100ms
Calculating -------------------------------------
             Oj.dump      1.173M (± 0.5%) i/s -      5.875M in   5.007040s
    Oj.dump (compat)      1.080M (± 0.7%) i/s -      5.483M in   5.075621s
     Oj.dump (rails)    880.951k (± 0.9%) i/s -      4.469M in   5.073530s
```

### 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|
  data = Oj.load(json, symbol_keys: true)

  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
```
  • Loading branch information
Watson1978 committed Aug 9, 2021
1 parent 2f05091 commit 1deb721
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 12 deletions.
2 changes: 1 addition & 1 deletion ext/oj/custom.c
Expand Up @@ -833,7 +833,7 @@ static void dump_struct(VALUE obj, int depth, Out out, bool as_ok) {
v = rb_struct_aref(obj, INT2FIX(i));
#endif
if (ma != Qnil) {
volatile VALUE s = rb_sym_to_s(rb_ary_entry(ma, i));
volatile VALUE s = rb_sym2str(rb_ary_entry(ma, i));

name = RSTRING_PTR(s);
len = (int)RSTRING_LEN(s);
Expand Down
5 changes: 1 addition & 4 deletions ext/oj/dump.c
Expand Up @@ -717,10 +717,7 @@ void oj_dump_str(VALUE obj, int depth, Out out, bool as_ok) {
}

void oj_dump_sym(VALUE obj, int depth, Out out, bool as_ok) {
// This causes a memory leak in 2.5.1. Maybe in other versions as well.
// const char *sym = rb_id2name(SYM2ID(obj));

volatile VALUE s = rb_sym_to_s(obj);
volatile VALUE s = rb_sym2str(obj);

oj_dump_cstr(RSTRING_PTR(s), (int)RSTRING_LEN(s), 0, 0, out);
}
Expand Down
4 changes: 2 additions & 2 deletions ext/oj/dump_object.c
Expand Up @@ -208,7 +208,7 @@ static void dump_str(VALUE obj, int depth, Out out, bool as_ok) {
}

static void dump_sym(VALUE obj, int depth, Out out, bool as_ok) {
volatile VALUE s = rb_sym_to_s(obj);
volatile VALUE s = rb_sym2str(obj);

oj_dump_cstr(RSTRING_PTR(s), (int)RSTRING_LEN(s), 1, 0, out);
}
Expand Down Expand Up @@ -694,7 +694,7 @@ static void dump_struct(VALUE obj, int depth, Out out, bool as_ok) {

*out->cur++ = '[';
for (i = 0; i < cnt; i++) {
volatile VALUE s = rb_sym_to_s(rb_ary_entry(ma, i));
volatile VALUE s = rb_sym2str(rb_ary_entry(ma, i));

name = RSTRING_PTR(s);
len = (int)RSTRING_LEN(s);
Expand Down
4 changes: 2 additions & 2 deletions ext/oj/parser.c
Expand Up @@ -1217,7 +1217,7 @@ static VALUE parser_new(VALUE self, VALUE mode) {

switch (rb_type(mode)) {
case RUBY_T_SYMBOL:
mode = rb_sym_to_s(mode);
mode = rb_sym2str(mode);
// fall through
case RUBY_T_STRING: ms = RSTRING_PTR(mode); break;
default:
Expand Down Expand Up @@ -1290,7 +1290,7 @@ static VALUE parser_missing(int argc, VALUE *argv, VALUE self) {
#endif
switch (rb_type(rkey)) {
case RUBY_T_SYMBOL:
rkey = rb_sym_to_s(rkey);
rkey = rb_sym2str(rkey);
// fall through
case RUBY_T_STRING: key = rb_string_value_ptr(&rkey); break;
default: rb_raise(rb_eArgError, "option method must be a symbol or string");
Expand Down
2 changes: 1 addition & 1 deletion ext/oj/rails.c
Expand Up @@ -157,7 +157,7 @@ static void dump_struct(VALUE obj, int depth, Out out, bool as_ok) {
assure_size(out, 2);
*out->cur++ = '{';
for (i = 0; i < cnt; i++) {
volatile VALUE s = rb_sym_to_s(rb_ary_entry(ma, i));
volatile VALUE s = rb_sym2str(rb_ary_entry(ma, i));

name = RSTRING_PTR(s);
len = (int)RSTRING_LEN(s);
Expand Down
4 changes: 2 additions & 2 deletions ext/oj/usual.c
Expand Up @@ -903,7 +903,7 @@ static VALUE opt_decimal_set(ojParser p, VALUE value) {
switch (rb_type(value)) {
case T_STRING: mode = RSTRING_PTR(value); break;
case T_SYMBOL:
s = rb_sym_to_s(value);
s = rb_sym2str(value);
mode = RSTRING_PTR(s);
break;
default:
Expand Down Expand Up @@ -1020,7 +1020,7 @@ static VALUE opt_missing_class_set(ojParser p, VALUE value) {
switch (rb_type(value)) {
case T_STRING: mode = RSTRING_PTR(value); break;
case T_SYMBOL:
s = rb_sym_to_s(value);
s = rb_sym2str(value);
mode = RSTRING_PTR(s);
break;
default:
Expand Down

0 comments on commit 1deb721

Please sign in to comment.