Skip to content

Commit

Permalink
Fix issue #80: Bug in large array size inputs for JavaBigDecimalParse…
Browse files Browse the repository at this point in the history 
…r.parseBigDecimal.

AbstractBigDecimalParserTest.java:
- Add a test for the input value that reproduces the problem.

JavaBigDecimalFromByteArray.java,
JavaBigDecimalFromCharArray.java,
JavaBigDecimalFromCharSequence.java:
- Fix the off-by-one issue in JavaBigDecimalFromCharArray.
- Harmonize the code in all 3 implementations.

Strings.java,
FftMultiplierTest.java,
JmhBigDecimalScalability.java,
JmhDoubleScalability.java,
JmhJavaBigIntegerFromByteArrayScalability.java,
JmhJavaDoubleFromCharSequenceScalability.java:
 - Implement a better method for creating Strings from repeated characters.
  • Loading branch information
@wrandelshofer
wrandelshofer committed Sep 13, 2024
1 parent 13272f5 commit e388c78
Show file tree
Hide file tree
Showing 10 changed files with 107 additions and 54 deletions.
17 changes: 9 additions & 8 deletions ...shofer.fastdoubleparser/ch/randelshofer/fastdoubleparser/JavaBigDecimalFromByteArray.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ public BigDecimal parseBigDecimalString(byte[] str, int offset, int length) {
BigDecimal parseBigDecimalStringWithManyDigits(byte[] str, int offset, int length) {
final int integerPartIndex;
final int nonZeroIntegerPartIndex;
int decimalPointIndex = -1;
int nonZeroFractionalPartIndex = -1;
int decimalPointIndex = -1;
final int exponentIndicatorIndex;

final int endIndex = offset + length;
Expand All @@ -169,15 +169,17 @@ BigDecimal parseBigDecimalStringWithManyDigits(byte[] str, int offset, int lengt
// -----------------
// skip leading zeroes
integerPartIndex = index;
while (index < endIndex - 8 && FastDoubleSwar.isEightZeroes(str, index)) {
// swarLimit: We can process blocks of eight chars with SWAR, we must process the remaining chars individually.
int swarLimit = Math.min(endIndex - 8, 1 << 30);
while (index < swarLimit && FastDoubleSwar.isEightZeroes(str, index)) {
index += 8;
}
while (index < endIndex && str[index] == '0') {
index++;
}
// Count digits of integer part
nonZeroIntegerPartIndex = index;
while (index < endIndex - 8 && FastDoubleSwar.isEightDigits(str, index)) {
while (index < swarLimit && FastDoubleSwar.isEightDigits(str, index)) {
index += 8;
}
while (index < endIndex && FastDoubleSwar.isDigit(ch = str[index])) {
Expand All @@ -186,15 +188,15 @@ BigDecimal parseBigDecimalStringWithManyDigits(byte[] str, int offset, int lengt
if (ch == '.') {
decimalPointIndex = index++;
// skip leading zeroes
while (index < endIndex - 8 && FastDoubleSwar.isEightZeroes(str, index)) {
while (index < swarLimit && FastDoubleSwar.isEightZeroes(str, index)) {
index += 8;
}
while (index < endIndex && str[index] == '0') {
index++;
}
nonZeroFractionalPartIndex = index;
// Count digits of fraction part
while (index < endIndex - 8 && FastDoubleSwar.isEightDigits(str, index)) {
while (index < swarLimit && FastDoubleSwar.isEightDigits(str, index)) {
index += 8;
}
while (index < endIndex && FastDoubleSwar.isDigit(ch = str[index])) {
Expand Down Expand Up @@ -247,8 +249,7 @@ BigDecimal parseBigDecimalStringWithManyDigits(byte[] str, int offset, int lengt
illegal |= integerPartIndex == decimalPointIndex && decimalPointIndex == exponentIndicatorIndex;
checkParsedBigDecimalBounds(illegal, index, endIndex, digitCountWithoutLeadingZeros, exponent);

return valueOfBigDecimalString(str, nonZeroIntegerPartIndex, decimalPointIndex, nonZeroFractionalPartIndex, exponentIndicatorIndex, isNegative, (int) exponent
);
return valueOfBigDecimalString(str, nonZeroIntegerPartIndex, decimalPointIndex, nonZeroFractionalPartIndex, exponentIndicatorIndex, isNegative, (int) exponent);
}

/**
Expand Down Expand Up @@ -313,7 +314,7 @@ BigDecimal valueOfBigDecimalString(byte[] str, int integerPartIndex, int decimal
} else {
fractionalPart = ParseDigitsTaskByteArray.parseDigitsIterative(str, nonZeroFractionalPartIndex, exponentIndicatorIndex);
}
// If the integer part is not 0, we combine it with the fraction part.
// If the integer part is 0, we can just use the fractional part.
if (integerPart.signum() == 0) {
significand = fractionalPart;
} else {
Expand Down
21 changes: 11 additions & 10 deletions ...shofer.fastdoubleparser/ch/randelshofer/fastdoubleparser/JavaBigDecimalFromCharArray.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ public BigDecimal parseBigDecimalString(char[] str, int offset, int length) {
* Parses a big decimal string that has many digits.
*/
BigDecimal parseBigDecimalStringWithManyDigits(char[] str, int offset, int length) {
final int nonZeroIntegerPartIndex;
final int integerPartIndex;
final int nonZeroIntegerPartIndex;
int nonZeroFractionalPartIndex = -1;
int decimalPointIndex = -1;
final int exponentIndicatorIndex;
Expand All @@ -164,7 +164,9 @@ BigDecimal parseBigDecimalStringWithManyDigits(char[] str, int offset, int lengt

// Count digits of significand
// -----------------
// skip leading zeroes
integerPartIndex = index;
// swarLimit: We can process blocks of eight chars with SWAR, we must process the remaining chars individually.
int swarLimit = Math.min(endIndex - 8, 1 << 30);
while (index < swarLimit && FastDoubleSwar.isEightZeroes(str, index)) {
index += 8;
Expand Down Expand Up @@ -244,10 +246,10 @@ BigDecimal parseBigDecimalStringWithManyDigits(char[] str, int offset, int lengt
illegal |= integerPartIndex == decimalPointIndex && decimalPointIndex == exponentIndicatorIndex;
checkParsedBigDecimalBounds(illegal, index, endIndex, digitCountWithoutLeadingZeros, exponent);

return valueOfBigDecimalString(str, nonZeroIntegerPartIndex, decimalPointIndex, nonZeroFractionalPartIndex, exponentIndicatorIndex, isNegative, (int) exponent);
return valueOfBigDecimalString(str, nonZeroIntegerPartIndex, decimalPointIndex, nonZeroFractionalPartIndex, exponentIndicatorIndex, isNegative, (int) exponent
);
}


/**
* Parses a big decimal string after we have identified the parts of the significand,
* and after we have obtained the exponent value.
Expand All @@ -273,8 +275,7 @@ BigDecimal parseBigDecimalStringWithManyDigits(char[] str, int offset, int lengt
* @return the parsed big decimal
*/
BigDecimal valueOfBigDecimalString(char[] str, int integerPartIndex, int decimalPointIndex, int nonZeroFractionalPartIndex, int exponentIndicatorIndex, boolean isNegative, int exponent) {
int integerExponent = exponentIndicatorIndex - decimalPointIndex - 1;
int fractionDigitsCount = exponentIndicatorIndex - nonZeroFractionalPartIndex;
int fractionDigitsCount = exponentIndicatorIndex - decimalPointIndex - 1;
int nonZeroFractionDigitsCount = exponentIndicatorIndex - nonZeroFractionalPartIndex;
int integerDigitsCount = decimalPointIndex - integerPartIndex;
NavigableMap<Integer, BigInteger> powersOfTen = null;
Expand Down Expand Up @@ -306,16 +307,16 @@ BigDecimal valueOfBigDecimalString(char[] str, int integerPartIndex, int decimal
if (powersOfTen == null) {
powersOfTen = createPowersOfTenFloor16Map();
}
fillPowersOfNFloor16Recursive(powersOfTen, decimalPointIndex + 1, exponentIndicatorIndex);
fractionalPart = ParseDigitsTaskCharArray.parseDigitsRecursive(str, decimalPointIndex + 1, exponentIndicatorIndex, powersOfTen, RECURSION_THRESHOLD);
fillPowersOfNFloor16Recursive(powersOfTen, nonZeroFractionalPartIndex, exponentIndicatorIndex);
fractionalPart = ParseDigitsTaskCharArray.parseDigitsRecursive(str, nonZeroFractionalPartIndex, exponentIndicatorIndex, powersOfTen, RECURSION_THRESHOLD);
} else {
fractionalPart = ParseDigitsTaskCharArray.parseDigitsIterative(str, decimalPointIndex + 1, exponentIndicatorIndex);
fractionalPart = ParseDigitsTaskCharArray.parseDigitsIterative(str, nonZeroFractionalPartIndex, exponentIndicatorIndex);
}
// If the integer part is not 0, we combine it with the fraction part.
// If the integer part is 0, we can just use the fractional part.
if (integerPart.signum() == 0) {
significand = fractionalPart;
} else {
BigInteger integerFactor = computePowerOfTen(powersOfTen, integerExponent);
BigInteger integerFactor = computePowerOfTen(powersOfTen, fractionDigitsCount);
significand = FftMultiplier.multiply(integerPart, integerFactor).add(fractionalPart);
}
} else {
Expand Down
17 changes: 9 additions & 8 deletions ...fer.fastdoubleparser/ch/randelshofer/fastdoubleparser/JavaBigDecimalFromCharSequence.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ public JavaBigDecimalFromCharSequence() {
*/
public BigDecimal parseBigDecimalString(CharSequence str, int offset, int length) {
try {
int size = str.length();
final int endIndex = checkBounds(size, offset, length);
final int endIndex = checkBounds(str.length(), offset, length);
if (hasManyDigits(length)) {
return parseBigDecimalStringWithManyDigits(str, offset, length);
}
Expand Down Expand Up @@ -137,7 +136,6 @@ public BigDecimal parseBigDecimalString(CharSequence str, int offset, int length
nfe.initCause(e);
throw nfe;
}

}

/**
Expand All @@ -146,8 +144,8 @@ public BigDecimal parseBigDecimalString(CharSequence str, int offset, int length
BigDecimal parseBigDecimalStringWithManyDigits(CharSequence str, int offset, int length) {
final int integerPartIndex;
final int nonZeroIntegerPartIndex;
int decimalPointIndex = -1;
int nonZeroFractionalPartIndex = -1;
int decimalPointIndex = -1;
final int exponentIndicatorIndex;

final int endIndex = offset + length;
Expand All @@ -169,15 +167,17 @@ BigDecimal parseBigDecimalStringWithManyDigits(CharSequence str, int offset, int
// -----------------
// skip leading zeroes
integerPartIndex = index;
while (index < endIndex - 8 && FastDoubleSwar.isEightZeroes(str, index)) {
// swarLimit: We can process blocks of eight chars with SWAR, we must process the remaining chars individually.
int swarLimit = Math.min(endIndex - 8, 1 << 30);
while (index < swarLimit && FastDoubleSwar.isEightZeroes(str, index)) {
index += 8;
}
while (index < endIndex && str.charAt(index) == '0') {
index++;
}
// Count digits of integer part
nonZeroIntegerPartIndex = index;
while (index < endIndex - 8 && FastDoubleSwar.isEightDigits(str, index)) {
while (index < swarLimit && FastDoubleSwar.isEightDigits(str, index)) {
index += 8;
}
while (index < endIndex && FastDoubleSwar.isDigit(ch = str.charAt(index))) {
Expand All @@ -186,15 +186,15 @@ BigDecimal parseBigDecimalStringWithManyDigits(CharSequence str, int offset, int
if (ch == '.') {
decimalPointIndex = index++;
// skip leading zeroes
while (index < endIndex - 8 && FastDoubleSwar.isEightZeroes(str, index)) {
while (index < swarLimit && FastDoubleSwar.isEightZeroes(str, index)) {
index += 8;
}
while (index < endIndex && str.charAt(index) == '0') {
index++;
}
nonZeroFractionalPartIndex = index;
// Count digits of fraction part
while (index < endIndex - 8 && FastDoubleSwar.isEightDigits(str, index)) {
while (index < swarLimit && FastDoubleSwar.isEightDigits(str, index)) {
index += 8;
}
while (index < endIndex && FastDoubleSwar.isDigit(ch = str.charAt(index))) {
Expand Down Expand Up @@ -312,6 +312,7 @@ BigDecimal valueOfBigDecimalString(CharSequence str, int integerPartIndex, int d
} else {
fractionalPart = ParseDigitsTaskCharSequence.parseDigitsIterative(str, nonZeroFractionalPartIndex, exponentIndicatorIndex);
}
// If the integer part is 0, we can just use the fractional part.
if (integerPart.signum() == 0) {
significand = fractionalPart;
} else {
Expand Down
19 changes: 11 additions & 8 deletions ...hofer.fastdoubleparser/ch/randelshofer/fastdoubleparser/AbstractBigDecimalParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,15 @@ protected List<NumberTestDataSupplier> createDataForLegalDecStrings() {
new NumberTestDataSupplier("min exponent", () -> new NumberTestData("1e" + (Integer.MIN_VALUE + 1), BigDecimal.ONE.scaleByPowerOfTen(Integer.MIN_VALUE + 1))),
new NumberTestDataSupplier("max exponent", () -> new NumberTestData("1e" + Integer.MAX_VALUE, BigDecimal.ONE.scaleByPowerOfTen(Integer.MAX_VALUE))),

new NumberTestDataSupplier("8.99...99e68", () -> new NumberTestData("8." + (repeat("9", 19)) + "e68", new BigDecimal("8." + (repeat("9", 19)) + "e68"))),
new NumberTestDataSupplier("8.99...99e68", () -> new NumberTestData("8." + (repeat('9', 19)) + "e68", new BigDecimal("8." + (repeat('9', 19)) + "e68"))),
new NumberTestDataSupplier("103203303403503603703803903.122232425262728292", () -> new NumberTestData("103203303403503603703803903.122232425262728292", new BigDecimal("103203303403503603703803903.122232425262728292"))),
new NumberTestDataSupplier("122232425262728292.103203303403503603703803903", () -> new NumberTestData("122232425262728292.103203303403503603703803903", new BigDecimal("122232425262728292.103203303403503603703803903"))),
new NumberTestDataSupplier("-103203303403503603703803903.122232425262728292e6789", () -> new NumberTestData("-103203303403503603703803903.122232425262728292e6789", new BigDecimal("-103203303403503603703803903.122232425262728292e6789"))),
new NumberTestDataSupplier("122232425262728292.103203303403503603703803903e-6789", () -> new NumberTestData("122232425262728292.103203303403503603703803903e-6789", new BigDecimal("122232425262728292.103203303403503603703803903e-6789"))),
new NumberTestDataSupplier("-122232425262728292.103203303403503603703803903e-6789", () -> new NumberTestData("-122232425262728292.103203303403503603703803903e-6789", new BigDecimal("-122232425262728292.103203303403503603703803903e-6789")))
new NumberTestDataSupplier("-122232425262728292.103203303403503603703803903e-6789", () -> new NumberTestData("-122232425262728292.103203303403503603703803903e-6789", new BigDecimal("-122232425262728292.103203303403503603703803903e-6789"))),
new NumberTestDataSupplier("-11000.0..0(652 fractional digits)",
() -> new NumberTestData("-11000." + repeat('0', 652),
new BigDecimal("-11000." + repeat('0', 652))))
);
}

Expand Down Expand Up @@ -239,16 +242,16 @@ protected List<NumberTestDataSupplier> createTestDataForInputClassesInMethodPars
*/
protected List<NumberTestDataSupplier> createTestDataForInputClassesInMethodParseBigDecimalStringWithManyDigits() {
return Arrays.asList(
new NumberTestDataSupplier("illegal only negative sign", () -> new NumberTestData("-" + repeat("\000", 32), AbstractNumberParser.SYNTAX_ERROR, NumberFormatException.class)),
new NumberTestDataSupplier("illegal only positive sign", () -> new NumberTestData("+" + repeat("\000", 32), AbstractNumberParser.SYNTAX_ERROR, NumberFormatException.class)),
new NumberTestDataSupplier("illegal only negative sign", () -> new NumberTestData("-" + repeat('\000', 32), AbstractNumberParser.SYNTAX_ERROR, NumberFormatException.class)),
new NumberTestDataSupplier("illegal only positive sign", () -> new NumberTestData("+" + repeat('\000', 32), AbstractNumberParser.SYNTAX_ERROR, NumberFormatException.class)),


new NumberTestDataSupplier("significand with 40 zeroes in integer part", () -> new NumberTestData("significand with 40 zeroes in integer part", repeat("0", 40), BigDecimal::new)),
new NumberTestDataSupplier("significand with 40 zeroes in fraction part", () -> new NumberTestData("." + repeat("0", 40), BigDecimal::new)),
new NumberTestDataSupplier("significand with 40 zeroes in fraction part", () -> new NumberTestData("." + repeat('0', 40), BigDecimal::new)),

new NumberTestDataSupplier("significand with 10 leading zeros and 30 digits in integer part", () -> new NumberTestData("significand with 10 leading zeros and 30 digits in integer part", repeat("0", 10) + repeat("9", 30), BigDecimal::new)),
new NumberTestDataSupplier("significand with 10 leading zeros and 30 digits in fraction part", () -> new NumberTestData("." + repeat("0", 10) + repeat("9", 30), BigDecimal::new)),
new NumberTestDataSupplier("significand with 10 leading zeros and 30 digits in integer part and in fraction part", () -> new NumberTestData("significand with 10 leading zeros and 30 digits in integer part and in fraction part", repeat("0", 10) + repeat("9", 30) + "." + repeat("0", 10) + repeat("9", 30), BigDecimal::new)),
new NumberTestDataSupplier("significand with 10 leading zeros and 30 digits in integer part", () -> new NumberTestData("significand with 10 leading zeros and 30 digits in integer part", repeat('0', 10) + repeat('9', 30), BigDecimal::new)),
new NumberTestDataSupplier("significand with 10 leading zeros and 30 digits in fraction part", () -> new NumberTestData("." + repeat('0', 10) + repeat('9', 30), BigDecimal::new)),
new NumberTestDataSupplier("significand with 10 leading zeros and 30 digits in integer part and in fraction part", () -> new NumberTestData("significand with 10 leading zeros and 30 digits in integer part and in fraction part", repeat('0', 10) + repeat("9", 30) + "." + repeat("0", 10) + repeat("9", 30), BigDecimal::new)),

new NumberTestDataSupplier("significand with 40 digits in integer part and exponent", () -> new NumberTestData("-1234567890123456789012345678901234567890e887799", BigDecimal::new)),
new NumberTestDataSupplier("no significand but exponent 40 digits", () -> new NumberTestData("-e12345678901234567890123456789012345678901234567890", AbstractNumberParser.SYNTAX_ERROR, NumberFormatException.class)),
Expand Down
24 changes: 12 additions & 12 deletions .../ch.randelshofer.fastdoubleparser/ch/randelshofer/fastdoubleparser/FftMultiplierTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,17 @@ public List<DynamicTest> dynamicTestsMultiply() {
"2147483647",
"9223372036854775807")),
dynamicTest("'3','0'**84 * '4','0'**84", () -> shouldMultiplyFft(
"3" + repeat("0", 84),
"4" + repeat("0", 84))),
"3" + repeat('0', 84),
"4" + repeat('0', 84))),
dynamicTest("'-','3','0'**84 * '4','0'**84", () -> shouldMultiplyFft(
"-3" + repeat("0", 84),
"4" + repeat("0", 84))),
"-3" + repeat('0', 84),
"4" + repeat('0', 84))),
dynamicTest("'-','3','0'**84 * '-','4','0'**84", () -> shouldMultiplyFft(
"-3" + repeat("0", 84),
"-4" + repeat("0", 84))),
"-3" + repeat('0', 84),
"-4" + repeat('0', 84))),
dynamicTest("'3','0'**100_000 * '4','0'**100_000", () -> shouldMultiplyFft(
"3" + repeat("0", 100_000),
"4" + repeat("0", 100_000)))
"3" + repeat('0', 100_000),
"4" + repeat('0', 100_000)))
);
}

Expand Down Expand Up @@ -140,16 +140,16 @@ private void shouldMultiplyFft(BigInteger a, BigInteger b, BigInteger expected)
public List<DynamicTest> dynamicTestsSquare() {
return Arrays.asList(
dynamicTest("'3','0'**84", () -> shouldSquare(
"3" + repeat("0", 84)
"3" + repeat('0', 84)
)),
dynamicTest("'-','3','0'**84", () -> shouldSquare(
"-3" + repeat("0", 84)
"-3" + repeat('0', 84)
)),
dynamicTest("'-','3','0'**84", () -> shouldSquare(
"-3" + repeat("0", 84)
"-3" + repeat('0', 84)
)),
dynamicTest("'3','0'**100_000", () -> shouldSquare(
"3" + repeat("0", 100_000)
"3" + repeat('0', 100_000)
))
);

Expand Down
Loading

0 comments on commit e388c78

Please sign in to comment.