search

bchavez / RethinkDb.Driver

:headphones: A NoSQL C#/.NET RethinkDB database driver with 100% ReQL API coverage.
http://rethinkdb.com/api/java
Other
384 stars 134 forks source link

Jil support #33

Open oliverjanik opened 8 years ago

oliverjanik commented 8 years ago

Would it make sense to to swap out Json.NET for Jil or provide alternative json implementation?

https://github.com/kevin-montrose/Jil

bchavez commented 8 years ago

Hi @oliverjanik ,

Thanks very much for your suggestion. I think you have a good idea. It would be a good thing to allow support for alternative JSON implementations. Currently, the Java driver is exploring something like this too: https://github.com/rethinkdb/rethinkdb/issues/5128

However, currently the driver relies on some Newtonsoft feature-magic (objects like JObject, JArray, and JToken) to get its work done. It would take some effort to re-factor the source to allow multiple JSON implementations, but should be do-able.

EDIT: Requires #77 to be implemented first.

Updated Benchmarks (thanks to @marshall007): http://theburningmonk.com/2016/01/binary-and-json-benchmarks-updated-4/

oliverjanik commented 8 years ago

It's good to hear you're open to suggestions.

Newtonsoft is one of the slowest json serializers around and has some crazy baggage (e.g. https://github.com/JamesNK/Newtonsoft.Json/issues/525)

bchavez commented 8 years ago

Hey Oliver, yeah. I totally understand. I've heard some arguments on both sides regarding Newtonsoft. Microsoft blessed Newtonsoft as the default JSON ser/des in ASP.NET MVC and it was one reason I picked it as the default here.

Currently, I don't have any immediate needs for an alternative JSON implementation right now, so this is a bit lower priority issue for me. However, I'd gladly help with guidance and accept a PR if you or anyone want to take a stab at it.

Side note: It would be interesting to see an updated version of the benchmarks. Looks like the benchmarks were ran against v6.0.7 some time ago. I think we're at v8+ now. I wonder if Newtonsoft made any progress at closing the gap since v6.0.7.

jakejscott commented 8 years ago

I think they are always working on improving performance of it http://james.newtonking.com/archive/2015/12/20/json-net-8-0-release-1-allocations-and-bug-fixes

jakejscott commented 8 years ago

And it's not too bad if you look at some benchmarks (which already look out of date) http://theburningmonk.com/benchmarks/

oliverjanik commented 8 years ago

It's less the speed, more the wild and inconsistent API and true WTF moments (like the one I linked to) for me.

Call me sceptical but a library that modifies! my timestamp by just loading json as JObject is an absolute no-go. Moreover there was zero discussion and the issue closed as won'tfix.