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

Does MultiJson introduce slowness? #113

Merged
merged 1 commit into from
May 10, 2013
Merged

Does MultiJson introduce slowness? #113

merged 1 commit into from
May 10, 2013

Conversation

rwz
Copy link
Member

@rwz rwz commented May 9, 2013

I could be using the library incorrectly, but doing a very basic test, I see results that aren't really that great.

I've got a test and results in the following Gist:
https://gist.github.com/AaronRustad/d6f20229b9f19f6d163e

If I'm doing something incorrectly that is causing these numbers to appear worse than they should, please let me know.

Thanks!

@sferik
Copy link
Member

sferik commented May 9, 2013

Preamble
Therefore, be it resolved, effective July 2013
  • I shall cease to maintain MultiJSON. (The library may continue to be maintained by Intridea, Inc., Michael Bleigh, Pavel Pravosud, Josh Kalderimis, and other volunteers from the Ruby community.)
  • I shall require a Ruby version of at least 1.9.2 in all libraries that I continue to maintain.
  • I shall replace the multi_json dependency in all other libraries I maintain with json.
  • I shall submit a pull request to Ruby on Rails that replaces the multi_json dependency with json.

@AaronRustad Thank you for creating this benchmark and opening this issue. After doing some additional benchmarking of my own and giving it much thought, I am convinced that the tradeoffs that motivated this library in 2010 are no longer worthwhile in 2013.

@AaronRustad
Copy link
Author

@sferik Thanks for the update. And thanks for your work on this in the past and up until now.

@rwz
Copy link
Member

rwz commented May 9, 2013

@sferik wow... Just wow.

@baldrailers
Copy link

@sferik 👍

@AlexRiedler
Copy link

This test is slightly flawed because it is such a small JSON object, as a result the overhead of initiating the call is exaggerated in the results (compared to most regular use cases). There is still for sure some performance improvements to MultiJSON that can be done.

Abandoning the project and moving to the built-in is a valid way to deal with this problem; but there is a lot of libraries that depend on MultiJSON; if this is you taking a stance and telling them to use the built-in and follow suit 👍 otherwise I will start becoming a contributor to see if we can make MultiJSON not have these performance concerns.

Best Regards!
Alex

@sferik
Copy link
Member

sferik commented May 9, 2013

@AlexRiedler For what it’s worth, I modified the benchmarks to dump and load larger chunks of JSON and the overhead was still significant.

@rwz
Copy link
Member

rwz commented May 9, 2013

I have unfinished branch with refactoring that improves performance by about 100% in this benchmark. It's not ready yet, but I expect to finish and push it quite soon.

@AlexRiedler
Copy link

@sferik I did as well, and actually ran some ruby-prof benchmarks as well to see where in multi-json the overhead is coming from; some of it may be a little tricky to remove but I might still look into it.

@rwz 👍

@rwz
Copy link
Member

rwz commented May 9, 2013

These are the results of benchmark on my MBA before and after the performance path:

before:

                        user     system      total        real
multi/json          2.200000   0.000000   2.200000 (  2.208964)
json                0.420000   0.010000   0.430000 (  0.418784)
multi/oj            1.210000   0.020000   1.230000 (  1.232994)
oj                  0.120000   0.010000   0.130000 (  0.140023)
multi/yajl          1.360000   0.010000   1.370000 (  1.361921)
yajl                0.350000   0.010000   0.360000 (  0.361245)
                        user     system      total        real
multi/json          1.520000   0.000000   1.520000 (  1.520601)
json                0.410000   0.000000   0.410000 (  0.416693)
multi/oj            1.230000   0.000000   1.230000 (  1.232759)
oj                  0.070000   0.000000   0.070000 (  0.067810)
multi/yajl          2.080000   0.010000   2.090000 (  2.090170)
yajl                0.350000   0.000000   0.350000 (  0.351001)

after:

                        user     system      total        real
multi/json          0.850000   0.000000   0.850000 (  0.857107)
json                0.630000   0.010000   0.640000 (  0.660886)
multi/oj            0.540000   0.020000   0.560000 (  0.556939)
oj                  0.120000   0.010000   0.130000 (  0.130667)
multi/yajl          0.980000   0.010000   0.990000 (  0.993099)
yajl                0.590000   0.010000   0.600000 (  0.611344)
                        user     system      total        real
multi/json          1.410000   0.010000   1.420000 (  1.436850)
json                0.430000   0.000000   0.430000 (  0.426912)
multi/oj            0.380000   0.000000   0.380000 (  0.388255)
oj                  0.050000   0.000000   0.050000 (  0.042039)
multi/yajl          0.790000   0.010000   0.800000 (  0.795145)
yajl                0.370000   0.000000   0.370000 (  0.378943)

@mbleigh
Copy link
Member

mbleigh commented May 9, 2013

I'll weigh in on this weighty issue. @sferik first thank you 1000x for all of your hard work on this over the years. I started MultiJson because at the time there were a ton of JSON libraries that were all being used in various projects and I didn't want my own open source to demand that users pick one over another. It was a library for library authors, and that's still how I see it.

I can certainly see sense in dropping down to the baked-in default now that json is a part of Ruby itself. I'm not sure if there's still a place for MultiJson in a post-1.8.7 world (and reserve the right to continue thinking on that for a while).

One thing I can say: addressing performance concerns is of the utmost priority moving forward with this library. The primary reason that people swap between JSON libraries is for performance, and if MultiJson is negatively impacting that performance than it is defeating the purpose in many ways. So thanks @rwz for starting to address this and I hope that other perf experts will chime in to get the code as optimized as possible.

I think to a large extent the longevity of MultiJson depends on how well these performance overhead concerns can be addressed.

@stve
Copy link

stve commented May 9, 2013

⛳ 👏 @sferik

@bf4
Copy link

bf4 commented Jun 3, 2013

I really like how multi_json could ensure you have a working json library across ruby vm's when requiring it as a gem dependency. e.g. metricfu/metric_fu@454b70d

@karmi
Copy link

karmi commented Jun 30, 2013

(...) there were a ton of JSON libraries that were all being used in various projects and I didn't want my own open source to demand that users pick one over another.

@mbleigh Very well put -- this is still true, in my opinion, regardless of “native” JSON support in Ruby 1.9 and higher. MultiJson is not a JSON serializer/deserializer, and provides a very good abstraction and common interface over JSON libraries, allowing users to pick the concrete implementations without changing the interface. Without an abstraction like this, library authors would have to provide this abstraction themselves.

zimbatm pushed a commit to zimbatm/raven-ruby that referenced this pull request Jul 9, 2013
zimbatm pushed a commit to zimbatm/raven-ruby that referenced this pull request Jul 9, 2013
zimbatm pushed a commit to zimbatm/raven-ruby that referenced this pull request Jul 9, 2013
The author of multi_json is deprecating the library and recommends using the
'json' gem[1]. It also fixes issues where MultiJson calls #to_json and
ActiveRecord::JSON implements IO#to_json to read on the file (which might fail
if the IO is opened on read-only).

[1]: intridea/multi_json#113
@karmi
Copy link

karmi commented Jul 21, 2013

@mbleigh @rwz Any news about this issue? Can we expect MultiJson to continue being maintained? Is the benchmark script somewhere accessible so people can run it for themselves?

@rwz
Copy link
Member

rwz commented Jul 23, 2013

@karmi The benchmark was a very straightforward and simple, just load/dump a small object back and forth.

The performance patch I've implemented made MultiJson significantly faster. It's still a bit slower than using the gems directly, but not by far.

I continue maintaining the gem.

@zimbatm
Copy link

zimbatm commented Jul 23, 2013

Also I believe that multijson's overhead is constant isn't it ? With larger
json objects the indirection cost would still be the same.
On Jul 23, 2013 8:38 AM, "Pavel Pravosud" notifications@github.com wrote:

@karmi https://github.com/karmi The benchmark was a very
straightforward and simple, just load/dump a small object back and forth.

The performance patch I've implemented made MultiJson significantly
faster. It's still a bit slower than using the gems directly, but not by
far.

I continue maintaining the gem.


Reply to this email directly or view it on GitHubhttps://github.com//pull/113#issuecomment-21397818
.

kainosnoema added a commit to crepe/crepe that referenced this pull request Dec 25, 2013
MultiJson isn't all its cracked up to be. It can introduce slowness,
its been removed as a dependency in activesupport, and there's now
full, fast support for JSON in Ruby 1.9+.

More info: intridea/multi_json#113

The fewer dependencies Crepe relies on, the better. If you need
super-duper fast JSON in your app, use the YAJL gem and simply
`require 'yajl/json_gem'` for full compatibility (though Ruby's JSON
is now faster then the YAJL readme declares).

Signed-off-by: Evan Owen <kainosnoema@gmail.com>
foca added a commit to foca/granola that referenced this pull request Feb 23, 2015
@drizzt
Copy link

drizzt commented Dec 11, 2015

I still prefer multi_json because stdlib "json" is slow and sometimes broken (with non-ASCII responses). Moreover multi_json is still supported and updated and supports different backends (like oj)

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.