ohler55 / oj

Optimized JSON
http://www.ohler.com/oj
MIT License
3.16k stars 253 forks source link

Some bugs #340

Closed stereobooster closed 7 years ago

stereobooster commented 7 years ago

It took me some time to pickup all settings. But I was able to setup it. And it is most compatible thing with Rails to_json, and much faster than Railsto_json. Thanks a lot for this gem.

My repo for benchmarks. During my tests found some bugs. Will report here:

Questions:

Suggestions:

Other notes: my OS is macOS Sierra.

+---------------------------------+---------------+------------------+------------------+---------------------------+
| class                           | JSON.generate | Oj.dump (object) | Oj.dump (compat) | Oj.dump (compat, as_json) |
+---------------------------------+---------------+------------------+------------------+---------------------------+
| Regexp                          | πŸ‘Œ            | ❌               | ❌               | πŸ‘Œ                        |
| FalseClass                      | πŸ‘Œ            | πŸ‘Œ               | πŸ‘Œ               | πŸ‘Œ                        |
| NilClass                        | πŸ‘Œ            | πŸ‘Œ               | πŸ‘Œ               | πŸ‘Œ                        |
| Object                          | ❌            | ❌               | πŸ‘Œ               | πŸ‘Œ                        |
| TrueClass                       | πŸ‘Œ            | πŸ‘Œ               | πŸ‘Œ               | πŸ‘Œ                        |
| String                          | πŸ‘Œ            | πŸ‘Œ               | πŸ‘Œ               | πŸ‘Œ                        |
| StringChinese                   | πŸ‘Œ            | πŸ‘Œ               | πŸ‘Œ               | πŸ‘Œ                        |
| Numeric                         | πŸ‘Œ            | πŸ‘Œ               | πŸ‘Œ               | πŸ‘Œ                        |
| Symbol                          | πŸ‘Œ            | ❌               | πŸ‘Œ               | πŸ‘Œ                        |
| Time                            | ❌            | ❌               | ❌               | πŸ‘Œ                        |
| Array                           | πŸ‘Œ            | πŸ‘Œ               | πŸ‘Œ               | πŸ‘Œ                        |
| Hash                            | πŸ‘Œ            | πŸ‘Œ               | πŸ‘Œ               | πŸ‘Œ                        |
| HashNotEmpty                    | πŸ‘Œ            | ❌               | πŸ‘Œ               | πŸ‘Œ                        |
| Date                            | πŸ‘Œ            | ❌               | πŸ‘Œ               | πŸ‘Œ                        |
| DateTime                        | ❌            | πŸ’€               | ❌               | πŸ‘Œ                        |
| Enumerable                      | ❌            | ❌               | ❌               | πŸ‘Œ                        |
| BigDecimal                      | πŸ‘Œ            | πŸ‘Œ               | πŸ‘Œ               | πŸ‘Œ                        |
| BigDecimalInfinity              | ❌            | ❌               | ❌               | πŸ‘Œ                        |
| Struct                          | ❌            | ❌               | ❌               | πŸ‘Œ                        |
| Float                           | πŸ‘Œ            | πŸ‘Œ               | πŸ‘Œ               | πŸ‘Œ                        |
| FloatInfinity                   | πŸ’€            | ❌               | πŸ‘Œ               | πŸ‘Œ                        |
| Range                           | πŸ‘Œ            | ❌               | πŸ‘Œ               | πŸ‘Œ                        |
| Process::Status                 | ❌            | ❌               | ❌               | πŸ‘Œ                        |
| ActiveSupport::TimeWithZone     | ❌            | ❌               | ❌               | πŸ‘Œ                        |
| ActiveModel::Errors             | ❌            | πŸ’€               | πŸ’€               | πŸ‘Œ                        |
| ActiveSupport::Duration         | ❌            | ❌               | ❌               | πŸ‘Œ                        |
| ActiveSupport::Multibyte::Chars | πŸ‘Œ            | ❌               | ❌               | πŸ‘Œ                        |
| ActiveRecord::Relation          | ❌            | ❌               | ❌               | πŸ‘Œ                        |
+---------------------------------+---------------+------------------+------------------+---------------------------+
ruby benchmark2.rb
Calculating -------------------------------------
            to_json:   437.048M memsize (    30.968k retained)
                         8.341M objects (   117.000  retained)
                        50.000  strings (    42.000  retained)
                 Oj:   101.720M memsize (     0.000  retained)
                         1.590M objects (     0.000  retained)
                        38.000  strings (     0.000  retained)

Comparison:
                 Oj::  101720000 allocated
            to_json::  437048167 allocated - 4.30x more

---------------------------------------------

                     user     system      total        real
to_json:         3.410000   1.090000   4.500000 (  4.544330)
Oj:              0.720000   0.010000   0.730000 (  0.727759)
stereobooster commented 7 years ago

Is there any way to load JSON first and then override three functions: JSON.parse, JSON.generate and .to_json? So JSON.parser will be handled by JSON

ohler55 commented 7 years ago

That could be done but then of course the json gem has to be loaded. If Oj can truly mimic the json gem then it should be able to mimic at least the parts users might use. Some stuff like what the name of the parser is doesn't seem that important at all though. I'll keep on pushing through.

stereobooster commented 7 years ago

Oj should not care about anything else than JSON.parse, JSON.generate and .to_json. But the same time we want to make sure that we do not break existing JSON interface. So if we can load JSON and override desired methods - that seems reasonable to me

ohler55 commented 7 years ago

Let me see how much effort it will take to handle the omits and pending. If they are fairly easy I might implement anyway. Some I'll skip though. I came across JSON.deep_const_get which really has nothing to do with JSON. It looks like something used internally. Glad to keep that omitted.

ohler55 commented 7 years ago

Working my way through the json gem unit tests. Should have it all working by the end of the weekend if all goes as planned. I have noticed that there are inconsistencies in the gem. As an example the nesting test is off by 1 between dump and generate and the unit tests only pass if the two are treated differently.

The issue I am starting to see is that the API changes between 2.3 ruby and 2.4 which makes the unit tests fail. Some of that is due to the setup I think where the json gem is loading first and defining state attributes that carry over into the tests. That can be fixed but it also mean the tests for the most recent gem are not a correct for the older json gem. Not sure how that should be addressed. If you want to give it some thought while I finish the compatibility for the latest we can discuss.

I thin I'm getting a better feel for the json gem and rails relationship as well and should be able to clearly describe the differences when I start on the rails mode.

stereobooster commented 7 years ago

First of all you deserve Open Source Hero prize (if there is one). I'm not aware of difference for 2.4 I specially checked that tests work for "json" gem. Will check latest branch.

ohler55 commented 7 years ago

Thanks. Almost done but need to add a new feature for string match and generic object. Then onto rails.

ohler55 commented 7 years ago

Would you be willing to update your benchmarks to use the new Oj on the rail-compat branch? I'd like to see the difference between using the additions and not.

stereobooster commented 7 years ago

Updated see https://github.com/stereobooster/ruby-json-benchmark/blob/master/test_report.txt

ohler55 commented 7 years ago

Thanks, I'd like to figure out what the recursion issue is with the object mode. Maybe after the Oj 3.0 release. I wasn't able to make sense out of some of the benchmarks. Maybe we can take that out of this issue and with email. My email address is peter@ohler.com.