From d19442399c004c78bff8a5ccf7c6975c7e583a07 Mon Sep 17 00:00:00 2001 From: Johannes Graham Date: Thu, 31 Jul 2025 17:50:18 +0000 Subject: [PATCH] 8358880: Performance of parsing with DecimalFormat can be improved Reviewed-by: jlu, liach, rgiulietti --- .../share/classes/java/text/DigitList.java | 71 ++++----------- .../jdk/internal/math/FloatingDecimal.java | 27 ++++++ .../text/Format/DecimalFormat/CloneTest.java | 7 +- .../java/text/DecimalFormatParseBench.java | 89 +++++++++++++++++++ 4 files changed, 136 insertions(+), 58 deletions(-) create mode 100644 test/micro/org/openjdk/bench/java/text/DecimalFormatParseBench.java diff --git a/src/java.base/share/classes/java/text/DigitList.java b/src/java.base/share/classes/java/text/DigitList.java index d757f03bb84..3eeabb7e77e 100644 --- a/src/java.base/share/classes/java/text/DigitList.java +++ b/src/java.base/share/classes/java/text/DigitList.java @@ -169,13 +169,7 @@ final class DigitList implements Cloneable { if (count == 0) { return 0.0; } - - return Double.parseDouble(getStringBuilder() - .append('.') - .append(digits, 0, count) - .append('E') - .append(decimalAt) - .toString()); + return FloatingDecimal.parseDoubleSignlessDigits(decimalAt, digits, count); } /** @@ -190,17 +184,22 @@ final class DigitList implements Cloneable { return 0; } - // We have to check for this, because this is the one NEGATIVE value + // Parse as unsigned to handle Long.MIN_VALUE, which is the one NEGATIVE value // we represent. If we tried to just pass the digits off to parseLong, // we'd get a parse failure. - if (isLongMIN_VALUE()) { - return Long.MIN_VALUE; + long v = Long.parseUnsignedLong(new String(digits, 0, count)); + if (v < 0) { + if (v == Long.MIN_VALUE) { + return Long.MIN_VALUE; + } + throw new NumberFormatException("Unexpected negative value"); + } + try { + long pow10 = Math.powExact(10L, Math.max(0, decimalAt - count)); + return Math.multiplyExact(v, pow10); + } catch (ArithmeticException e) { + throw new NumberFormatException("Value does not fit into a long"); } - - StringBuilder temp = getStringBuilder(); - temp.append(digits, 0, count); - temp.append("0".repeat(Math.max(0, decimalAt - count))); - return Long.parseLong(temp.toString()); } /** @@ -210,11 +209,7 @@ final class DigitList implements Cloneable { */ public final BigDecimal getBigDecimal() { if (count == 0) { - if (decimalAt == 0) { - return BigDecimal.ZERO; - } else { - return new BigDecimal("0E" + decimalAt); - } + return BigDecimal.valueOf(0, -decimalAt); } if (decimalAt == count) { @@ -726,11 +721,10 @@ final class DigitList implements Cloneable { System.arraycopy(digits, 0, newDigits, 0, digits.length); other.digits = newDigits; - // data and tempBuilder do not need to be copied because they do - // not carry significant information. They will be recreated on demand. - // Setting them to null is needed to avoid sharing across clones. + // Data does not need to be copied because it does + // not carry significant information. It will be recreated on demand. + // Setting it to null is needed to avoid sharing across clones. other.data = null; - other.tempBuilder = null; return other; } catch (CloneNotSupportedException e) { @@ -738,23 +732,7 @@ final class DigitList implements Cloneable { } } - /** - * Returns true if this DigitList represents Long.MIN_VALUE; - * false, otherwise. This is required so that getLong() works. - */ - private boolean isLongMIN_VALUE() { - if (decimalAt != count || count != MAX_COUNT) { - return false; - } - - for (int i = 0; i < count; ++i) { - if (digits[i] != LONG_MIN_REP[i]) return false; - } - - return true; - } - - private static final int parseInt(char[] str, int offset, int strLen) { + private static int parseInt(char[] str, int offset, int strLen) { char c; boolean positive = true; if ((c = str[offset]) == '-') { @@ -787,17 +765,6 @@ final class DigitList implements Cloneable { return "0." + new String(digits, 0, count) + "x10^" + decimalAt; } - private StringBuilder tempBuilder; - - private StringBuilder getStringBuilder() { - if (tempBuilder == null) { - tempBuilder = new StringBuilder(MAX_COUNT); - } else { - tempBuilder.setLength(0); - } - return tempBuilder; - } - private void extendDigits(int len) { if (len > digits.length) { digits = new char[len]; diff --git a/src/java.base/share/classes/jdk/internal/math/FloatingDecimal.java b/src/java.base/share/classes/jdk/internal/math/FloatingDecimal.java index 81887144ed1..cc68ce1282b 100644 --- a/src/java.base/share/classes/jdk/internal/math/FloatingDecimal.java +++ b/src/java.base/share/classes/jdk/internal/math/FloatingDecimal.java @@ -122,6 +122,19 @@ public class FloatingDecimal{ return readJavaFormatString(s, BINARY_32_IX).floatValue(); } + /** + * Converts a sequence of digits ('0'-'9') as well as an exponent to a positive + * double value + * + * @param decExp The decimal exponent of the value to generate + * @param digits The digits of the significand. + * @param length Number of digits to use + * @return The double-precision value of the conversion + */ + public static double parseDoubleSignlessDigits(int decExp, char[] digits, int length) { + return readDoubleSignlessDigits(decExp, digits, length).doubleValue(); + } + /** * A converter which can process single or double precision floating point * values into an ASCII String representation. @@ -1824,6 +1837,20 @@ public class FloatingDecimal{ return buf; } + static ASCIIToBinaryConverter readDoubleSignlessDigits(int decExp, char[] digits, int length) { + + // Prevent an extreme negative exponent from causing overflow issues in doubleValue(). + // Large positive values are handled within doubleValue(); + if (decExp < MIN_DECIMAL_EXPONENT) { + return A2BC_POSITIVE_ZERO; + } + byte[] buf = new byte[length]; + for (int i = 0; i < length; i++) { + buf[i] = (byte) digits[i]; + } + return new ASCIIToBinaryBuffer(false, decExp, buf, length); + } + /** * The input must match the {@link Double#valueOf(String) rules described here}, * about leading and trailing whitespaces, and the grammar. diff --git a/test/jdk/java/text/Format/DecimalFormat/CloneTest.java b/test/jdk/java/text/Format/DecimalFormat/CloneTest.java index fa46b09d6ac..f11d576a39a 100644 --- a/test/jdk/java/text/Format/DecimalFormat/CloneTest.java +++ b/test/jdk/java/text/Format/DecimalFormat/CloneTest.java @@ -23,7 +23,7 @@ /* * @test - * @bug 8354522 + * @bug 8354522 8358880 * @summary Check for cloning interference * @library /test/lib * @run junit/othervm --add-opens=java.base/java.text=ALL-UNNAMED CloneTest @@ -95,11 +95,6 @@ public class CloneTest { assertNotSame(data, valFromDigitList(dfClone, "data")); } - Object tempBuilder = valFromDigitList(original, "tempBuilder"); - if (tempBuilder != null) { - assertNotSame(data, valFromDigitList(dfClone, "data")); - } - assertEquals(digitListField.get(original), digitListField.get(dfClone)); } catch (ReflectiveOperationException e) { throw new SkippedException("reflective access in white-box test failed", e); diff --git a/test/micro/org/openjdk/bench/java/text/DecimalFormatParseBench.java b/test/micro/org/openjdk/bench/java/text/DecimalFormatParseBench.java new file mode 100644 index 00000000000..f51591ee73f --- /dev/null +++ b/test/micro/org/openjdk/bench/java/text/DecimalFormatParseBench.java @@ -0,0 +1,89 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package org.openjdk.bench.java.text; + +import java.text.DecimalFormat; +import java.text.ParseException; +import java.util.concurrent.TimeUnit; + +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OperationsPerInvocation; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; +import org.openjdk.jmh.runner.Runner; +import org.openjdk.jmh.runner.options.Options; +import org.openjdk.jmh.runner.options.OptionsBuilder; + +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(TimeUnit.MILLISECONDS) +@Warmup(iterations = 5, time = 1) +@Measurement(iterations = 5, time = 1) +@Fork(3) +@State(Scope.Benchmark) +public class DecimalFormatParseBench { + + public String[] valuesLong; + public String[] valuesDouble; + + @Setup + public void setup() { + valuesLong = new String[]{ + "123", "149", "180", "170000000000000000", "0", "-149", "-15000", "99999123", "1494", "1495", "1030", "25996", "-25996" + }; + + valuesDouble = new String[]{ + "1.23", "1.49", "1.80", "17000000000000000.1", "0.01", "-1.49", "-1.50", "9999.9123", "1.494", "1.495", "1.03", "25.996", "-25.996" + }; + } + + private DecimalFormat dnf = new DecimalFormat(); + + @Benchmark + @OperationsPerInvocation(13) + public void testParseLongs(final Blackhole blackhole) throws ParseException { + for (String value : valuesLong) { + blackhole.consume(this.dnf.parse(value)); + } + } + + @Benchmark + @OperationsPerInvocation(13) + public void testParseDoubles(final Blackhole blackhole) throws ParseException { + for (String value : valuesDouble) { + blackhole.consume(this.dnf.parse(value)); + } + } + public static void main(String... args) throws Exception { + Options opts = new OptionsBuilder().include(DefFormatterBench.class.getSimpleName()).shouldDoGC(true).build(); + new Runner(opts).run(); + } + +}