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

Remove multi_json dependency #6

Closed
arthurnn opened this issue Apr 12, 2015 · 8 comments
Closed

Remove multi_json dependency #6

arthurnn opened this issue Apr 12, 2015 · 8 comments

Comments

@arthurnn
Copy link

A few other gems are removing multi_json dependency, as the default JSON gem has a good performance too.
Do you consider removing it as a dependency from this gem too?
Did you try it already? and it didnt perform well?

If the answer is Yes, I can send a PR. let me know.

@trevorrowe
Copy link
Contributor

I would be more inclined to replace the MultiJson dependency with a thin shim that provides the ability to use the fastest available JSON parsing engine, such as Oj.

Is there a particular reason you would prefer to see it removed?

@arthurnn
Copy link
Author

Is there a particular reason you would prefer to see it removed?

No particular one. I am just bringing up the point as a lot of gems out there are removing the dependency.

@sferik
Copy link
Contributor

sferik commented Apr 23, 2015

I would be more inclined to replace the MultiJson dependency with a thin shim that provides the ability to use the fastest available JSON parsing engine, such as Oj.

What you have described is MultiJson.

Is there a particular reason you would prefer to see it removed?

I was once a core maintainer of MultiJson. I now try to dissuade people from using it. MultiJson made sense at a time when JSON wasn’t part of the Ruby standard library. During this time (before MultiJson), every library that needed to parse JSON chose its own JSON parser (from oj, yajl, json, json-pure, gson, jrjackson, etc.). At the time, it wasn’t uncommon for a single application to transitively depend on 4 different JSON parsers.

MultiJson solved this problem by encouraging library maintainers to use MultiJson instead of a specific JSON parser, allowing the application developer to specify a single JSON parser that was right for their application/platform. If the application developer bundled multiple JSON parsers, MultiJson would choose the fastest one. If the application developer didn’t bundle any, MultiJson would fallback to okjson, an embedded JSON parser.

When Ruby 1.9.2 was released in August 2010, the decision was made by the Ruby core team to bless the json gem and make it part of the Ruby standard library. Once this decision was made, the json gem (a.k.a. stdlib json) became the de-facto choice of the Ruby community. Everybody had json, regardless of whether they wanted it or not.

Since we are stuck with stdlib json, I believe we should put all our wood behind that arrow. If we (as a community), believe that oj is superior to json, we should convince the Ruby core team to replace json with oj. This is exactly what happened when FasterCSV replaced CSV in the standard library.

MultiJson served a purpose when there was no official JSON library. That time has long since passed. If stdlib json doesn’t meet your needs, please open an issue with the json team. Any improvements made to json will benefit the entire Ruby community. Using a different JSON library sweeps the problem under the rug and fragments the community.

@sferik
Copy link
Contributor

sferik commented Apr 24, 2015

I hope I didn’t leave you with the impression that I don’t support libraries like oj or that I prefer json over oj. In fact, the opposite is true. I am a big fan of oj and hope it replaces json in the standard library some day (with a few tweaks made to ensure compatibility, just as was done with FasterCSV).

@trevorrowe
Copy link
Contributor

I would be more inclined to replace the MultiJson dependency with a thin shim that provides the ability to use the fastest available JSON parsing engine, such as Oj.

What you have described is MultiJson.

Most of these comments apply more to the aws/aws-sdk-ruby repositry, not as much to jmespath. I'm leaving them here, to keep them local to the discussion.

The SDK does not expose the MultiJson interface to the user. This limits the surface area of what the shim would need to provide. Additionally, I am aware of a number of latency sensitive applications where time spent parsing JSON documents is very critical, such as with DynamoDB. Multiple customs rely on being able to load Oj at runtime. Parsing 1MB JSON documents with thousands of smaller objects has measurable difference.

Given the small surface area required, I would like to explore removing the MultiJson dependency, while preserving the faster parsing options until at such time as Oj becomes standard.

@sferik
Copy link
Contributor

sferik commented Apr 24, 2015

Given the small surface area required, I would like to explore removing the MultiJson dependency, while preserving the faster parsing options until at such time as Oj becomes standard.

If you’re able to achieve this, I’d encourage you to contribute this code to MultiJson.

@trevorrowe
Copy link
Contributor

Sorry for the slow response. I've been out of the office on travel. I'm back in town now.

The jmespath gem does not do any heavy marshaling or unmarshaling of JSON objects. It simply uses the parse method to deal with JSON literals. Merging this change for this particular repo is fine and should have any major performance impact.

I'll respond to the multi-parser issue in the other repo. Thanks for your patience.

@trevorrowe
Copy link
Contributor

Closed by #7

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

No branches or pull requests

3 participants