-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8366017: Extend the set of inputs handled by fast paths in FloatingDecimal #26990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
👋 Welcome back rgiulietti! A progress list of the required criteria for merging this PR into |
|
@rgiulietti This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 37 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
|
@rgiulietti The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
|
Parsing proper has been the topic of a previous PR. However, the mathematical conversion from the decimal representation itself still relied on existing code. The plan is to rework the conversion algorithms and end up with a 3 tiers schema.
|
| private static final int EXP_SHIFT = DoubleConsts.SIGNIFICAND_WIDTH - 1; | ||
| private static final long FRACT_HOB = 1L << EXP_SHIFT; // assumed High-Order bit | ||
| private static final long EXP_ONE = (long) DoubleConsts.EXP_BIAS << EXP_SHIFT; // exponent of 1.0 | ||
| private static final int MAX_SMALL_BIN_EXP = 62; | ||
| private static final int MIN_SMALL_BIN_EXP = -63 / 3; | ||
| private static final int MAX_DECIMAL_DIGITS = 15; // max{n : 10^n <= 2^P} | ||
| private static final int FLOG_10_MAX_LONG = 18; // max{i : 10^i ≤ Long.MAX_VALUE} | ||
|
|
||
| private static final int SINGLE_EXP_SHIFT = FloatConsts.SIGNIFICAND_WIDTH - 1; | ||
| private static final int SINGLE_FRACT_HOB = 1 << SINGLE_EXP_SHIFT; | ||
| private static final int SINGLE_MAX_DECIMAL_DIGITS = 7; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| private static final int EXP_SHIFT = DoubleConsts.SIGNIFICAND_WIDTH - 1; | |
| private static final long FRACT_HOB = 1L << EXP_SHIFT; // assumed High-Order bit | |
| private static final long EXP_ONE = (long) DoubleConsts.EXP_BIAS << EXP_SHIFT; // exponent of 1.0 | |
| private static final int MAX_SMALL_BIN_EXP = 62; | |
| private static final int MIN_SMALL_BIN_EXP = -63 / 3; | |
| private static final int MAX_DECIMAL_DIGITS = 15; // max{n : 10^n <= 2^P} | |
| private static final int FLOG_10_MAX_LONG = 18; // max{i : 10^i ≤ Long.MAX_VALUE} | |
| private static final int SINGLE_EXP_SHIFT = FloatConsts.SIGNIFICAND_WIDTH - 1; | |
| private static final int SINGLE_FRACT_HOB = 1 << SINGLE_EXP_SHIFT; | |
| private static final int SINGLE_MAX_DECIMAL_DIGITS = 7; | |
| private static final int | |
| EXP_SHIFT = DoubleConsts.SIGNIFICAND_WIDTH - 1, | |
| MAX_SMALL_BIN_EXP = 62, | |
| MIN_SMALL_BIN_EXP = -63 / 3, | |
| MAX_DECIMAL_DIGITS = 15, // max{n : 10^n <= 2^P} | |
| FLOG_10_MAX_LONG = 18, // max{i : 10^i ≤ Long.MAX_VALUE} | |
| SINGLE_EXP_SHIFT = FloatConsts.SIGNIFICAND_WIDTH - 1, | |
| SINGLE_FRACT_HOB = 1 << SINGLE_EXP_SHIFT, | |
| SINGLE_MAX_DECIMAL_DIGITS = 7; | |
| private static final long | |
| FRACT_HOB = 1L << EXP_SHIFT, // assumed High-Order bit | |
| EXP_ONE = (long) DoubleConsts.EXP_BIAS << EXP_SHIFT; // exponent of 1.0 |
How about this style?
| private static final BinaryToASCIIConverter B2AC_POSITIVE_INFINITY = new ExceptionalBinaryToASCIIBuffer(INFINITY_REP); | ||
| private static final BinaryToASCIIConverter B2AC_NEGATIVE_INFINITY = new ExceptionalBinaryToASCIIBuffer("-" + INFINITY_REP); | ||
| private static final BinaryToASCIIConverter B2AC_NOT_A_NUMBER = new ExceptionalBinaryToASCIIBuffer(NAN_REP); | ||
| private static final BinaryToASCIIConverter B2AC_POSITIVE_ZERO = new BinaryToASCIIBuffer(false, new byte[] {'0'}); | ||
| private static final BinaryToASCIIConverter B2AC_NEGATIVE_ZERO = new BinaryToASCIIBuffer(true, new byte[] {'0'}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| private static final BinaryToASCIIConverter B2AC_POSITIVE_INFINITY = new ExceptionalBinaryToASCIIBuffer(INFINITY_REP); | |
| private static final BinaryToASCIIConverter B2AC_NEGATIVE_INFINITY = new ExceptionalBinaryToASCIIBuffer("-" + INFINITY_REP); | |
| private static final BinaryToASCIIConverter B2AC_NOT_A_NUMBER = new ExceptionalBinaryToASCIIBuffer(NAN_REP); | |
| private static final BinaryToASCIIConverter B2AC_POSITIVE_ZERO = new BinaryToASCIIBuffer(false, new byte[] {'0'}); | |
| private static final BinaryToASCIIConverter B2AC_NEGATIVE_ZERO = new BinaryToASCIIBuffer(true, new byte[] {'0'}); | |
| private static final BinaryToASCIIConverter | |
| B2AC_POSITIVE_INFINITY = new ExceptionalBinaryToASCIIBuffer(INFINITY_REP), | |
| B2AC_NEGATIVE_INFINITY = new ExceptionalBinaryToASCIIBuffer("-" + INFINITY_REP), | |
| B2AC_NOT_A_NUMBER = new ExceptionalBinaryToASCIIBuffer(NAN_REP), | |
| B2AC_POSITIVE_ZERO = new BinaryToASCIIBuffer(false, new byte[] {'0'}), | |
| B2AC_NEGATIVE_ZERO = new BinaryToASCIIBuffer(true, new byte[] {'0'}); |
| private static final PreparedASCIIToBinaryBuffer A2BC_POSITIVE_INFINITY = | ||
| new PreparedASCIIToBinaryBuffer(Double.POSITIVE_INFINITY, Float.POSITIVE_INFINITY); | ||
| private static final PreparedASCIIToBinaryBuffer A2BC_NEGATIVE_INFINITY = | ||
| new PreparedASCIIToBinaryBuffer(Double.NEGATIVE_INFINITY, Float.NEGATIVE_INFINITY); | ||
| private static final PreparedASCIIToBinaryBuffer A2BC_NOT_A_NUMBER = | ||
| new PreparedASCIIToBinaryBuffer(Double.NaN, Float.NaN); | ||
| private static final PreparedASCIIToBinaryBuffer A2BC_POSITIVE_ZERO = | ||
| new PreparedASCIIToBinaryBuffer(0.0d, 0.0f); | ||
| private static final PreparedASCIIToBinaryBuffer A2BC_NEGATIVE_ZERO = | ||
| new PreparedASCIIToBinaryBuffer(-0.0d, -0.0f); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| private static final PreparedASCIIToBinaryBuffer A2BC_POSITIVE_INFINITY = | |
| new PreparedASCIIToBinaryBuffer(Double.POSITIVE_INFINITY, Float.POSITIVE_INFINITY); | |
| private static final PreparedASCIIToBinaryBuffer A2BC_NEGATIVE_INFINITY = | |
| new PreparedASCIIToBinaryBuffer(Double.NEGATIVE_INFINITY, Float.NEGATIVE_INFINITY); | |
| private static final PreparedASCIIToBinaryBuffer A2BC_NOT_A_NUMBER = | |
| new PreparedASCIIToBinaryBuffer(Double.NaN, Float.NaN); | |
| private static final PreparedASCIIToBinaryBuffer A2BC_POSITIVE_ZERO = | |
| new PreparedASCIIToBinaryBuffer(0.0d, 0.0f); | |
| private static final PreparedASCIIToBinaryBuffer A2BC_NEGATIVE_ZERO = | |
| new PreparedASCIIToBinaryBuffer(-0.0d, -0.0f); | |
| private static final PreparedASCIIToBinaryBuffer | |
| A2BC_POSITIVE_INFINITY = new PreparedASCIIToBinaryBuffer(Double.POSITIVE_INFINITY, Float.POSITIVE_INFINITY), | |
| A2BC_NEGATIVE_INFINITY = new PreparedASCIIToBinaryBuffer(Double.NEGATIVE_INFINITY, Float.NEGATIVE_INFINITY), | |
| A2BC_NOT_A_NUMBER = new PreparedASCIIToBinaryBuffer(Double.NaN, Float.NaN), | |
| A2BC_POSITIVE_ZERO = new PreparedASCIIToBinaryBuffer(0.0d, 0.0f), | |
| A2BC_NEGATIVE_ZERO = new PreparedASCIIToBinaryBuffer(-0.0d, -0.0f); |
Webrevs
|
|
This work's focus is on:
In particular, parsing outcomes of the Before After |
| CDS.initializeFromArchive(FDBigInteger.class); | ||
| Object[] caches = archivedCaches; | ||
| if (caches == null) { | ||
| long[] long5pow = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worthwhile to make these arrays @Stable?
In the future, might be a candidate for StableConstant/LazyValue functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worthwhile to make these arrays
@Stable?In the future, might be a candidate for StableConstant/LazyValue functionality.
PS If each array is read only once, then perhaps the other data structures would be better initialized in a loop.
|
|
||
| private static void testFastPaths() { | ||
| /* Exercises the fast paths in jdk.internal.math.FloatingDecimal. */ | ||
| check("1", 1.0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something to consider: specify the double values in hex literals -- this would exercise entirely different code paths than the decimal ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I fully agree.
Updating the test to use hexadecimals...
|
/integrate |
|
Going to push as commit deb7edb.
Your commit was automatically rebased without conflicts. |
|
@rgiulietti Pushed as commit deb7edb. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Yet another step in modernizing
FloatingDecimals floating-point parsing.Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26990/head:pull/26990$ git checkout pull/26990Update a local copy of the PR:
$ git checkout pull/26990$ git pull https://git.openjdk.org/jdk.git pull/26990/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 26990View PR using the GUI difftool:
$ git pr show -t 26990Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26990.diff
Using Webrev
Link to Webrev Comment