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

Compability with ES6 / V8 #87

Open
cyberphone opened this issue Jan 13, 2019 · 12 comments
Open

Compability with ES6 / V8 #87

cyberphone opened this issue Jan 13, 2019 · 12 comments
Labels
enhancement New feature or request Java Affects the Java implementation in src/main/java.

Comments

@cyberphone
Copy link

I'm working with something that possibly will become an IETF standard:
https://tools.ietf.org/html/draft-rundgren-json-canonicalization-scheme-02
It depends on number formatting compatible with ES6/V8.

A short test revealed that the Java version of Ryu is not entirely compatible with this scheme.
x0000000000000001 returns 5e-324 in ES6/V8 while 4.9e-324 in Ryu

This is not really a bug but could be worth knowing.

https://github.com/cyberphone/json-canonicalization/tree/master/testdata#es6-numbers

@plokhotnyuk
Copy link
Contributor

plokhotnyuk commented Jan 13, 2019

@cyberphone btw, sorting of properties during serialization of JSON is a quite inefficient approach, that can eat all gain of using the great Ryu algorithm.

@cyberphone
Copy link
Author

@plokhotnyuk well, this "bug" report only deals with the mathematical differences between Ryu and other algorithms including the one in Python, ES6, and Go. I would be a bit concerned if the Go folks swapped their current (partly buggy) algorithm for Ryu. golang/go#29491 (comment)

Property sorting is unfortunately unavoidable for obtaining a canonical form.

@ulfjack
Copy link
Owner

ulfjack commented Jan 14, 2019

The Java implementation attempts to follow the specification of Java's Double.toString [1], which says that it must return at least two digits. Out of all two-digit numbers, 4.9e-324 is closest to the exact floating point value in this case, so that's what it returns. The problem described in #83 is that it sometimes doesn't return the closest two-digit number because there's a one-digit number that's shorter. The C implementation doesn't do that, and I have not found any differences between it and Grisu over a fairly large number of tests.

It's unfortunate that Java's spec differs from everyone else in this regard, but what can you do. You could easily add a flag to allow the Java impl to return the other value instead.

[1] https://docs.oracle.com/javase/7/docs/api/java/lang/Double.html#toString(double)

There must be at least one digit to represent the fractional part

ulfjack added a commit that referenced this issue Jan 14, 2019
As noted in #87, the Java implementation doesn't match existing and upcoming specs for floating point output that require the shortest output. The implementation here attempts to follow Double.toString, which requires at least two digits of output.
@ulfjack
Copy link
Owner

ulfjack commented Jan 14, 2019

Thanks for bringing this up. I added a note to the README which hopefully clarifies the current situation. I'll leave this open as a feature request to add a flag to the Java implementation to generate the shortest output rather than outputting at least two digits.

@ulfjack ulfjack added enhancement New feature or request Java Affects the Java implementation in src/main/java. labels Jan 14, 2019
@cyberphone
Copy link
Author

@ulfjack Thanx for the explanation! The 100M test suite fully validated with the Go implementation here: https://github.com/remyoudompheng/go/tree/ryu

Since ES6 JSON number formatting is quite different (-0 is equal to 0, "g" with specific range, forbidden Nan/Infinity), I guess making a dedicated port of your Java code is my best option if I wanted to achieve the same performance improvement as I experienced in Go? I did a port of another library for .NET which also could use an update: https://github.com/cyberphone/json-canonicalization/tree/master/dotnet/es6numberserializer

@cyberphone
Copy link
Author

cyberphone commented Jan 17, 2019

Did a dedicated port in 3 hours and it worked like a charm!
Will soon replace the original and pretty complex V8-port done by Mozilla.
Great work BTW!

I will refer to Ryu in the next I-D revision as a recommend algorithm for JSON/JCS serialization.

https://github.com/cyberphone/json-canonicalization/blob/master/java/canonicalizer/src/org/webpki/jcs/NumberToJSON.java

@ulfjack
Copy link
Owner

ulfjack commented Jan 17, 2019

Cool, thanks!

@croraf
Copy link

croraf commented Jun 18, 2020

Is ryu moving forward on being ECMAScript specification compliant https://tc39.es/ecma262/#sec-numeric-types-number-tostring or?

@cyberphone
Copy link
Author

@croraf that would be cool because then RYU would be compatible with RFC 8785 (JSON Canonicalization Scheme) that will be published soon (it is the RFC editors' queue).
https://www.rfc-editor.org/authors/rfc8785.htm

@ulfjack
Copy link
Owner

ulfjack commented Jun 19, 2020

I was hoping to provide a low-level API that could be used to generate multiple formats, but at least in the C version, that has a measurable performance cost, which - so far - I haven't been willing to pay. That said, for the C side, adding a compile-time symbol to select between formats (libstdc++ / JSON) certainly seems reasonable. Java doesn't really have compile-time settings, so it would have to be a runtime setting.

Unfortunately, I have been entirely preoccupied with running my own company, with precious little time to dedicate to Ryu. :-/

@croraf
Copy link

croraf commented Jun 19, 2020

@ulfjack perhaps the community can do the heavy-lifting, the important is your decision on the direction to take.

@ulfjack
Copy link
Owner

ulfjack commented Jun 24, 2020

I want the code to be fast, and also widely useable. It's been integrated into some C++ standard libraries, so it needs - at least - to be configurable such that it's compatible with that. As I said, adding a compile-time symbol to the C version seems fine to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Java Affects the Java implementation in src/main/java.
Projects
None yet
Development

No branches or pull requests

4 participants