Open cyberphone opened 5 years ago
Hi @cyberphone , isnt it an implementation optimization? Then it belongs to yasson bugtracker.
Side note: current serialization must stay since we got a 1.0 so if not an implementation detail you can do ryu adapters to achieve it.
@rmannibucau I don't know where this "belongs" but both Go and C# is in the process of replacing their current number serializers with Ryu.
My personal interest is more on the ES6 compatibility side than on performance.
Hmm, can you point out es6 - even es5? - incompatibilities maybe? I used it quick a lot with primitives already and issues didnt pop up yet both ways.
ES6 compatibility is only needed for canonicalization. I just hoped to get this as a "bonus" 😀 since JCS is not a target for JSONB. The speed improvement was pretty impressive.
You'll find all links in the Internet-Draft.
https://github.com/ulfjack/ryu states that the java impl output may differ from the Double#toString methods. Can that break jsonb-spec 3.3.2 section?
Looks it is compatible but it is also a draft so quite bad for a jakata spec - keep in mind jsonschema which is in draft already broke features. Also nothing requires an impl to use valueOf - guess they all do but this is not required AFAIK since not part of user facing API - this is why it is an impl detail for me.
So only question is for me the json number representation and while any round trip (java/json) works - which means js can consume it - I guess we are covered at spec level.
If a new final json spec pops up it could become a toggle - config property and annotation - IMHO.
Does it make sense?
@rmannibucau JS (as well as any correctly implemented JSON parser), can consume both your existing and proposed format, it is only canonicalization that requires absolute ES6 compliance.
Regarding JDK roundtripping, the ES6/Ryu adaptation succeeds using my 100M test file: https://github.com/cyberphone/json-canonicalization/tree/master/testdata#es6-numbers
I would not bother with a toggle since the "problem" rather is in the spec.
The test program failed on C# but it turned out to be due to a bug in the .NET number parser. After reporting it, Microsoft fixed it as well!
JSON Canonicalization now works on 5 platforms.
Hmm, my point is I fail to see the problem in the spec. To be clear I can see it in some implementations but not the spec.
@cyberphone What version of https://github.com/ulfjack/ryu is used in your comparison table? I've grabbed java sources from their master and have different output. Scientific notation E
sign is a "big E" instead of small and is not followed by a '+' sign. I run the test for "Selected values" from your table and the output differs only for one line:
IEEE-754 Double#toString RyuDouble#doubleToString
44b52d02c7e14af6 9.999999999999999E22 1.0E23
@bravehorsie I took the Java version as it was 6 month ago and modified formatting slightly to make it comply with ES6 rules.
It is just one file/class: https://github.com/cyberphone/openkeystore/blob/master/library/src/org/webpki/json/NumberToJSON.java
@cyberphone I have tried your Ryu extraction and it produces the values as is in the table. That is different from what current java Ryu master code produces. Why I am mentioning this is that in your NumberToJSON
the output breaks the contract specified by the Double#toString(double d)
javadoc, which in turn breaks the jsonb-spec 3.3.2 section.
@bravehorsie That's correct, my variation follows the ES6 specification which is implemented in all browsers and Node.js. They are fortunately both 100% JSON compatible so from an interoperability point of view it doesn't matter what you select unless somebody is using a dedicated non-compliant parser. I would consider a spec upgrade but that is of course for the specification committee to decide.
This recently published RFC https://tools.ietf.org/html/rfc8785 also builds on the Ryu/EcmaScript algorithm.
@cyberphone I'm not sure we can break the spec that much - actually I hope we don't break it like that - but did you evaluate the option to impl a ryu-jsonb-de/serializer? Would kind of make everyone happy I think and avoid endless discussion about backward compatibility, a potential new flag hard to justify today and things like that. If ryu-jsonb-integration is used a lot then the spec could check back if a flag is worth it IMHO. Wdyt?
@rmannibucau The spec has gained traction so I must concentrate on the next step which is RFC-ing JSF: https://mobilepki.org/jsf-lab/home
The Ryu algorithm (https://github.com/ulfjack/ryu) for IEEE-754 serialization offer a number of interesting features such a simple, fast and (with a minor tweak) 100% compatible with ES6. Using the Ryu algorithm also makes https://tools.ietf.org/html/draft-rundgren-json-canonicalization-scheme-06 more realistic for JSONB, particularly with the proposed upgrade in https://github.com/eclipse-ee4j/jsonp/issues/160. Here is a comparison made between String.valueOf(double) and my Ryu adaption for Java where each value has been serialized 1M times.