A guide to PR3402 ("[Double,Float].toString()") code reviewers
Raffaello Giulietti
raffaello.giulietti at gmail.com
Tue Nov 16 20:10:21 UTC 2021
Hello,
as mentioned in [1], the Schubfach writing accompanying JBS issues [2]
and [3] has recently undergone a thorough review by renowned researchers
(in addition to Dmitry Nadezhin back in 2018-19 and myself on a
continuous basis) [4].
For the *code* reviewers of PR [5] this means that there's no need for
them to deal with the more mathematical aspects of the writing. They can
instead concentrate in crosschecking that the Java code is in sync with
the pseudo-code in the writing, as well as about its readability.
To facilitate this review, there are comments scattered in the code that
refer back to the writing for easy reference.
# MathUtils
For the flog*() methods in MathUtils refer to section 9.1.1.
The large lookup table in MathUtils is defined in result 24 as well as
in javadoc.
MathUtilsChecker contains fully commented code that tests the
correctness of the table as well as the flog*() methods to an extent
sufficient for the conversions of doubles.
(The table has also been checked independently by one of the article
reviewers.)
# DoubleToDecimal
With regard to DoubleToDecimal, the fast path described in 8.3 is coded
as part of toDecimal(double).
In DoubleToDecimal, toDecimal(int,long,int) is a Java translation of the
Schubfach skeleton in figure 7 combined with the suggestions in figure 9.
Figure 8 is embodied in rop(long,long,long).
§10 about digits extraction is implemented in toChars(long,int) and its
dependencies.
The rest of the code is just glue and should be easy to understand.
# FloatToDecimal
The code of FloatToDecimal is almost identical but adapted to the 32 bit
case. Method rop(long,long) is from figure 11.
# Test classes
I hope it turns out that the test classes are either commented well
enough to guide the reader or are kind of straightforward to understand.
There are some package private constants in the code that are not
referenced anywhere else in src/ and could thus be removed. However,
since there's no great harm retaining them and since they might turn out
to be useful in later work, I would suggest not to get rid of them.
Of course, I'm at the disposal of this community for any question
related to the PR and the writing.
HTH
Raffaello
----
[1]
https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/083536.html
[2] https://bugs.openjdk.java.net/browse/JDK-4511638
[3] https://bugs.openjdk.java.net/browse/JDK-8202555
[4] https://drive.google.com/file/d/1IEeATSVnEE6TkrHlCYNY2GjaraBjOT4f
[5] https://github.com/openjdk/jdk/pull/3402
More information about the core-libs-dev
mailing list