From 29c3da207b69ee439f0aae4b9619bcd424c31449 Mon Sep 17 00:00:00 2001 From: Peter Ohler Date: Sun, 11 Apr 2021 14:17:00 -0400 Subject: [PATCH 1/2] Work around the special case of rails and generate --- CHANGELOG.md | 5 +++++ ext/oj/dump.c | 4 ++-- ext/oj/dump_compat.c | 16 ++++++++++++++++ ext/oj/mimic_json.c | 5 ++++- ext/oj/oj.h | 3 ++- notes | 6 ++++++ 6 files changed, 35 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73b17e34..246a25b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,13 @@ - Code re-formatted with clang-format. Thanks goes to BuonOmo for suggesting and encouraging the use of a formatter and getting the effort started. + - Added support for `GC.compact` on `Oj::Doc` +- Fixed compatibility issue with Rails and the JSON gem that requires + a special case when using JSON.generate versus call `to_json` on an + object. + ## 3.11.3 - 2021-03-09 - Fixed `respond_to?` method on `Oj::EasyHash`. diff --git a/ext/oj/dump.c b/ext/oj/dump.c index 56e387df..ded508df 100644 --- a/ext/oj/dump.c +++ b/ext/oj/dump.c @@ -599,8 +599,8 @@ void oj_dump_obj_to_json(VALUE obj, Options copts, Out out) { void oj_dump_obj_to_json_using_params(VALUE obj, Options copts, Out out, int argc, VALUE *argv) { if (0 == out->buf) { out->buf = ALLOC_N(char, 4096); - out->end = out->buf + 4095 - - BUFFER_EXTRA; // 1 less than end plus extra for possible errors + // 1 less than end plus extra for possible errors + out->end = out->buf + 4095 - BUFFER_EXTRA; out->allocated = true; } out->cur = out->buf; diff --git a/ext/oj/dump_compat.c b/ext/oj/dump_compat.c index a5fac992..fbe90f51 100644 --- a/ext/oj/dump_compat.c +++ b/ext/oj/dump_compat.c @@ -120,6 +120,22 @@ dump_to_json(VALUE obj, Out out) { if (Yes == out->opts->trace) { oj_trace("to_json", obj, __FILE__, __LINE__, 0, TraceRubyIn); } + if (out->generate) { + // There is some intricate monkey patching going on between rails and + // the json gem to in order to mimic the interaction we have to keep + // track of how the encoding was called. If from JSON.generate then + // skip and ActiveSupport to_json and outout as a string. + VALUE method = rb_obj_method(obj, ID2SYM(rb_intern("to_json"))); + VALUE owner = rb_funcall(method, rb_intern("owner"), 0); + + if (0 == strncmp(rb_class2name(owner), "ActiveSupport::", 15)) { + oj_dump_obj_to_s(obj, out); + if (Yes == out->opts->trace) { + oj_trace("to_json", obj, __FILE__, __LINE__, 0, TraceRubyOut); + } + return; + } + } if (0 == rb_obj_method_arity(obj, oj_to_json_id)) { rs = rb_funcall(obj, oj_to_json_id, 0); } else { diff --git a/ext/oj/mimic_json.c b/ext/oj/mimic_json.c index 483e7f6b..fd1e0e93 100644 --- a/ext/oj/mimic_json.c +++ b/ext/oj/mimic_json.c @@ -211,6 +211,7 @@ static VALUE mimic_dump(int argc, VALUE *argv, VALUE self) { out.buf = buf; out.end = buf + sizeof(buf) - 10; out.allocated = false; + out.generate = false; out.caller = CALLER_DUMP; copts.escape_mode = JXEsc; copts.mode = CompatMode; @@ -371,6 +372,7 @@ static VALUE mimic_generate_core(int argc, VALUE *argv, Options copts) { out.allocated = false; out.omit_nil = copts->dump_opts.omit_nil; out.caller = CALLER_GENERATE; + out.generate = true; // For obj.to_json or generate nan is not allowed but if called from dump // it is. copts->dump_opts.nan_dump = RaiseNan; @@ -746,6 +748,7 @@ static VALUE mimic_object_to_json(int argc, VALUE *argv, VALUE self) { out.end = buf + sizeof(buf) - 10; out.allocated = false; out.omit_nil = copts.dump_opts.omit_nil; + out.generate = false; copts.mode = CompatMode; copts.to_json = No; if (1 <= argc && Qnil != argv[0]) { @@ -755,7 +758,7 @@ static VALUE mimic_object_to_json(int argc, VALUE *argv, VALUE self) { // seem to prefer the option of changing that. // oj_dump_obj_to_json(self, &mimic_object_to_json_options, &out); oj_dump_obj_to_json_using_params(self, &copts, &out, argc, argv); - if (0 == out.buf) { + if (NULL == out.buf) { rb_raise(rb_eNoMemError, "Not enough memory."); } rstr = rb_str_new2(out.buf); diff --git a/ext/oj/oj.h b/ext/oj/oj.h index a49da513..adf5bd83 100644 --- a/ext/oj/oj.h +++ b/ext/oj/oj.h @@ -185,6 +185,7 @@ typedef struct _out { uint32_t hash_cnt; bool allocated; bool omit_nil; + bool generate; // indicating dump by JSON.generate int argc; VALUE * argv; DumpCaller caller; // used for the mimic json only @@ -247,7 +248,7 @@ extern void oj_parse_options(VALUE ropts, Options copts); extern void oj_dump_obj_to_json(VALUE obj, Options copts, Out out); extern void - oj_dump_obj_to_json_using_params(VALUE obj, Options copts, Out out, int argc, VALUE *argv); +oj_dump_obj_to_json_using_params(VALUE obj, Options copts, Out out, int argc, VALUE *argv); extern void oj_write_obj_to_file(VALUE obj, const char *path, Options copts); extern void oj_write_obj_to_stream(VALUE obj, VALUE stream, Options copts); extern void oj_dump_leaf_to_json(Leaf leaf, Options copts, Out out); diff --git a/notes b/notes index e8e609a4..1db27383 100644 --- a/notes +++ b/notes @@ -3,6 +3,12 @@ ^c^d hide subtree ^c^s show subtree +- todo + - keep a flag on out options to note that it is json-generate + - in dump_compat.c:dump_to_json + - try with arrays and hash as well as just objects + + Strict Dump Performance JSON::Ext.dump 20000 times in 0.173 seconds or 115493.546 dump/sec. Oj:strict.dump 20000 times in 0.050 seconds or 401475.753 dump/sec. From 74f8fb6c9385ea37d363941e508fbd2d76d92b7d Mon Sep 17 00:00:00 2001 From: Peter Ohler Date: Tue, 13 Apr 2021 19:57:32 -0400 Subject: [PATCH 2/2] Alternate approach to generate issue --- ext/oj/dump_compat.c | 16 ---------------- ext/oj/mimic_json.c | 11 +++++++---- ext/oj/oj.h | 1 - notes | 6 ------ 4 files changed, 7 insertions(+), 27 deletions(-) diff --git a/ext/oj/dump_compat.c b/ext/oj/dump_compat.c index fbe90f51..a5fac992 100644 --- a/ext/oj/dump_compat.c +++ b/ext/oj/dump_compat.c @@ -120,22 +120,6 @@ dump_to_json(VALUE obj, Out out) { if (Yes == out->opts->trace) { oj_trace("to_json", obj, __FILE__, __LINE__, 0, TraceRubyIn); } - if (out->generate) { - // There is some intricate monkey patching going on between rails and - // the json gem to in order to mimic the interaction we have to keep - // track of how the encoding was called. If from JSON.generate then - // skip and ActiveSupport to_json and outout as a string. - VALUE method = rb_obj_method(obj, ID2SYM(rb_intern("to_json"))); - VALUE owner = rb_funcall(method, rb_intern("owner"), 0); - - if (0 == strncmp(rb_class2name(owner), "ActiveSupport::", 15)) { - oj_dump_obj_to_s(obj, out); - if (Yes == out->opts->trace) { - oj_trace("to_json", obj, __FILE__, __LINE__, 0, TraceRubyOut); - } - return; - } - } if (0 == rb_obj_method_arity(obj, oj_to_json_id)) { rs = rb_funcall(obj, oj_to_json_id, 0); } else { diff --git a/ext/oj/mimic_json.c b/ext/oj/mimic_json.c index fd1e0e93..c6a9cc08 100644 --- a/ext/oj/mimic_json.c +++ b/ext/oj/mimic_json.c @@ -211,7 +211,6 @@ static VALUE mimic_dump(int argc, VALUE *argv, VALUE self) { out.buf = buf; out.end = buf + sizeof(buf) - 10; out.allocated = false; - out.generate = false; out.caller = CALLER_DUMP; copts.escape_mode = JXEsc; copts.mode = CompatMode; @@ -372,7 +371,6 @@ static VALUE mimic_generate_core(int argc, VALUE *argv, Options copts) { out.allocated = false; out.omit_nil = copts->dump_opts.omit_nil; out.caller = CALLER_GENERATE; - out.generate = true; // For obj.to_json or generate nan is not allowed but if called from dump // it is. copts->dump_opts.nan_dump = RaiseNan; @@ -386,8 +384,14 @@ static VALUE mimic_generate_core(int argc, VALUE *argv, Options copts) { rb_raise(rb_eTypeError, "nil not allowed."); } */ - oj_dump_obj_to_json_using_params(*argv, copts, &out, argc - 1, argv + 1); + if (1 < argc) { + oj_dump_obj_to_json_using_params(*argv, copts, &out, argc - 1, argv + 1); + } else { + VALUE active_hack[1]; + active_hack[0] = rb_funcall(state_class, oj_new_id, 0); + oj_dump_obj_to_json_using_params(*argv, copts, &out, 1, active_hack); + } if (0 == out.buf) { rb_raise(rb_eNoMemError, "Not enough memory."); } @@ -748,7 +752,6 @@ static VALUE mimic_object_to_json(int argc, VALUE *argv, VALUE self) { out.end = buf + sizeof(buf) - 10; out.allocated = false; out.omit_nil = copts.dump_opts.omit_nil; - out.generate = false; copts.mode = CompatMode; copts.to_json = No; if (1 <= argc && Qnil != argv[0]) { diff --git a/ext/oj/oj.h b/ext/oj/oj.h index adf5bd83..de14eaeb 100644 --- a/ext/oj/oj.h +++ b/ext/oj/oj.h @@ -185,7 +185,6 @@ typedef struct _out { uint32_t hash_cnt; bool allocated; bool omit_nil; - bool generate; // indicating dump by JSON.generate int argc; VALUE * argv; DumpCaller caller; // used for the mimic json only diff --git a/notes b/notes index 1db27383..e8e609a4 100644 --- a/notes +++ b/notes @@ -3,12 +3,6 @@ ^c^d hide subtree ^c^s show subtree -- todo - - keep a flag on out options to note that it is json-generate - - in dump_compat.c:dump_to_json - - try with arrays and hash as well as just objects - - Strict Dump Performance JSON::Ext.dump 20000 times in 0.173 seconds or 115493.546 dump/sec. Oj:strict.dump 20000 times in 0.050 seconds or 401475.753 dump/sec.