RFR: 4511638: Double.toString(double) sometimes produces incorrect results
Hello, here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing. The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions. Greetings Raffaello ------------- Commit messages: - 4511638: Double.toString(double) sometimes produces incorrect results Changes: https://git.openjdk.java.net/jdk/pull/3402/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-4511638 Stats: 4237 lines in 15 files changed: 4116 ins; 54 del; 67 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Forgot to add that other changes in the code are the use of switch expressions and the use of instanceof patterns. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Thu, 8 Apr 2021 21:20:34 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Forgot to add that other changes in the code are the use of switch expressions and the use of instanceof patterns.
Hello, here's some background information for those that didn't follow the mailing list for the last couple of years. Some enjoyable properties of the novel algorithm: * No intermediate objects are instantiated. * Loop-free core algorithm. * Only int and long arithmetic. * No divisions at all. * 17.7x speedup (jmh) [1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-April/065921.html). * Randomized, yet reproducible deep diving tests (jtreg). * Clear, unambiguous spec. * All floats have been tested to fully meet the spec. * Fully documented in [2](https://drive.google.com/file/d/1luHhyQF9zKlM8yJ1nebU0OgVYhfC6CBN/view) and/or in comments. See [3](https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-September/062580....) for some (invented) Q&A. The last Q&A deals with your investment in time for an informed review. Greetings Raffaello ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 9 Apr 2021 16:33:43 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Forgot to add that other changes in the code are the use of switch expressions and the use of instanceof patterns.
Hello,
here's some background information for those that didn't follow the mailing list for the last couple of years.
Some enjoyable properties of the novel algorithm: * No intermediate objects are instantiated. * Loop-free core algorithm. * Only int and long arithmetic. * No divisions at all. * 17.7x speedup (jmh) [1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-April/065921.html). * Randomized, yet reproducible deep diving tests (jtreg). * Clear, unambiguous spec. * All floats have been tested to fully meet the spec. * Fully documented in [2](https://drive.google.com/file/d/1luHhyQF9zKlM8yJ1nebU0OgVYhfC6CBN/view) and/or in comments.
See [3](https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-September/062580....) for some (invented) Q&A. The last Q&A deals with your investment in time for an informed review.
Greetings Raffaello
@rgiulietti Please issue the `/csr/` command here [1]. Speaking of which, does the CSR [2] need to be updated? [1] https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullReques... [2] https://bugs.openjdk.java.net/browse/JDK-8202555 ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 9 Apr 2021 16:39:45 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Hello,
here's some background information for those that didn't follow the mailing list for the last couple of years.
Some enjoyable properties of the novel algorithm: * No intermediate objects are instantiated. * Loop-free core algorithm. * Only int and long arithmetic. * No divisions at all. * 17.7x speedup (jmh) [1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-April/065921.html). * Randomized, yet reproducible deep diving tests (jtreg). * Clear, unambiguous spec. * All floats have been tested to fully meet the spec. * Fully documented in [2](https://drive.google.com/file/d/1luHhyQF9zKlM8yJ1nebU0OgVYhfC6CBN/view) and/or in comments.
See [3](https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-September/062580....) for some (invented) Q&A. The last Q&A deals with your investment in time for an informed review.
Greetings Raffaello
@rgiulietti Please issue the `/csr/` command here [1]. Speaking of which, does the CSR [2] need to be updated?
[1] https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullReques... [2] https://bugs.openjdk.java.net/browse/JDK-8202555
@bplb No, the CSR [1] (https://bugs.openjdk.java.net/browse/JDK-8202555) needs no updates. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 9 Apr 2021 16:44:18 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
@rgiulietti Please issue the `/csr/` command here [1]. Speaking of which, does the CSR [2] need to be updated?
[1] https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullReques... [2] https://bugs.openjdk.java.net/browse/JDK-8202555
@bplb No, the CSR [1] (https://bugs.openjdk.java.net/browse/JDK-8202555) needs no updates.
The langtools/tier1 test tools/javac/sym/ElementStructureTest.java fails on all 4 platforms supported by the automatic GH actions. Does anybody have a clue? Is this something I should be worried about? Thanks ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 9 Apr 2021 16:39:45 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Hello,
here's some background information for those that didn't follow the mailing list for the last couple of years.
Some enjoyable properties of the novel algorithm: * No intermediate objects are instantiated. * Loop-free core algorithm. * Only int and long arithmetic. * No divisions at all. * 17.7x speedup (jmh) [1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-April/065921.html). * Randomized, yet reproducible deep diving tests (jtreg). * Clear, unambiguous spec. * All floats have been tested to fully meet the spec. * Fully documented in [2](https://drive.google.com/file/d/1luHhyQF9zKlM8yJ1nebU0OgVYhfC6CBN/view) and/or in comments.
See [3](https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-September/062580....) for some (invented) Q&A. The last Q&A deals with your investment in time for an informed review.
Greetings Raffaello
@rgiulietti Please issue the `/csr/` command here [1]. Speaking of which, does the CSR [2] need to be updated?
[1] https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullReques... [2] https://bugs.openjdk.java.net/browse/JDK-8202555
@bplb No, the CSR [1] (https://bugs.openjdk.java.net/browse/JDK-8202555) needs no updates.
OK, good. I wonder whether it should be moved back to `Draft` until someone else other than me has reviewed it? ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Regarding the `ElementStructureTest`, it prints the API elements (including compile-time constants) computes hash for the printed API and compares it with an expected hash. doubles and floats are printed using String.valueOf, and it apparently changed for `Float.MIN_NORMAL` from `1.17549435E-38` to `1.1754944E-38` (I assume that is intentional). So regarding `ElementStructureTest.java` we can just update it. How about this? diff --git a/test/langtools/tools/javac/sym/ElementStructureTest.java b/test/langtools/tools/javac/sym/ElementStructureTest.java index 29776ce28c2..d15f2447749 100644 --- a/test/langtools/tools/javac/sym/ElementStructureTest.java +++ b/test/langtools/tools/javac/sym/ElementStructureTest.java @@ -121,29 +121,22 @@ import toolbox.ToolBox; */ public class ElementStructureTest { - static final byte[] hash6 = new byte[] { - (byte) 0x99, (byte) 0x34, (byte) 0x82, (byte) 0xCF, - (byte) 0xE0, (byte) 0x53, (byte) 0xF3, (byte) 0x13, - (byte) 0x4E, (byte) 0xCF, (byte) 0x49, (byte) 0x32, - (byte) 0xB7, (byte) 0x52, (byte) 0x0F, (byte) 0x68 - }; static final byte[] hash7 = new byte[] { - (byte) 0x3C, (byte) 0x03, (byte) 0xEA, (byte) 0x4A, - (byte) 0x62, (byte) 0xD2, (byte) 0x18, (byte) 0xE5, - (byte) 0xA5, (byte) 0xC2, (byte) 0xB7, (byte) 0x85, - (byte) 0x90, (byte) 0xFA, (byte) 0x98, (byte) 0xCD + (byte) 0xA7, (byte) 0x3B, (byte) 0x91, (byte) 0xF6, + (byte) 0xEF, (byte) 0x99, (byte) 0x07, (byte) 0xF2, + (byte) 0x79, (byte) 0xAB, (byte) 0x19, (byte) 0xF4, + (byte) 0x59, (byte) 0x44, (byte) 0xF7, (byte) 0x65 }; static final byte[] hash8 = new byte[] { - (byte) 0x24, (byte) 0x38, (byte) 0x52, (byte) 0x1C, - (byte) 0x5E, (byte) 0x83, (byte) 0x82, (byte) 0xE6, - (byte) 0x41, (byte) 0xC2, (byte) 0xDD, (byte) 0x2A, - (byte) 0xFD, (byte) 0xFF, (byte) 0x5E, (byte) 0x2F + (byte) 0xF3, (byte) 0x93, (byte) 0xCA, (byte) 0x53, + (byte) 0xFD, (byte) 0xA3, (byte) 0x5D, (byte) 0x57, + (byte) 0xD2, (byte) 0xED, (byte) 0x39, (byte) 0xC5, + (byte) 0x56, (byte) 0x62, (byte) 0xE0, (byte) 0x1F }; final static Map<String, byte[]> version2Hash = new HashMap<>(); static { - version2Hash.put("6", hash6); version2Hash.put("7", hash7); version2Hash.put("8", hash8); } @@ -484,7 +477,7 @@ public class ElementStructureTest { return null; try { analyzeElement(e); - out.write(String.valueOf(e.getConstantValue())); + writeConstant(e.getConstantValue()); out.write("\n"); } catch (IOException ex) { ex.printStackTrace(); @@ -514,6 +507,16 @@ public class ElementStructureTest { throw new IllegalStateException("Should not get here."); } + private void writeConstant(Object value) throws IOException { + if (value instanceof Double) { + out.write(Long.toString(Double.doubleToRawLongBits((Double) value))); + } else if (value instanceof Float) { + out.write(Integer.toString(Float.floatToRawIntBits((Float) value))); + } else { + out.write(String.valueOf(value)); + } + } + } final class TestFileManager implements JavaFileManager { ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
Hi Jan, I recommend using {Double, Float}.toHexString to get a straightforward textual form of the floating-point values. The hex string is isomorphic to the big-level value, but is (more) human readable as a numerical quantity. -Joe On 4/15/2021 10:26 AM, Jan Lahoda wrote:
On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello Regarding the `ElementStructureTest`, it prints the API elements (including compile-time constants) computes hash for the printed API and compares it with an expected hash. doubles and floats are printed using String.valueOf, and it apparently changed for `Float.MIN_NORMAL` from `1.17549435E-38` to `1.1754944E-38` (I assume that is intentional). So regarding `ElementStructureTest.java` we can just update it. How about this?
diff --git a/test/langtools/tools/javac/sym/ElementStructureTest.java b/test/langtools/tools/javac/sym/ElementStructureTest.java index 29776ce28c2..d15f2447749 100644 --- a/test/langtools/tools/javac/sym/ElementStructureTest.java +++ b/test/langtools/tools/javac/sym/ElementStructureTest.java @@ -121,29 +121,22 @@ import toolbox.ToolBox; */ public class ElementStructureTest {
- static final byte[] hash6 = new byte[] { - (byte) 0x99, (byte) 0x34, (byte) 0x82, (byte) 0xCF, - (byte) 0xE0, (byte) 0x53, (byte) 0xF3, (byte) 0x13, - (byte) 0x4E, (byte) 0xCF, (byte) 0x49, (byte) 0x32, - (byte) 0xB7, (byte) 0x52, (byte) 0x0F, (byte) 0x68 - }; static final byte[] hash7 = new byte[] { - (byte) 0x3C, (byte) 0x03, (byte) 0xEA, (byte) 0x4A, - (byte) 0x62, (byte) 0xD2, (byte) 0x18, (byte) 0xE5, - (byte) 0xA5, (byte) 0xC2, (byte) 0xB7, (byte) 0x85, - (byte) 0x90, (byte) 0xFA, (byte) 0x98, (byte) 0xCD + (byte) 0xA7, (byte) 0x3B, (byte) 0x91, (byte) 0xF6, + (byte) 0xEF, (byte) 0x99, (byte) 0x07, (byte) 0xF2, + (byte) 0x79, (byte) 0xAB, (byte) 0x19, (byte) 0xF4, + (byte) 0x59, (byte) 0x44, (byte) 0xF7, (byte) 0x65 }; static final byte[] hash8 = new byte[] { - (byte) 0x24, (byte) 0x38, (byte) 0x52, (byte) 0x1C, - (byte) 0x5E, (byte) 0x83, (byte) 0x82, (byte) 0xE6, - (byte) 0x41, (byte) 0xC2, (byte) 0xDD, (byte) 0x2A, - (byte) 0xFD, (byte) 0xFF, (byte) 0x5E, (byte) 0x2F + (byte) 0xF3, (byte) 0x93, (byte) 0xCA, (byte) 0x53, + (byte) 0xFD, (byte) 0xA3, (byte) 0x5D, (byte) 0x57, + (byte) 0xD2, (byte) 0xED, (byte) 0x39, (byte) 0xC5, + (byte) 0x56, (byte) 0x62, (byte) 0xE0, (byte) 0x1F };
final static Map<String, byte[]> version2Hash = new HashMap<>();
static { - version2Hash.put("6", hash6); version2Hash.put("7", hash7); version2Hash.put("8", hash8); } @@ -484,7 +477,7 @@ public class ElementStructureTest { return null; try { analyzeElement(e); - out.write(String.valueOf(e.getConstantValue())); + writeConstant(e.getConstantValue()); out.write("\n"); } catch (IOException ex) { ex.printStackTrace(); @@ -514,6 +507,16 @@ public class ElementStructureTest { throw new IllegalStateException("Should not get here."); }
+ private void writeConstant(Object value) throws IOException { + if (value instanceof Double) { + out.write(Long.toString(Double.doubleToRawLongBits((Double) value))); + } else if (value instanceof Float) { + out.write(Integer.toString(Float.floatToRawIntBits((Float) value))); + } else { + out.write(String.valueOf(value)); + } + } + }
final class TestFileManager implements JavaFileManager {
-------------
On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Hi Jan, I had to change a string in test test/jdk/java/lang/String/concat/ImplicitStringConcatBoundaries.java because it failed with the current string but passes with the new one. Indeed, the new implementation of Float.toString(float) produces the new string, which, like the current one, is correct in the sense that, upon reading, it recovers Float.MIN_NORMAL. However, I didn't change the definition of MIN_NORMAL in java.lang.Float because there it is already expressed in hex notation. As suggested before and by Joe, using the hex representation instead of the decimal would be more robust because the conversions from/to hex are almost trivial, hence much less subject to slight errors. So, rather than printing the raw bits as you suggest, you could use the hex string rendering instead. Thanks Raffaello ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Sure, here you are: diff --git a/test/langtools/tools/javac/sym/ElementStructureTest.java b/test/langtools/tools/javac/sym/ElementStructureTest.java index 29776ce28c2..428ba03361f 100644 --- a/test/langtools/tools/javac/sym/ElementStructureTest.java +++ b/test/langtools/tools/javac/sym/ElementStructureTest.java @@ -121,29 +121,22 @@ import toolbox.ToolBox; */ public class ElementStructureTest { - static final byte[] hash6 = new byte[] { - (byte) 0x99, (byte) 0x34, (byte) 0x82, (byte) 0xCF, - (byte) 0xE0, (byte) 0x53, (byte) 0xF3, (byte) 0x13, - (byte) 0x4E, (byte) 0xCF, (byte) 0x49, (byte) 0x32, - (byte) 0xB7, (byte) 0x52, (byte) 0x0F, (byte) 0x68 - }; static final byte[] hash7 = new byte[] { - (byte) 0x3C, (byte) 0x03, (byte) 0xEA, (byte) 0x4A, - (byte) 0x62, (byte) 0xD2, (byte) 0x18, (byte) 0xE5, - (byte) 0xA5, (byte) 0xC2, (byte) 0xB7, (byte) 0x85, - (byte) 0x90, (byte) 0xFA, (byte) 0x98, (byte) 0xCD + (byte) 0x65, (byte) 0x41, (byte) 0xC8, (byte) 0x17, + (byte) 0xF0, (byte) 0xB1, (byte) 0x62, (byte) 0x9A, + (byte) 0xD8, (byte) 0x19, (byte) 0xBA, (byte) 0xB0, + (byte) 0xC1, (byte) 0x70, (byte) 0x5E, (byte) 0x3E }; static final byte[] hash8 = new byte[] { - (byte) 0x24, (byte) 0x38, (byte) 0x52, (byte) 0x1C, - (byte) 0x5E, (byte) 0x83, (byte) 0x82, (byte) 0xE6, - (byte) 0x41, (byte) 0xC2, (byte) 0xDD, (byte) 0x2A, - (byte) 0xFD, (byte) 0xFF, (byte) 0x5E, (byte) 0x2F + (byte) 0x83, (byte) 0x62, (byte) 0x2F, (byte) 0x1C, + (byte) 0x95, (byte) 0x6D, (byte) 0x31, (byte) 0x18, + (byte) 0xF5, (byte) 0xB2, (byte) 0x8C, (byte) 0x39, + (byte) 0x81, (byte) 0x2E, (byte) 0x2C, (byte) 0x34 }; final static Map<String, byte[]> version2Hash = new HashMap<>(); static { - version2Hash.put("6", hash6); version2Hash.put("7", hash7); version2Hash.put("8", hash8); } @@ -484,7 +477,7 @@ public class ElementStructureTest { return null; try { analyzeElement(e); - out.write(String.valueOf(e.getConstantValue())); + writeConstant(e.getConstantValue()); out.write("\n"); } catch (IOException ex) { ex.printStackTrace(); @@ -514,6 +507,16 @@ public class ElementStructureTest { throw new IllegalStateException("Should not get here."); } + private void writeConstant(Object value) throws IOException { + if (value instanceof Double) { + out.write(Double.toHexString((Double) value)); + } else if (value instanceof Float) { + out.write(Float.toHexString((Float) value)); + } else { + out.write(String.valueOf(value)); + } + } + } final class TestFileManager implements JavaFileManager { ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Fine, thanks! Will your change be integrated soon on master? What am I supposed to do then, rebasing my branch with the updated master? (BTW, you could make use of instanceof pattern matching if you don't need backward compat ;-) ) ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
@ rgiulietti, I thought you'd simply add the test change to your patch. But I can start a PR for it, if you prefer. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 08:22:41 GMT, Jan Lahoda <jlahoda@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
@ rgiulietti, I thought you'd simply add the test change to your patch. But I can start a PR for it, if you prefer.
@lahodaj Oh, my understanding was that yours is a permanent change worth of integrating anyway. But yes, I can add your change to my changeset. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 4511638: Double.toString(double) sometimes produces incorrect results ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3402/files - new: https://git.openjdk.java.net/jdk/pull/3402/files/cc10a92d..22092f0c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=00-01 Stats: 26 lines in 1 file changed: 10 ins; 7 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 69:
67: 68: /* 10^(E_MIN - 1) <= MIN_VALUE < 10^E_MIN */ 69: static final int E_MIN = -323;
It seems that `E_MIN`/`E_MAX`/`K_MIN`/`K_MAX` are not used in production code. I think it worth to move them to tests. src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 117:
115: * where there are H digits d 116: */ 117: public final int MAX_CHARS = H + 7;
Can it be made `static` ? ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
Hi Andrey, thanks for your careful reading. I'll keep a note and collect yours with changes coming from other reviewers before committing a larger batch of small changes. I would like to avoid wasting a lot of energy right now just to rebuild everything and to run the automatic tests on GitHub :-) Greetings Raffaello On 2021-04-18 23:19, Andrey Turbanov wrote:
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 69:
67: 68: /* 10^(E_MIN - 1) <= MIN_VALUE < 10^E_MIN */ 69: static final int E_MIN = -323;
It seems that `E_MIN`/`E_MAX`/`K_MIN`/`K_MAX` are not used in production code. I think it worth to move them to tests.
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 117:
115: * where there are H digits d 116: */ 117: public final int MAX_CHARS = H + 7;
Can it be made `static` ?
-------------
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Hi, a reminder to keep PR [1] alive and to invite interested reviewers to comment it. The corresponding CSR is in [2] Greetings Raffaello --- [1] https://github.com/openjdk/jdk/pull/3402 [2] https://bugs.openjdk.java.net/browse/JDK-8202555 ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
This is just to keep [1] alive. No news. [1] https://github.com/openjdk/jdk/pull/3402 ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
The usual refresh comment to keep this PR to remain active. Nothing new. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
My turn to keep this PR alive. If we'd had any actual face-to-face contributors' meetings this year I would have raised this stuck PR as an issue with our processes. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 28 Sep 2021 08:27:22 GMT, Andrew Haley <aph@openjdk.org> wrote:
My turn to keep this PR alive. If we'd had any actual face-to-face contributors' meetings this year I would have raised this stuck PR as an issue with our processes.
I believe that the previous discussion on this topic had an AI to reach out to @GuySteele to ask for comment on the "The Schubfach way to render doubles” paper. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Thanks for helping me keeping this PR alive. I was pondering whether to propose adding a launch-time option on the command line to choose between the current and the proposed implementation for some time, so to help the community gaining confidence in the new algorithm and still have the current one as a fallback, just in case. (AFAIU, a launch-time option can be constant folded by the JIT compiler to help it eliminate dead code, right?) On the one hand, this adds complexity. On the other hand, it could help revive this dormant PR. What do you think? Would this be a viable option? Greetings Raffaello ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On 9/28/21 10:30, Raffaello Giulietti wrote:
I was pondering whether to propose adding a launch-time option on the command line to choose between the current and the proposed implementation for some time, so to help the community gaining confidence in the new algorithm and still have the current one as a fallback, just in case. (AFAIU, a launch-time option can be constant folded by the JIT compiler to help it eliminate dead code, right?)
On the one hand, this adds complexity. On the other hand, it could help revive this dormant PR.
What do you think? Would this be a viable option?
It's a good-enough idea in theory, but I don't think it'd be accepted. Pinging Joe Darcy: do you (or any of your friends) have a good way to contact Guy Steele? -- Andrew Haley (he/him) Java Platform Lead Engineer Red Hat UK Ltd. <https://www.redhat.com> https://keybase.io/andrewhaley EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
I forwarded the issue to Guy again recently. No reply as yet. He’s a busy dude. Do we have any alternates? 📱
On Oct 5, 2021, at 10:58 AM, Andrew Haley <aph-open@littlepinkcloud.com> wrote:
On 9/28/21 10:30, Raffaello Giulietti wrote:
I was pondering whether to propose adding a launch-time option on the command line to choose between the current and the proposed implementation for some time, so to help the community gaining confidence in the new algorithm and still have the current one as a fallback, just in case. (AFAIU, a launch-time option can be constant folded by the JIT compiler to help it eliminate dead code, right?)
On the one hand, this adds complexity. On the other hand, it could help revive this dormant PR.
What do you think? Would this be a viable option?
It's a good-enough idea in theory, but I don't think it'd be accepted.
Pinging Joe Darcy: do you (or any of your friends) have a good way to contact Guy Steele?
-- Andrew Haley (he/him) Java Platform Lead Engineer Red Hat UK Ltd. <https://www.redhat.com> https://keybase.io/andrewhaley EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Hello, thanks for reading my writing. In some way, Ryu and Schubfach are similar in the sense that both are based on the observation that good, fixed size approximations of powers of 10 lead to extremely efficient algorithms. Both algorithms end up with a lookup table of 617 approximations. The proof of the central theorem of Schubfach is based on continued fractions. It was prepared on ACL2 by the late Dmitry Nadezhin, who was a member of Oracle's formal verification group. Dmitry also knew my writing inside-out: not a formal peer review but perhaps even more insightful. Schubfach has been exhaustively tested on all 2^32 floats, with approximations of powers of 10 of 63 bits each, rather than the full 126 bits used for doubles (the minimum size for doubles is 123 bits). It has also been tested for months on doubles, accumulating several trillions of witnesses and no failure. The exhaustive test on floats is an optionally executed part of the contributed code. After more than 25 years the current JDK implementation still contains anomalies, so I guess it isn't tested as well as Schubfach. A complete Schubfach conversion only depends on some dozens of primitive operations (it doesn't even need any sort of hardware division at all) and on a lookup table (fully tested for correctness in the contributed code). The current implementation in the JDK relies on the correctness of BigInteger and related classes, which are way more complex to thoroughly test and to be confident upon. Checking whether the 617 tabulated powers of 10 are very close to powers of 2 would be very easy, so I'm not sure I understand your point. Identifying the hard cases would require even more theory and would be an endeavor in itself. This theory could, in turn, contain small errors and would probably require identifying its hard cases... According to its author, DragonBox combines Schubfach and Grisu. AFAIK, there's no new fundamental underlying theory. During development of Schubfach, I experimented with a similar mix: on the JVM of the time (2018) performance was worse than pure Schubfach and the code more complex, so I gave up and switched back to the simpler design, which C2 likes better. Greetings Raffaello ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Hi, here's Guy's original email which I got intact in my mailbox but which appears completely garbled on this PR page (if you are reading it) or does not appear at all in this mailing list (if you are reading that). I repeat my comment as well HTH ---- Hi, sorry for the silence. Back in April 2021 I did read the entire Schubfach paper as well as two papers about Ryu by Adams. Schubfach looks intriguing, but it depends (as so many of these algorithms do) on a subtle proof. I am glad to see that the proof, or parts of it, are machine-checked in some way. I would be a lot more confident if this proof were also subjected to peer review. These algorithms are very much like the buggy Pentium FDIV: nearly all the cases are easy, but the cases that are hard (and therefore may potentially fail if you get that part of the algorithm wrong) are very rare, so doing good testing is tricky and deserves attention. If we can identify those hard cases (typically related to edge cases in the table entries) perhaps we can develop a good testing methodology. I would suspect that these hard cases relate to situations where a power of (1/5) is very close to a power of (1/2) (and therefore the corresponding power of 5 is very close to a power of 2), but t he details matter. Now that I have finished addressing bug https://bugs.openjdk.java.net/browse/JDK-8273792 (JumpableGenerator.rngs() documentation refers to wrong method) and bug https://bugs.openjdk.java.net/browse/JDK-8273056 (java.util.random does not correctly sample exponential or Gaussian distributions), I am now re-reading the Schubfach paper and am also investigating mentions of an implementation of that algorithm called DragonBox. I will have more to say by Friday (I have a Thursday deadline for something else). —Guy ---- Hello, thanks for reading my writing. In some way, Ryu and Schubfach are similar in the sense that both are based on the observation that good, fixed size approximations of powers of 10 lead to extremely efficient algorithms. Both algorithms end up with a lookup table of 617 approximations. The proof of the central theorem of Schubfach is based on continued fractions. It was prepared on ACL2 by the late Dmitry Nadezhin, who was a member of Oracle's formal verification group. Dmitry also knew my writing inside-out: not a formal peer review but perhaps even more insightful. Schubfach has been exhaustively tested on all 2^32 floats, with approximations of powers of 10 of 63 bits each, rather than the full 126 bits used for doubles (the minimum size for doubles is 123 bits). It has also been tested for months on doubles, accumulating several trillions of witnesses and no failure. The exhaustive test on floats is an optionally executed part of the contributed code. After more than 25 years the current JDK implementation still contains anomalies, so I guess it isn't tested as well as Schubfach. A complete Schubfach conversion only depends on some dozens of primitive operations (it doesn't even need any sort of hardware division at all) and on a lookup table (fully tested for correctness in the contributed code). The current implementation in the JDK relies on the correctness of BigInteger and related classes, which are way more complex to thoroughly test and to be confident upon. Checking whether the 617 tabulated powers of 10 are very close to powers of 2 would be very easy, so I'm not sure I understand your point. Identifying the hard cases would require even more theory and would be an endeavor in itself. This theory could, in turn, contain small errors and would probably require identifying its hard cases... According to its author, DragonBox combines Schubfach and Grisu. AFAIK, there's no new fundamental underlying theory. During development of Schubfach, I experimented with a similar mix: on the JVM of the time (2018) performance was worse than pure Schubfach and the code more complex, so I gave up and switched back to the simpler design, which C2 likes better. Greetings Raffaello ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Oct 5, 2021, at 5:05 PM, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.java.net<mailto:github.com+70726043+rgiulietti@openjdk.java.net>> wrote: The proof of the central theorem of Schubfach is based on continued fractions. It was prepared on ACL2 by the late Dmitry Nadezhin, who was a member of Oracle's formal verification group. Dmitry also knew my writing inside-out: not a formal peer review but perhaps even more insightful. Dmitry (Dima) was excellent and made a number of fine contributions to Java core-libs math. Here is something he posted a few years back about formal checking of the contribution here under discussion: https://mail.openjdk.java.net/pipermail/core-libs-dev/2018-September/055768.... Schubfach has been exhaustively tested on all 2^32 floats, with approximations of powers of 10 of 63 bits each, rather than the full 126 bits used for doubles (the minimum size for doubles is 123 bits). It has also been tested for months on doubles, accumulating several trillions of witnesses and no failure. The exhaustive test on floats is an optionally executed part of the contributed code. I have run the exhaustive 32-bit test myself without seeing any errors. I also ran a 64-bit round trip test for probably at least a week without seeing any failures. I don’t know whether it has been mentioned in this recent thread as yet, but the performance of Schubfach is also much better then the JDK implementation, although I can’t recall with certainty the improvement. I think however that it was something like 14X? Brian
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
(ungarbled comment from Guy Steele) Hi, thanks for writing to me! I am glad to “meet” you. Schubfach looks like an excellent idea; I have simply been trying to assess just how confident we can be about the correctness of the final code. The explanations you have just provided to me about the proof process and testing are very reassuring. I will keep those in mind as I finish re-reading your paper, and will have more to say on Friday. And if you happen to have a list of a few examples where the current Java implementation of FP print fails but Schubfach succeeds, I would be very interested to study them. Thanks! —Guy ---- (my answer) Hi, it's my pleasure to virtually meet the mythical Guy Steele! For some reason, your comments end up garbled on this PR GitHub page and don't appear at all on the core-libs-dev mailing list. You may perhaps try to post them in plain ASCII? You can find some of the anomalies in the bug report from exactly 20 years ago! [1] A tiny collection of the anomalies are coded in the tests [2], [3]. Here's the relevant excerpt for doubles: /* * There are tons of doubles that are rendered incorrectly by older JDK. * While the renderings correctly round back to the original value, * they are longer than needed or are not the closest decimal to the double. * Here are just a very few examples. */ private static final String[] Anomalies = { /* JDK renders these, and others, with 18 digits! */ "2.82879384806159E17", "1.387364135037754E18", "1.45800632428665E17", /* JDK renders these longer than needed */ "1.6E-322", "6.3E-322", "7.3879E20", "2.0E23", "7.0E22", "9.2E22", "9.5E21", "3.1E22", "5.63E21", "8.41E21", /* JDK does not render these, and many others, as the closest */ "9.9E-324", "9.9E-323", "1.9400994884341945E25", "3.6131332396758635E25", "2.5138990223946153E25", }; To be clear and as emphasized in the comment, the outputs of the current JDK implementation are all information preserving. I could never find a real error in this regard. However, typing 2e23 and getting back 1.9999999999999998E23 as a response is surprising and unnecessary if one knows David Matula's results from the late '60-ies. Also, many powers of 2 are output with way too many digits on current JDK. Schubfach's outputs pass the stringent tests. Greetings Raffaello ---- [1] https://bugs.openjdk.java.net/browse/JDK-4511638 [2] https://github.com/openjdk/jdk/pull/3402/files#diff-ee9b0bcfb171cb200f6aa1e3... [3] https://github.com/openjdk/jdk/pull/3402/files#diff-c4a866bc9e7d1beb5342823f... ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
The last time I ran JMH over the whole range of bitwise uniformly distributed random doubles, I measured a speedup of about 17.7x https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-April/065921.html This is about 70 ns/conversion on a 2013 consumer-grade laptop. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
(comment by Guy Steele) Hi, Raffaello, I will try to compose this message in plain ASCII and hope it does not get garbled. I have now re-read the Schubfach paper, and with the extra information in your previous email, I think I understood it a lot better this time. I am very enthusiastic about the approach. This is exactly what JonL White was trying to puzzle out some five decades ago: just how many digits do you need in your powers of ten to get accurate print-read round-trips? I am especially appreciative of the attention paid in the Schubfach paper to parameter M, which ensures that two-digit possibilities are considered in cases where it is required to print two significant digits anyway (scientific notation). I have also read the two papers I could find about Ryu. Schubfach is clearly an improvement over Ryu because it avoids the iteration, among other reasons. But the second paper, about Ryu for printf, addresses the specific difficulties of generating digits for printf format specifiers, and raises the interesting question of whether it would be worthwhile also to design a version of Schubfach that handles printf requirements. So I think it would be well worth incorporating the Schubfach algorithm into the JDK codebase. I am reassured that Schubfach has undergone extensive testing on trillions of randomly chosen values. But I would be further reassured by a statement that two important classes of values have been _exhaustively_ tested: (1) All positive powers of 2 representable in the double format, plus the maximum representable value. (These are the values that are surrounded by irregular spacing.) Furthermore, out of caution one should also test every fp value within Z ulps of such a value; perhaps Z should be 64 or even 1024. (2) All representable fp values that are the result of rounding-to-nearest some decimal value of the form y•10^n, where y is a member of, say, either D_1, D_2, D_3, or D_4. (These are values that may be especially susceptible to generation of too many digits by an insufficiently careful algorithm, and one reason might be insufficient precision or other algorithmic error in connection with a table of powers of ten.) Furthermore, out of caution one should also test every fp value within Y ulps of such a value; perhaps Y should be 64 or even 1024. In every case, the testing should include (a) ensuring round-trip print-read behavior; (b) comparing to what is produced by the current JDK algorithm, and investigating any cases that differ. And if other similar “edge cases” can be identified from the structure of the code, those would be worth focusing on as well. If this testing has been or could be done, I would say, yes, certainly adopt Schubfach for Java. I would also recommend submitting the Schubfach paper to an appropriate conference or journal to get the benefit of further peer review of the algorithm and its write-up. —Guy ---- (my reply) Hi Guy, for some reason your comments still appear garbled on the GitHub PR page and don't make it to the core-libs-dev mailing list at all. Luckily, they appear intelligible in my mailbox, so I'll keep going with prepending your comments in my replies: not ideal but good enough. Thanks so much for re-reading my "paper". printf() There are some issues to consider when trying to apply Schubfach to printf(), the main one being that printf() allows to specify an arbitrary length for the resulting decimal. This means, for example, that unlimited precision arithmetic is unavoidable. But it might be worthwhile to investigate using Schubfach for lengths up to H (9 and 17 for float and double, resp.) and fall back to unlimited precision beyond that. Before that, however, I would prefer to finally push Schubfach in the OpenJDK codebase for the toString() cases and close this PR. Tests Below, by "extensive test" I mean not only that the outcomes convert back without loss of information, but that they fully meet the spec about minimal number of digits, closeness, correct formatting (normal viz scientific), character set, etc. All currently available tests are in the contributed code of this PR and will be part of the OpenJDK once integrated. - All powers of 2 and the extreme values are already extensively tested. - All values closest to powers of 10 are extensively tested. - All values proposed by Paxson [1] are extensively tested. - A configurable number of random values are tested at each round (currently 4 * 1'000'000 random values). Should a value fail, there's enough diagnostic information for further investigation. I'll add extensive tests for the values you propose in point (1) and (2), setting Z = Y = 1024. As for comparison with the current JDK behavior, there are already a bunch of values for which extensive tests fail on the current JDK but pass with Schubfach. It would be cumbersome, if possible at all, to have both the current JDK and Schubfach implementations in the same OpenJDK codebase to be able to compare the outcomes. I performed comparisons in a different constellation, with Schubfach as an external library, but this is hardly a possibility in the core-libs. Needless to say, Schubfach outcomes are either the same as in JDK or better (shorter or closest to the fp value). Peer reviewed publication Shortening my 27 pages writing and re-formating it to meet a journal standards for publication would require investing yet another substantial amount of time. I'm not sure I'm prepared for that, given that I've no personal interest in a journal publication: I'm not an academic, I'm not pursuing a career... But I promise I'll think about reconsidering my position on this ;-) Greetings Raffaello ---- [1] Paxson V, "A Program for Testing IEEE Decimal-Binary Conversion" ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Hi Guy, while implementing the additional test recommended in your point (2), it occurred to me that the numbers of the form y 10^n, y in D_k (k = 1, 2, 3, 4) end up being of the form y' 10^n', where y' = y / 10^k, n' = n + k, plus the 2 * Y values around these. Such numbers do not seem to show any special structure worth a dedicated test, so I'm wondering if you mean something else instead. Perhaps you mean y to have at most 4 digits, i.e., 0 <= y < 10^4? Greetings Raffaello P.S. The test recommended in point (1) pass successfully. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
(Guy Steele reply to a previous comment of mine) Yes, thank you, I stated my suggested criterion incorrectly. —Guy On Oct 11, 2021, at 4:16 AM, Raffaello Giulietti ***@***.******@***.***>> wrote:> Hi Guy,
while implementing the additional test recommended in your point (2), it occurred to me that the numbers of the form y 10^n, y in D_k (k = 1, 2, 3, 4) end up being of the form y' 10^n', where y' = y / 10^k, n' = n + k, plus the 2 * Y values around these. Such numbers do not seem to show any special structure worth a dedicated test, so I'm wondering if you mean something else instead.
Perhaps you mean y to have at most 4 digits, i.e., 0 <= y < 10^4?
Greetings Raffaello
P.S. The test recommended in point (1) pass successfully.
------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
(Replies from Guy Steele)
Hi Guy,
for some reason your comments still appear garbled on the GitHub PR page and don't make it to the core-libs-dev mailing list at all. Luckily, they appear intelligible in my mailbox, so I'll keep going with prepending your comments in my replies: not ideal but good enough.
Thanks so much for re-reading my "paper".
printf()
There are some issues to consider when trying to apply Schubfach to printf(), the main one being that printf() allows to specify an arbitrary length for the resulting decimal. This means, for example, that unlimited precision arithmetic is unavoidable. But it might be worthwhile to investigate using Schubfach for lengths up to H (9 and 17 for float and double, resp.) and fall back to unlimited precision beyond that. Before that, however, I would prefer to finally push Schubfach in the OpenJDK codebase for the toString() cases and close this PR.
I completely agree that using Schubfach to solve only the toString() problems would be a _major_ improvement in the situation, and this should not wait for exploration of the printf problem. But I suspect that using Schubfach for lengths up to H would cover a very large fraction of actual usage, and would improve both quality and speed, and therefore would be worth exploring later.
Tests
Below, by "extensive test" I mean not only that the outcomes convert back without loss of information, but that they fully meet the spec about minimal number of digits, closeness, correct formatting (normal viz scientific), character set, etc.
All currently available tests are in the contributed code of this PR and will be part of the OpenJDK once integrated.
* All powers of 2 and the extreme values are already extensively tested. * All values closest to powers of 10 are extensively tested. * All values proposed by Paxson [1] are extensively tested.
I have now read through the Paxson paper. Does this refer to the values listed in his Tables 3 and 4, or to other values instead or in addition?
* A configurable number of random values are tested at each round (currently 4 * 1'000'000 random values). Should a value fail, there's enough diagnostic information for further investigation.
I'll add extensive tests for the values you propose in point (1) and (2), setting Z = Y = 1024.
I do think that would lend further confidence.
As for comparison with the current JDK behavior, there are already a bunch of values for which extensive tests fail on the current JDK but pass with Schubfach.
Yes, thanks for supplying some of those.
It would be cumbersome, if possible at all, to have both the current JDK and Schubfach implementations in the same OpenJDK codebase to be able to compare the outcomes. I performed comparisons in a different constellation, with Schubfach as an external library, but this is hardly a possibility in the core-libs. Needless to say, Schubfach outcomes are either the same as in JDK or better (shorter or closest to the fp value).
Okay. I will mention here, for the record, that there is one other sort of test that could be performed that I think I have not yet seen you mention: a monotonicity test of the kind used by David Hough’s Testbase (mentioned by Paxson). However, a little thought reveals that such a test made unnecessary by the round-trip test. So a monotonicity test would be a good idea when testing printf case, but is not needed for the toString case. Therefore, if you add the few tests I have suggested, I think that we can say with the best certainty we can have, short of separately testing every possible double value, that Schubfach is extremely well tested and ready for adoption into Java.
Peer reviewed publication
Shortening my 27 pages writing and re-formating it to meet a journal standards for publication would require investing yet another substantial amount of time. I'm not sure I'm prepared for that, given that I've no personal interest in a journal publication: I'm not an academic, I'm not pursuing a career... But I promise I'll think about reconsidering my position on this ;-)
Please do think about reconsidering. There are several reasons to publish an “academic” paper: - Earning “merit badges” that lead to academic tenure - Reputation more generally—which maybe you don’t care much about, but it’s one way to ensure that the contributions of Dmitry Nadezhin are not forgotten - Making sure that the technical ideas are not lost: an academic journal provides a “permanent home” for documentation and a search engine - Provides a place for others who build on the work to cite - The publication process engages other minds and eyeballs that may improve the writeup, sometimes in surprisingly good ways (in particular, I am sure that good referees would insist that you include all the details about testing that I had to drag out of you over the course of several email exchanges—if this information had beennin the original writeup, I should simply have said, “Great! All done! Go for it!”). There are many different archival venues with different tradeoffs. - Publishing at arxiv.org<http://arxiv.org> is free, takes very little time, imposes no special formatting restrictions, has no page limit (so you don’t have to shorten anything), and has no review process. I would recommend doing this much right away. - Some academic journals have no page limit, or have page limits up around 50 pages; this takes time but would give you rigorous peer review (multiple rounds if necessary). - Conferences do have page limits (anywhere from 5 to 30 pages, depending on the conference), but take less time and give you some peer review. If this code goes into the Java codebase, then first and foremost I want to make sure that some version of your complete writeup, expanded to describe the testing procedures, remains available to those who will have to maintain the code decades into the future. Second, I would like to make it easy for implementors of other programming languages to adopt this solution also. This is too important a problem and EVERY programming language that supports floating-point values must solve it. I want to help make sure that from now on it will be solved well, and right now Schubfach is the best solution I have seen. —Guy ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Hi Guy,
I have now read through the Paxson paper. Does this refer to the values listed in his Tables 3 and 4, or to other values instead or in addition?
Right.
Shortening my 27 pages writing and re-formating it to meet a journal standards for publication would require investing yet another substantial amount of time. I'm not sure I'm prepared for that, given that I've no personal interest in a journal publication: I'm not an academic, I'm not pursuing a career... But I promise I'll think about reconsidering my position on this ;-)
Please do think about reconsidering.
OK, thanks for the useful suggestions. Greetings Raffaello ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Email from GuySteele <notifications@github.com> follows: (my reply) Hi Guy, for some reason your comments still appear garbled on the GitHub PR page and don't make it to the core-libs-dev mailing list at all. Luckily, they appear intelligible in my mailbox, so I'll keep going with prepending your comments in my replies: not ideal but good enough. Thanks so much for re-reading my "paper". printf() There are some issues to consider when trying to apply Schubfach to printf(), the main one being that printf() allows to specify an arbitrary length for the resulting decimal. This means, for example, that unlimited precision arithmetic is unavoidable. But it might be worthwhile to investigate using Schubfach for lengths up to H (9 and 17 for float and double, resp.) and fall back to unlimited precision beyond that. Before that, however, I would prefer to finally push Schubfach in the OpenJDK codebase for the toString() cases and close this PR. I completely agree that using Schubfach to solve only the toString() problems would be a _major_ improvement in the situation, and this should not wait for exploration of the printf problem. But I suspect that using Schubfach for lengths up to H would cover a very large fraction of actual usage, and would improve both quality and speed, and therefore would be worth exploring later. Tests Below, by "extensive test" I mean not only that the outcomes convert back without loss of information, but that they fully meet the spec about minimal number of digits, closeness, correct formatting (normal viz scientific), character set, etc. All currently available tests are in the contributed code of this PR and will be part of the OpenJDK once integrated. * All powers of 2 and the extreme values are already extensively tested. * All values closest to powers of 10 are extensively tested. * All values proposed by Paxson [1] are extensively tested. I have now read through the Paxson paper. Does this refer to the values listed in his Tables 3 and 4, or to other values instead or in addition? * A configurable number of random values are tested at each round (currently 4 * 1'000'000 random values). Should a value fail, there's enough diagnostic information for further investigation. I'll add extensive tests for the values you propose in point (1) and (2), setting Z = Y = 1024. I do think that would lend further confidence. As for comparison with the current JDK behavior, there are already a bunch of values for which extensive tests fail on the current JDK but pass with Schubfach. Yes, thanks for supplying some of those. It would be cumbersome, if possible at all, to have both the current JDK and Schubfach implementations in the same OpenJDK codebase to be able to compare the outcomes. I performed comparisons in a different constellation, with Schubfach as an external library, but this is hardly a possibility in the core-libs. Needless to say, Schubfach outcomes are either the same as in JDK or better (shorter or closest to the fp value). Okay. I will mention here, for the record, that there is one other sort of test that could be performed that I think I have not yet seen you mention: a monotonicity test of the kind used by David Hough’s Testbase (mentioned by Paxson). However, a little thought reveals that such a test made unnecessary by the round-trip test. So a monotonicity test would be a good idea when testing printf case, but is not needed for the toString case. Therefore, if you add the few tests I have suggested, I think that we can say with the best certainty we can have, short of separately testing every possible double value, that Schubfach is extremely well tested and ready for adoption into Java. Peer reviewed publication Shortening my 27 pages writing and re-formating it to meet a journal standards for publication would require investing yet another substantial amount of time. I'm not sure I'm prepared for that, given that I've no personal interest in a journal publication: I'm not an academic, I'm not pursuing a career... But I promise I'll think about reconsidering my position on this ;-) Please do think about reconsidering. There are several reasons to publish an “academic” paper: - Earning “merit badges” that lead to academic tenure - Reputation more generally—which maybe you don’t care much about, but it’s one way to ensure that the contributions of Dmitry Nadezhin are not forgotten - Making sure that the technical ideas are not lost: an academic journal provides a “permanent home” for documentation and a search engine - Provides a place for others who build on the work to cite - The publication process engages other minds and eyeballs that may improve the writeup, sometimes in surprisingly good ways (in particular, I am sure that good referees would insist that you include all the details about testing that I had to drag out of you over the course of several email exchanges—if this information had beennin the original writeup, I should simply have said, “Great! All done! Go for it!”). There are many different archival venues with different tradeoffs. - Publishing at arxiv.org<http://arxiv.org> is free, takes very little time, imposes no special formatting restrictions, has no page limit (so you don’t have to shorten anything), and has no review process. I would recommend doing this much right away. - Some academic journals have no page limit, or have page limits up around 50 pages; this takes time but would give you rigorous peer review (multiple rounds if necessary). - Conferences do have page limits (anywhere from 5 to 30 pages, depending on the conference), but take less time and give you some peer review. If this code goes into the Java codebase, then first and foremost I want to make sure that some version of your complete writeup, expanded to describe the testing procedures, remains available to those who will have to maintain the code decades into the future. Second, I would like to make it easy for implementors of other programming languages to adopt this solution also. This is too important a problem and EVERY programming language that supports floating-point values must solve it. I want to help make sure that from now on it will be solved well, and right now Schubfach is the best solution I have seen. —Guy -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/openjdk/jdk/pull/3402#issuecomment-941807665 ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Hello, Paul Zimmermann of INRIA kindly provided me 19'545 doubles generated on purpose as hard test cases. All have the form 2^q*10^(-k), with k as in my writing (https://drive.google.com/file/d/1luHhyQF9zKlM8yJ1nebU0OgVYhfC6CBN/view) They all pass on my machine and will be added to the this PR asap. Greetings Raffaello ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Thu, 14 Oct 2021 16:51:42 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
All have the form 2^q*10^(-k) not quite, for example 0x1.00003eaba12cap-804 = 2251808225167717/2^855 is not of this form, but these cases come from the continued fraction expansion of 2^q*10^(-k).
------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
"All have the form 2^q10^(-k)" (Paul Zimmermann's reply) not quite, for example 0x1.00003eaba12cap-804 = 2251808225167717/2^855 is not of this form, but these cases come from the continued fraction expansion of 2^q10^(-k). (my reply) Right, it is correctly stated in the test class (to be pushed) but incorrectly in my previous post Raffaello ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti <github.com+70726043+rgiulietti@openjdk.org> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Modified test/langtools/tools/javac/sym/ElementStructureTest.java as suggested by @lahodaj ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits: - 4511638: Double.toString(double) sometimes produces incorrect results Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results Refactored test classes to better match OpenJDK conventions. Added tests recommended by Guy Steele and Paul Zimmermann. - Changed MAX_CHARS to static - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results ------------- Changes: https://git.openjdk.java.net/jdk/pull/3402/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=02 Stats: 23941 lines in 16 files changed: 23805 ins; 61 del; 75 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits: - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) comments. - 4511638: Double.toString(double) sometimes produces incorrect results Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results Refactored test classes to better match OpenJDK conventions. Added tests recommended by Guy Steele and Paul Zimmermann. - Changed MAX_CHARS to static - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results ------------- Changes: https://git.openjdk.java.net/jdk/pull/3402/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=03 Stats: 23945 lines in 16 files changed: 23809 ins; 61 del; 75 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 4511638: Double.toString(double) sometimes produces incorrect results Enhanced intervals in MathUtils. Updated references to Schubfach v4. ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3402/files - new: https://git.openjdk.java.net/jdk/pull/3402/files/37acd52a..45881cd4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=03-04 Stats: 26 lines in 3 files changed: 0 ins; 0 del; 26 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
On Mon, 15 Nov 2021 19:31:09 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4.
Also there is Dragonbox algorithm which might be faster: https://github.com/jk-jeon/dragonbox/blob/master/other_files/Dragonbox.pdf ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
Hi Suminda, please read my reply to you from May https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077081.html Greetings Raffaello On 2021-11-17 14:54, Suminda Sirinath Salpitikorala Dharmasena wrote:
On Mon, 15 Nov 2021 19:31:09 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4.
Also there is Dragonbox algorithm which might be faster: https://github.com/jk-jeon/dragonbox/blob/master/other_files/Dragonbox.pdf
-------------
On Mon, 15 Nov 2021 19:31:09 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4.
DragonBox is a very impressive further achievement over Schubfach, but its extra complexity is not without risk at this time. Also, I have not yet seen an analysis of its additional space overhead (code and data), if any. I concur that it is a good decision at this point to get plain Schubfach going and well-tested in Java. If all goes well, then DragonBox could be considered in the future. Guy Steele On Nov 17, 2021, at 9:20 AM, mlbridge[bot] ***@***.******@***.***>> wrote: Mailing list message from Raffaello ***@***.***> on ***@***.***>: Hi Suminda, please read my reply to you from May https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077081.html Greetings Raffaello On 2021-11-17 14:54, Suminda Sirinath Salpitikorala Dharmasena wrote: — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/3402*issuecomment-971626422__;Iw!!ACWV5N9M2RV99hQ!am0x1_Iy6HoXH4-mh9O90Q6qvI-DzE2WPaqSGojmEbjiwO6Hzl4NgG_JzfLUVQ_Q$>, or unsubscribe<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AAXU22XTDNNY7LNWW3SKSE3UMO22FANCNFSM42TWJIOA__;!!ACWV5N9M2RV99hQ!am0x1_Iy6HoXH4-mh9O90Q6qvI-DzE2WPaqSGojmEbjiwO6Hzl4NgG_JzeS4T6_V$>. Triage notifications on the go with GitHub Mobile for iOS<https://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!ACWV5N9M2RV99hQ!am0x1_Iy6HoXH4-mh9O90Q6qvI-DzE2WPaqSGojmEbjiwO6Hzl4NgG_Jzc0Hy-AJ$> or Android<https://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign*3Dnotification-email*26utm_medium*3Demail*26utm_source*3Dgithub__;JSUlJSU!!ACWV5N9M2RV99hQ!am0x1_Iy6HoXH4-mh9O90Q6qvI-DzE2WPaqSGojmEbjiwO6Hzl4NgG_JzZIzVFIU$>. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Mon, 15 Nov 2021 19:31:09 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4.
Has the paper been published? What is holding this back from merging? ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Mon, 15 Nov 2021 19:31:09 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4.
En attendant go! go! ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 11 Jan 2022 09:30:49 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4.
En attendant go! go!
@rgiulietti Should we expect any more commits from you in this PR? Thanks. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 8 Feb 2022 19:23:54 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
En attendant go! go!
@rgiulietti Should we expect any more commits from you in this PR? Thanks.
@bplb The last commit was pushed on Nov 15. I guess I would have to merge against the latest master because of test/langtools/tools/javac/sym/ElementStructureTest.java and the intervening switch from JDK18 to JDK19. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 8 Feb 2022 19:41:48 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
@rgiulietti Should we expect any more commits from you in this PR? Thanks.
@bplb The last commit was pushed on Nov 15. I guess I would have to merge against the latest master because of test/langtools/tools/javac/sym/ElementStructureTest.java and the intervening switch from JDK18 to JDK19.
@rgiulietti Also the copyright year. So no further substantive updates needed? ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 8 Feb 2022 19:47:25 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
@bplb The last commit was pushed on Nov 15. I guess I would have to merge against the latest master because of test/langtools/tools/javac/sym/ElementStructureTest.java and the intervening switch from JDK18 to JDK19.
@rgiulietti Also the copyright year. So no further substantive updates needed?
@bplb Why the copyright year? Nothing is expected to change, except for the test mentioned. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 8 Feb 2022 19:51:53 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
@rgiulietti Also the copyright year. So no further substantive updates needed?
@bplb Why the copyright year? Nothing is expected to change, except for the test mentioned.
@rgiulietti Well you know I am not sure about the year. I'll ask. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Mon, 15 Nov 2021 19:31:09 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4.
Can someone look into this please. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Mon, 15 Nov 2021 19:31:09 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4.
The work (the code) has been published here on GitHub, so afaik the copyright year cannot be simply changed without modifying something (beside the year itself, which would be self-justifying) ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 8 Feb 2022 20:06:38 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4.
The work (the code) has been published here on GitHub, so afaik the copyright year cannot be simply changed without modifying something (beside the year itself, which would be self-justifying)
@rgiulietti You are correct: no year update needed unless there's a change to the file. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 8 Feb 2022 20:18:22 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
The work (the code) has been published here on GitHub, so afaik the copyright year cannot be simply changed without modifying something (beside the year itself, which would be self-justifying)
@rgiulietti You are correct: no year update needed unless there's a change to the file.
@bplb I'm not even sure that just correcting a typo is considered enough of a change to be entitled to postpone the copyright year. But mileage may vary between jurisdictions ;-) ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits: - 4511638: Double.toString(double) sometimes produces incorrect results Adapted hashes in ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results Enhanced intervals in MathUtils. Updated references to Schubfach v4. - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) comments. - 4511638: Double.toString(double) sometimes produces incorrect results Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results Refactored test classes to better match OpenJDK conventions. Added tests recommended by Guy Steele and Paul Zimmermann. - Changed MAX_CHARS to static - ... and 2 more: https://git.openjdk.java.net/jdk/compare/92f4f40d...c29dff76 ------------- Changes: https://git.openjdk.java.net/jdk/pull/3402/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=05 Stats: 23939 lines in 16 files changed: 23809 ins; 54 del; 76 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 8 Feb 2022 22:11:34 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:
- 4511638: Double.toString(double) sometimes produces incorrect results
Adapted hashes in ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4. - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) comments. - 4511638: Double.toString(double) sometimes produces incorrect results
Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results
Refactored test classes to better match OpenJDK conventions. Added tests recommended by Guy Steele and Paul Zimmermann. - Changed MAX_CHARS to static - ... and 2 more: https://git.openjdk.java.net/jdk/compare/92f4f40d...c29dff76
Marked as reviewed by aturbanov (Committer). ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 8 Feb 2022 22:11:34 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:
- 4511638: Double.toString(double) sometimes produces incorrect results
Adapted hashes in ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4. - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) comments. - 4511638: Double.toString(double) sometimes produces incorrect results
Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results
Refactored test classes to better match OpenJDK conventions. Added tests recommended by Guy Steele and Paul Zimmermann. - Changed MAX_CHARS to static - ... and 2 more: https://git.openjdk.java.net/jdk/compare/92f4f40d...c29dff76
Keep PR open. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 8 Feb 2022 22:11:34 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:
- 4511638: Double.toString(double) sometimes produces incorrect results
Adapted hashes in ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4. - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) comments. - 4511638: Double.toString(double) sometimes produces incorrect results
Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results
Refactored test classes to better match OpenJDK conventions. Added tests recommended by Guy Steele and Paul Zimmermann. - Changed MAX_CHARS to static - ... and 2 more: https://git.openjdk.java.net/jdk/compare/92f4f40d...c29dff76
Why is this still not merged? ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 8 Feb 2022 22:11:34 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:
- 4511638: Double.toString(double) sometimes produces incorrect results
Adapted hashes in ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4. - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) comments. - 4511638: Double.toString(double) sometimes produces incorrect results
Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results
Refactored test classes to better match OpenJDK conventions. Added tests recommended by Guy Steele and Paul Zimmermann. - Changed MAX_CHARS to static - ... and 2 more: https://git.openjdk.java.net/jdk/compare/92f4f40d...c29dff76
Hi Suminda, before merging, an OpenJDK PR needs to be properly reviewed by officially nominated reviewers other than the author of a PR. In addition, this PR also needs a CSR approval because the proposed spec has changed. As you can see on the top of the PR, neither of these two prerequisites for merging are checked as of today. Greetings Raffaello On 3/10/22 07:37, Suminda Sirinath Salpitikorala Dharmasena wrote:
Why is this still not merged?
— Reply to this email directly, view it on GitHub <https://github.com/openjdk/jdk/pull/3402#issuecomment-1063713753>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AQ3TDG2HH4HZA4AFVFNFEYLU7GKBPANCNFSM42TWJIOA>. Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 8 Feb 2022 22:11:34 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:
- 4511638: Double.toString(double) sometimes produces incorrect results
Adapted hashes in ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4. - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) comments. - 4511638: Double.toString(double) sometimes produces incorrect results
Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results
Refactored test classes to better match OpenJDK conventions. Added tests recommended by Guy Steele and Paul Zimmermann. - Changed MAX_CHARS to static - ... and 2 more: https://git.openjdk.java.net/jdk/compare/92f4f40d...c29dff76
for the record, I post here the review we did with Jean-Michel Muller of the Schubfach algorithm in last October [review.pdf](https://github.com/openjdk/jdk/files/8471359/review.pdf) ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 8 Feb 2022 22:11:34 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:
- 4511638: Double.toString(double) sometimes produces incorrect results
Adapted hashes in ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4. - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) comments. - 4511638: Double.toString(double) sometimes produces incorrect results
Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results
Refactored test classes to better match OpenJDK conventions. Added tests recommended by Guy Steele and Paul Zimmermann. - Changed MAX_CHARS to static - ... and 2 more: https://git.openjdk.java.net/jdk/compare/92f4f40d...c29dff76
Thanks Paul, really appreciate. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 13 commits: - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results Adapted hashes in ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results Enhanced intervals in MathUtils. Updated references to Schubfach v4. - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) comments. - 4511638: Double.toString(double) sometimes produces incorrect results Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results Refactored test classes to better match OpenJDK conventions. Added tests recommended by Guy Steele and Paul Zimmermann. - ... and 3 more: https://git.openjdk.java.net/jdk/compare/dd06cc63...a36ff8ac ------------- Changes: https://git.openjdk.java.net/jdk/pull/3402/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=06 Stats: 23938 lines in 16 files changed: 23809 ins; 54 del; 75 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 4511638: Double.toString(double) sometimes produces incorrect results ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3402/files - new: https://git.openjdk.java.net/jdk/pull/3402/files/a36ff8ac..904ba115 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=06-07 Stats: 389 lines in 5 files changed: 40 ins; 216 del; 133 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 4511638: Double.toString(double) sometimes produces incorrect results ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3402/files - new: https://git.openjdk.java.net/jdk/pull/3402/files/904ba115..e7c4bd25 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=07-08 Stats: 4 lines in 3 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
On Fri, 6 May 2022 08:40:40 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Match the CSR Updated Schubfach writing URL ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 16 commits: - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results Adapted hashes in ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results Enhanced intervals in MathUtils. Updated references to Schubfach v4. - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) comments. - 4511638: Double.toString(double) sometimes produces incorrect results Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - ... and 6 more: https://git.openjdk.java.net/jdk/compare/d4474b58...bd323d15 ------------- Changes: https://git.openjdk.java.net/jdk/pull/3402/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=09 Stats: 23708 lines in 16 files changed: 23625 ins; 46 del; 37 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
On Mon, 9 May 2022 09:26:20 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 16 commits:
- 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Adapted hashes in ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4. - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) comments. - 4511638: Double.toString(double) sometimes produces incorrect results
Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - ... and 6 more: https://git.openjdk.java.net/jdk/compare/d4474b58...bd323d15
Post Loom merge ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Mon, 9 May 2022 09:26:20 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 16 commits:
- 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Adapted hashes in ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Enhanced intervals in MathUtils. Updated references to Schubfach v4. - 4511638: Double.toString(double) sometimes produces incorrect results
Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results
Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) comments. - 4511638: Double.toString(double) sometimes produces incorrect results
Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - ... and 6 more: https://git.openjdk.java.net/jdk/compare/d4474b58...bd323d15
Getting rid of ThreadLocal ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 4511638: Double.toString(double) sometimes produces incorrect results ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3402/files - new: https://git.openjdk.java.net/jdk/pull/3402/files/bd323d15..907abfd6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=09-10 Stats: 30 lines in 2 files changed: 2 ins; 24 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
On Mon, 9 May 2022 12:34:20 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Marked as reviewed by limck599@github.com (no known OpenJDK username). ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Mon, 9 May 2022 12:34:20 GMT, limck599 <duke@openjdk.java.net> wrote:
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Marked as reviewed by limck599@github.com (no known OpenJDK username).
@limck599 While we at OpenJDK appreciate constructive reviews from GitHub users not registered in the [census](https://openjdk.java.net/census), only officially nominated reviewers have the authority to approve this PR. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Mon, 9 May 2022 12:50:31 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Marked as reviewed by limck599@github.com (no known OpenJDK username).
@limck599 While we at OpenJDK appreciate constructive reviews from GitHub users not registered in the [census](https://openjdk.java.net/census), only officially nominated reviewers have the authority to approve this PR.
@rgiulietti friendly ping, noob question: Do you believe some parts of the algorithm could exploit explicit SIMD instructions (via the high level [Vector API](https://openjdk.java.net/jeps/417)) for even better performance (such as https://github.com/wrandelshofer/FastDoubleParser ) ? ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
Hi, the aim of this PR was firstly to have a correct implementation of the spec. Speed improvement was a secondary goal, which was intentionally pursued with conventional Java arithmetic and just a few calls to simple methods in the standard library. Whether the Vector API (which is still in incubation phase) might help in this specific case remains to be explored. As a first impression, there seem to be no good spots to parallelize using SIMD instructions. But engineers with more SIMD experience might find creative ways to make good use of them. HTH Raffaello On 2022-06-07 18:35, LifeIsStrange wrote:
On Mon, 9 May 2022 12:50:31 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Marked as reviewed by limck599@github.com (no known OpenJDK username).
@limck599 While we at OpenJDK appreciate constructive reviews from GitHub users not registered in the [census](https://openjdk.java.net/census), only officially nominated reviewers have the authority to approve this PR.
@rgiulietti friendly ping, noob question: Do you believe some parts of the algorithm could exploit explicit SIMD instructions (via the high level [Vector API](https://openjdk.java.net/jeps/417)) for even better performance (such as https://github.com/wrandelshofer/FastDoubleParser ) ?
-------------
On Mon, 9 May 2022 12:38:48 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Generally looks solid! src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 882:
880: try { 881: FloatToDecimal.appendTo(f, this); 882: } catch (IOException ignored) {
What is the motivation for wrapping with IOException? src/java.base/share/classes/java/lang/Double.java line 33:
31: import java.util.Optional; 32: 33: import jdk.internal.math.FloatingDecimal;
Presumably the FloatingDecimal import here and in Float can be removed. src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 38:
36: * This class exposes a method to render a {@code double} as a string. 37: * 38: * @author Raffaello Giulietti
New code shouldn't use author tags. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 10 May 2022 03:21:21 GMT, Joe Darcy <darcy@openjdk.org> wrote:
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 882:
880: try { 881: FloatToDecimal.appendTo(f, this); 882: } catch (IOException ignored) {
What is the motivation for wrapping with IOException?
`[Float|Double]ToDecimal` do not have access to `AbstractStringBuilder`, so have to fail over `Appendable`, which can throw `IOException` in `append(*)` methods. I have to find another way if this wrapping to make the compiler happy is unacceptable. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 10 May 2022 15:50:26 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
`[Float|Double]ToDecimal` do not have access to `AbstractStringBuilder`, so have to fail over `Appendable`, which can throw `IOException` in `append(*)` methods.
I have to find another way if this wrapping to make the compiler happy is unacceptable.
Ah, I haven't used it myself recently enough to recall the details, but I believe the shared secrets mechanism is intended to allow such cross-package access. If the current code is kept and if the exception can actually be thrown, explicitly throwing an assertion error is preferable to "assert false". ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 10 May 2022 03:22:01 GMT, Joe Darcy <darcy@openjdk.org> wrote:
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/java/lang/Double.java line 33:
31: import java.util.Optional; 32: 33: import jdk.internal.math.FloatingDecimal;
Presumably the FloatingDecimal import here and in Float can be removed.
`FloatingDecimal` is used in the `parse*(String)` methods ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 4511638: Double.toString(double) sometimes produces incorrect results ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3402/files - new: https://git.openjdk.java.net/jdk/pull/3402/files/907abfd6..b40d7e80 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=10-11 Stats: 28 lines in 7 files changed: 0 ins; 9 del; 19 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 4511638: Double.toString(double) sometimes produces incorrect results ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3402/files - new: https://git.openjdk.java.net/jdk/pull/3402/files/b40d7e80..31ca4e10 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=12 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=11-12 Stats: 54 lines in 4 files changed: 0 ins; 0 del; 54 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 4511638: Double.toString(double) sometimes produces incorrect results ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3402/files - new: https://git.openjdk.java.net/jdk/pull/3402/files/31ca4e10..ad146ec4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=13 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=12-13 Stats: 19594 lines in 1 file changed: 0 ins; 19594 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/MathUtils.java line 38:
36: * 37: * Giulietti, "The Schubfach way to render doubles", 38: * https://drive.google.com/file/d/1gp5xv4CAa78SVgCeWfGqqI4FfYYYuNFb
Even though not public, should the reference use the `<cite/>` tag and perhaps be in a `@see` annotation? @see <a href=“https://drive.google.com/file/d/1gp5xv4CAa78SVgCeWfGqqI4FfYYYuNFb”> <cite>The Schubfach way to render doubles</cite></a> src/java.base/share/classes/jdk/internal/math/MathUtils.java line 193:
191: */ 192: private static final long[] g = { 193: /* -324 */ 0x4F0C_EDC9_5A71_8DD4L, 0x5B01_E8B0_9AA0_D1B5L,
Not significant, but might this be clearer instead to comment the array elements on the right? 0x4F0C_EDC9_5A71_8DD4L, 0x5B01_E8B0_9AA0_D1B5L, // -324 ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 31 May 2022 21:35:08 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/MathUtils.java line 38:
36: * 37: * Giulietti, "The Schubfach way to render doubles", 38: * https://drive.google.com/file/d/1gp5xv4CAa78SVgCeWfGqqI4FfYYYuNFb
Even though not public, should the reference use the `<cite/>` tag and perhaps be in a `@see` annotation?
@see <a href=“https://drive.google.com/file/d/1gp5xv4CAa78SVgCeWfGqqI4FfYYYuNFb”> <cite>The Schubfach way to render doubles</cite></a>
These references are inside a normal multi-line comment, so I'm not sure that Javadoc or html tags or have a well-defined semantics there. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Wed, 1 Jun 2022 08:56:38 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
src/java.base/share/classes/jdk/internal/math/MathUtils.java line 38:
36: * 37: * Giulietti, "The Schubfach way to render doubles", 38: * https://drive.google.com/file/d/1gp5xv4CAa78SVgCeWfGqqI4FfYYYuNFb
Even though not public, should the reference use the `<cite/>` tag and perhaps be in a `@see` annotation?
@see <a href=“https://drive.google.com/file/d/1gp5xv4CAa78SVgCeWfGqqI4FfYYYuNFb”> <cite>The Schubfach way to render doubles</cite></a>
These references are inside a normal multi-line comment, so I'm not sure that Javadoc or html tags or have a well-defined semantics there.
I'm not sure about linking to a document on Google Drive, even from JDK internal classes. Is the paper uploaded to anywhere else that could be linked to instead? ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 47:
45: * [2] IEEE Computer Society, "IEEE Standard for Floating-Point Arithmetic" 46: * 47: * [3] Bouvier & Zimmermann, "Division-Free Binary-to-Decimal Conversion"
Similar comment concerning `<cite/>` tag as in `MathUtils`. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 97:
95: private static final int MASK_28 = (1 << 28) - 1; 96: 97: private static final int NON_SPECIAL = 0;
Would these constants be better as an enum? src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 118:
116: private int index; 117: 118: private DoubleToDecimal() {
Maybe add a comment like /** * Prevent instantiation. */ or // Prevent instantiation of this class. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 31 May 2022 21:57:44 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 118:
116: private int index; 117: 118: private DoubleToDecimal() {
Maybe add a comment like
/** * Prevent instantiation. */
or
// Prevent instantiation of this class.
The constructor _is_ invoked, so the comment would be inappropriate. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Wed, 1 Jun 2022 09:21:43 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 118:
116: private int index; 117: 118: private DoubleToDecimal() {
Maybe add a comment like
/** * Prevent instantiation. */
or
// Prevent instantiation of this class.
The constructor _is_ invoked, so the comment would be inappropriate.
... but I added a `throw` in the constructor of `MathUtils` ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Jun 1, 2022, at 2:25 AM, Raffaello Giulietti <duke@openjdk.java.net<mailto:duke@openjdk.java.net>> wrote: On Tue, 31 May 2022 21:57:44 GMT, Brian Burkhalter <bpb@openjdk.org<mailto:bpb@openjdk.org>> wrote: Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 4511638: Double.toString(double) sometimes produces incorrect results src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 118: 116: private int index; 117: 118: private DoubleToDecimal() { Maybe add a comment like /** * Prevent instantiation. */ or // Prevent instantiation of this class. The constructor _is_ invoked, so the comment would be inappropriate. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402 Mea culpa.
On Tue, 31 May 2022 21:55:16 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 97:
95: private static final int MASK_28 = (1 << 28) - 1; 96: 97: private static final int NON_SPECIAL = 0;
Would these constants be better as an enum?
An enum would make much sense if it were used by other parts of the codebase, and then it would be moved to `MathUtils`. This might well happen in the near future, when this code could be enhanced to be used in formatting conversions, like in "`printf()`" and friends. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/FloatToDecimal.java line 40:
38: final public class FloatToDecimal { 39: /* 40: * For full details about this code see the following references:
Same comment about `<cite/>`. src/java.base/share/classes/jdk/internal/math/FloatToDecimal.java line 97:
95: private static final int MASK_28 = (1 << 28) - 1; 96: 97: private static final int NON_SPECIAL = 0;
As these are shared with `DoubleToDecimal` would these constants be better moved to a common location, e.g., `MathUtils` whether converted to an `enum` or not? src/java.base/share/classes/jdk/internal/math/FloatToDecimal.java line 118:
116: private int index; 117: 118: private FloatToDecimal() {
Same comment about preventing instantiation. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Tue, 31 May 2022 22:11:54 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/FloatToDecimal.java line 97:
95: private static final int MASK_28 = (1 << 28) - 1; 96: 97: private static final int NON_SPECIAL = 0;
As these are shared with `DoubleToDecimal` would these constants be better moved to a common location, e.g., `MathUtils` whether converted to an `enum` or not?
As long as the values are constant `ints`, moving them to `MathUtils` is less robust. Changing any value would require remembering to recompile the "clients". The move would make sense if these were an enum. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Jun 1, 2022, at 3:32 AM, Raffaello Giulietti <duke@openjdk.java.net> wrote:
On Tue, 31 May 2022 22:11:54 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/FloatToDecimal.java line 97:
95: private static final int MASK_28 = (1 << 28) - 1; 96: 97: private static final int NON_SPECIAL = 0;
As these are shared with `DoubleToDecimal` would these constants be better moved to a common location, e.g., `MathUtils` whether converted to an `enum` or not?
As long as the values are constant `ints`, moving them to `MathUtils` is less robust. Changing any value would require remembering to recompile the "clients". The move would make sense if these were an enum.
-------------
Understood. All of that can wait until later.
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 72:
70: static final int E_MAX = 309; 71: 72: /* Threshold to detect tiny values, as in section 8.2.1 of [1] */
Comments like this one pointing to specific places in the Schubfach paper are very helpful in understanding the constants and the various steps in the algorithm. ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 4511638: Double.toString(double) sometimes produces incorrect results ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3402/files - new: https://git.openjdk.java.net/jdk/pull/3402/files/ad146ec4..93711bae Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=14 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=13-14 Stats: 618 lines in 1 file changed: 1 ins; 0 del; 617 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
On Wed, 1 Jun 2022 10:37:23 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
I think it's time for this excellent work to advance. ------------- Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3402
On Wed, 1 Jun 2022 10:37:23 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Marked as reviewed by darcy (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Wed, 1 Jun 2022 10:37:23 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
Hallelujah! ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti <duke@openjdk.java.net> wrote:
Hello,
here's a PR for a patch submitted on March 2020 [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a thing.
The patch has been edited to adhere to OpenJDK code conventions about multi-line (block) comments. Nothing in the code proper has changed, except for the addition of redundant but clarifying parentheses in some expressions.
Greetings Raffaello
This pull request has now been integrated. Changeset: 72bcf2aa Author: Raffaello Giulietti <raffaello.giulietti@oracle.com> Committer: Joe Darcy <darcy@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/72bcf2aa03d53b0f68eb07a902575b4e8628... Stats: 4121 lines in 18 files changed: 4001 ins; 46 del; 74 mod 4511638: Double.toString(double) sometimes produces incorrect results Reviewed-by: aturbanov, darcy, bpb ------------- PR: https://git.openjdk.java.net/jdk/pull/3402
participants (21)
-
Alan Bateman
-
Andrew Dinn
-
Andrew Haley
-
Andrew Haley
-
Andrey Turbanov
-
Andrey Turbanov
-
Brian Burkhalter
-
Brian Burkhalter
-
GuySteele
-
Jan Lahoda
-
Jim Laskey
-
Joe Darcy
-
Joe Darcy
-
LifeIsStrange
-
limck599
-
Raffaello Giulietti
-
Raffaello Giulietti
-
Raffaello Giulietti
-
Raffaello Giulietti
-
Suminda Sirinath Salpitikorala Dharmasena
-
zimmermann6