From be53bb5a5dc9bcbd8c212d00db2f1feb14bf3797 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 30 Oct 2024 20:41:44 -0700 Subject: [PATCH] Fix #1328: optimize `JsonPointer.head()` (#1354) --- release-notes/VERSION-2.x | 2 +- .../fasterxml/jackson/core/JsonPointer.java | 62 +++++++++++++------ .../jackson/failing/JsonPointer1328Test.java | 19 +++++- 3 files changed, 61 insertions(+), 22 deletions(-) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 5c3f9fabe6..866a61d490 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -16,7 +16,7 @@ a pure JSON library. 2.19.0 (not yet released) -- +#1328: Optimize handling of `JsonPointer.head()` 2.18.1 (28-Oct-2024) diff --git a/src/main/java/com/fasterxml/jackson/core/JsonPointer.java b/src/main/java/com/fasterxml/jackson/core/JsonPointer.java index 51f1978a65..0b95237d4b 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonPointer.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonPointer.java @@ -1,6 +1,7 @@ package com.fasterxml.jackson.core; import java.io.*; +import java.util.ArrayList; import com.fasterxml.jackson.core.io.NumberInput; @@ -102,7 +103,7 @@ public class JsonPointer implements Serializable * We will retain representation of the pointer, as a String, * so that {@link #toString} should be as efficient as possible. *

- * NOTE: starting with 2.14, there is no accompanying + * NOTE: starting with 2.14, there is now accompanying * {@link #_asStringOffset} that MUST be considered with this String; * this {@code String} may contain preceding path, as it is now full path * of parent pointer, except for the outermost pointer instance. @@ -168,6 +169,24 @@ protected JsonPointer(String fullString, int fullStringOffset, _matchingElementIndex = matchIndex; } + // @since 2.19 + protected JsonPointer(JsonPointer src, JsonPointer next) { + _asString = src._asString; + _asStringOffset = src._asStringOffset; + _nextSegment = next; + _matchingPropertyName = src._matchingPropertyName; + _matchingElementIndex = src._matchingElementIndex; + } + + // @since 2.19 + protected JsonPointer(JsonPointer src, String newFullString, int newFullStringOffset) { + _asString = newFullString; + _asStringOffset = newFullStringOffset; + _nextSegment = null; + _matchingPropertyName = src._matchingPropertyName; + _matchingElementIndex = src._matchingElementIndex; + } + /* /********************************************************** /* Factory methods @@ -814,31 +833,38 @@ private static void _appendEscape(StringBuilder sb, char c) { protected JsonPointer _constructHead() { - // ok; find out who we are to drop + // ok; find out the segment we are to drop JsonPointer last = last(); if (last == this) { return EMPTY; } // and from that, length of suffix to drop - int suffixLength = last.length(); - JsonPointer next = _nextSegment; + final int suffixLength = last.length(); + + // Initialize a list to store intermediate JsonPointers in reverse + ArrayList pointers = new ArrayList<>(); + + JsonPointer current = this; String fullString = toString(); - return new JsonPointer(fullString.substring(0, fullString.length() - suffixLength), 0, - _matchingPropertyName, - _matchingElementIndex, next._constructHead(suffixLength, last)); - } + // Make sure to share the new full string for path segments + fullString = fullString.substring(0, fullString.length() - suffixLength); + int offset = 0; - protected JsonPointer _constructHead(int suffixLength, JsonPointer last) - { - if (this == last) { - return EMPTY; + while (current != last) { + JsonPointer nextSegment = new JsonPointer(current, + fullString, offset); + offset += suffixLength; + pointers.add(nextSegment); + current = current._nextSegment; } - JsonPointer next = _nextSegment; - String str = toString(); - // !!! TODO 07-Oct-2022, tatu: change to iterative, not recursive - return new JsonPointer(str.substring(0, str.length() - suffixLength), 0, - _matchingPropertyName, - _matchingElementIndex, next._constructHead(suffixLength, last)); + + // Iteratively build the JsonPointer chain from the list in reverse + JsonPointer head = EMPTY; + for (int i = pointers.size() - 1; i >= 0; i--) { + head = new JsonPointer(pointers.get(i), head); + } + + return head; } /* diff --git a/src/test/java/com/fasterxml/jackson/failing/JsonPointer1328Test.java b/src/test/java/com/fasterxml/jackson/failing/JsonPointer1328Test.java index 33cc5721b4..035e09fe04 100644 --- a/src/test/java/com/fasterxml/jackson/failing/JsonPointer1328Test.java +++ b/src/test/java/com/fasterxml/jackson/failing/JsonPointer1328Test.java @@ -6,6 +6,7 @@ import com.fasterxml.jackson.core.JsonPointer; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.assertj.core.api.Assertions.assertThat; class JsonPointer1328Test extends JUnit5TestBase { @@ -17,14 +18,26 @@ class JsonPointer1328Test extends JUnit5TestBase void deepHead() { final String INPUT = repeat("/a", DEPTH); - JsonPointer ptr = JsonPointer.compile(INPUT); - assertEquals(repeat("/a", DEPTH - 1), ptr.head().toString()); + JsonPointer origPtr = JsonPointer.compile(INPUT); + JsonPointer head = origPtr.head(); + final String fullHead = head.toString(); + assertEquals(repeat("/a", DEPTH - 1), fullHead); + + // But let's also traverse the hierarchy to make sure: + for (JsonPointer ctr = head; ctr != JsonPointer.empty(); ctr = ctr.tail()) { + assertThat(fullHead).endsWith(ctr.toString()); + } } private final static String repeat(String part, int count) { StringBuilder sb = new StringBuilder(count * part.length()); + int index = 0; while (--count >= 0) { - sb.append(part); + // Add variation so we'll have "a0" .. "a8"; this to try + // to avoid possibility of accidentally "working" with + // super-repetitive paths + sb.append(part).append(index % 9); + ++index; } return sb.toString(); }