From eb85a336b1baa50b40f7758a24f0e93d814e70a0 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sat, 1 Apr 2023 13:56:32 -0700 Subject: [PATCH 1/3] Add new failing tests to help investigation --- .../failing/PerfBigDecimalParser967.java | 37 +++++++++++++++++++ .../failing/PerfBigDecimalToInteger968.java | 30 +++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalParser967.java create mode 100644 src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalToInteger968.java diff --git a/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalParser967.java b/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalParser967.java new file mode 100644 index 0000000000..d57cdc0b7c --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalParser967.java @@ -0,0 +1,37 @@ +package com.fasterxml.jackson.failing; + +import org.junit.Assert; +import org.junit.Test; + +import com.fasterxml.jackson.core.*; + +// For [core#967] +public class PerfBigDecimalParser967 +{ + private final JsonFactory JSON_F = new JsonFactory(); + + // For [core#967]: shouldn't take multiple seconds + @Test(timeout = 35000) + public void bigDecimalFromString() throws Exception { + // Jackson's BigDecimalParser seems to be slower than JDK's; + // won't fail if using latter. + StringBuilder sb = new StringBuilder(900); + for (int i = 0; i < 500; ++i) { + sb.append('1'); + } + sb.append("1e10000000"); + final String DOC = sb.toString(); + + try (JsonParser p = JSON_F.createParser(DOC)) { + assertToken(JsonToken.VALUE_NUMBER_FLOAT, p.nextToken()); + Assert.assertNotNull(p.getDecimalValue()); + } + } + + protected void assertToken(JsonToken expToken, JsonToken actToken) + { + if (actToken != expToken) { + Assert.fail("Expected token "+expToken+", current token "+actToken); + } + } +} diff --git a/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalToInteger968.java b/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalToInteger968.java new file mode 100644 index 0000000000..1c4bda79bf --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalToInteger968.java @@ -0,0 +1,30 @@ +package com.fasterxml.jackson.failing; + +import org.junit.Assert; +import org.junit.Test; + +import com.fasterxml.jackson.core.*; + +// For [core#968]] +public class PerfBigDecimalToInteger968 +{ + private final JsonFactory JSON_F = new JsonFactory(); + + // For [core#968]]: shouldn't take multiple seconds + @Test(timeout = 3000) + public void bigIntegerViaBigDecimal() throws Exception { + final String DOC = "1e20000000"; + + try (JsonParser p = JSON_F.createParser(DOC)) { + assertToken(JsonToken.VALUE_NUMBER_FLOAT, p.nextToken()); + Assert.assertNotNull(p.getBigIntegerValue()); + } + } + + protected void assertToken(JsonToken expToken, JsonToken actToken) + { + if (actToken != expToken) { + Assert.fail("Expected token "+expToken+", current token "+actToken); + } + } +} From 079a81b7a38a5813c3119bbb6bf4c12c7dbc90d1 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 4 Apr 2023 15:39:17 -0700 Subject: [PATCH 2/3] Fix #967 (#973): BigDecimalParser performance for edge cases --- release-notes/VERSION-2.x | 1 + .../com/fasterxml/jackson/core/io/BigDecimalParser.java | 9 ++++++++- .../jackson/failing/PerfBigDecimalParser967.java | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 8d8cc04706..1961f3e0ad 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -51,6 +51,7 @@ JSON library. #912: Optional padding Base64Variant still throws exception on missing padding character (reported by @Vity01) +#967: Address performance issue with `BigDecimalParser` 2.14.2 (28-Jan-2023) diff --git a/src/main/java/com/fasterxml/jackson/core/io/BigDecimalParser.java b/src/main/java/com/fasterxml/jackson/core/io/BigDecimalParser.java index ff3043431c..0e42d163e2 100644 --- a/src/main/java/com/fasterxml/jackson/core/io/BigDecimalParser.java +++ b/src/main/java/com/fasterxml/jackson/core/io/BigDecimalParser.java @@ -202,6 +202,13 @@ private static BigDecimal toBigDecimalRec(final char[] chars, final int off, fin return left.add(right); } - return len == 0 ? BigDecimal.ZERO : new BigDecimal(chars, off, len).movePointRight(scale); + if (len == 0) { + return BigDecimal.ZERO; + } + // 02-Apr-2023, tatu: [core#967] Looks like "scaleByPowerOfThen" avoids performance issue + // there would be with "movePointRight" (both doing about same thing), so) + return new BigDecimal(chars, off, len) +// .movePointRight(scale); + .scaleByPowerOfTen(scale); } } diff --git a/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalParser967.java b/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalParser967.java index d57cdc0b7c..42e7f6d8fd 100644 --- a/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalParser967.java +++ b/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalParser967.java @@ -11,7 +11,7 @@ public class PerfBigDecimalParser967 private final JsonFactory JSON_F = new JsonFactory(); // For [core#967]: shouldn't take multiple seconds - @Test(timeout = 35000) + @Test(timeout = 3000) public void bigDecimalFromString() throws Exception { // Jackson's BigDecimalParser seems to be slower than JDK's; // won't fail if using latter. From d2608589985d29dbf40af3fa984acce873702162 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 4 Apr 2023 15:39:17 -0700 Subject: [PATCH 3/3] Fix #967 (#973): BigDecimalParser performance for edge cases --- release-notes/VERSION-2.x | 1 + .../com/fasterxml/jackson/core/io/BigDecimalParser.java | 9 ++++++++- .../jackson/failing/PerfBigDecimalParser967.java | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 99e3bc448a..939ac3ddbb 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -25,6 +25,7 @@ No changes since 2.14 #912: Optional padding Base64Variant still throws exception on missing padding character (reported by @Vity01) +#967: Address performance issue with `BigDecimalParser` 2.14.2 (28-Jan-2023) diff --git a/src/main/java/com/fasterxml/jackson/core/io/BigDecimalParser.java b/src/main/java/com/fasterxml/jackson/core/io/BigDecimalParser.java index 173af8300e..1afba6d0d6 100644 --- a/src/main/java/com/fasterxml/jackson/core/io/BigDecimalParser.java +++ b/src/main/java/com/fasterxml/jackson/core/io/BigDecimalParser.java @@ -178,6 +178,13 @@ private static BigDecimal toBigDecimalRec(final char[] chars, final int off, fin return left.add(right); } - return len == 0 ? BigDecimal.ZERO : new BigDecimal(chars, off, len).movePointRight(scale); + if (len == 0) { + return BigDecimal.ZERO; + } + // 02-Apr-2023, tatu: [core#967] Looks like "scaleByPowerOfThen" avoids performance issue + // there would be with "movePointRight" (both doing about same thing), so) + return new BigDecimal(chars, off, len) +// .movePointRight(scale); + .scaleByPowerOfTen(scale); } } diff --git a/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalParser967.java b/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalParser967.java index d57cdc0b7c..42e7f6d8fd 100644 --- a/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalParser967.java +++ b/src/test/java/com/fasterxml/jackson/failing/PerfBigDecimalParser967.java @@ -11,7 +11,7 @@ public class PerfBigDecimalParser967 private final JsonFactory JSON_F = new JsonFactory(); // For [core#967]: shouldn't take multiple seconds - @Test(timeout = 35000) + @Test(timeout = 3000) public void bigDecimalFromString() throws Exception { // Jackson's BigDecimalParser seems to be slower than JDK's; // won't fail if using latter.