Skip to content

Further reduce memory usage of JsonPointer (wrt #818) #820

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 7, 2022
Merged

Further reduce memory usage of JsonPointer (wrt #818) #820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c77299f
Further work on #818 to reduce memory usage
cowtowncoder Oct 7, 2022
7d2b2d6
Change code to avoid eagerly constructing N Strings for N-segment Jso…
cowtowncoder Oct 7, 2022
47a1e85
add todos for further work
cowtowncoder Oct 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 46 additions & 24 deletions src/main/java/com/fasterxml/jackson/core/JsonPointer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,17 @@ 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.
*<p>
* NOTE: starting with 2.14, there is no accompanying
* {@link #_asStringOffset} that MUST be considered with this String;
*/
protected final String _asString;

/**
* @since 2.14
*/
protected final int _asStringOffset;

protected final String _matchingPropertyName;

protected final int _matchingElementIndex;
Expand All @@ -88,21 +96,26 @@ protected JsonPointer() {
_matchingPropertyName = null;
_matchingElementIndex = -1;
_asString = "";
_asStringOffset = 0;
}

// Constructor used for creating non-empty Segments
protected JsonPointer(String fullString, String segment, JsonPointer next) {
protected JsonPointer(String fullString, int fullStringOffset,
String segment, JsonPointer next)
{
_asString = fullString;
_asStringOffset = fullStringOffset;
_nextSegment = next;
// Ok; may always be a property
_matchingPropertyName = segment;
// but could be an index, if parsable
_matchingElementIndex = _parseIndex(segment);
}

// @since 2.5
protected JsonPointer(String fullString, String segment, int matchIndex, JsonPointer next) {
protected JsonPointer(String fullString, int fullStringOffset,
String segment, int matchIndex, JsonPointer next) {
_asString = fullString;
_asStringOffset = fullStringOffset;
_nextSegment = next;
_matchingPropertyName = segment;
_matchingElementIndex = matchIndex;
Expand Down Expand Up @@ -199,11 +212,11 @@ public static JsonPointer forPath(JsonStreamContext context,
if (seg == null) { // is this legal?
seg = "";
}
tail = new JsonPointer(_fullPath(tail, seg), seg, tail);
tail = new JsonPointer(_fullPath(tail, seg), 0, seg, tail);
} else if (context.inArray() || includeRoot) {
int ix = context.getCurrentIndex();
String ixStr = String.valueOf(ix);
tail = new JsonPointer(_fullPath(tail, ixStr), ixStr, ix, tail);
tail = new JsonPointer(_fullPath(tail, ixStr), 0, ixStr, ix, tail);
}
// NOTE: this effectively drops ROOT node(s); should have 1 such node,
// as the last one, but we don't have to care (probably some paths have
Expand Down Expand Up @@ -517,7 +530,13 @@ public JsonPointer head() {
/**********************************************************
*/

@Override public String toString() { return _asString; }
@Override public String toString() {
if (_asStringOffset <= 0) {
return _asString;
}
return _asString.substring(_asStringOffset);
}

@Override public int hashCode() { return _asString.hashCode(); }

@Override public boolean equals(Object o) {
Expand Down Expand Up @@ -563,21 +582,21 @@ private final static int _parseIndex(String str) {
return NumberInput.parseInt(str);
}

protected static JsonPointer _parseTail(String fullPath)
protected static JsonPointer _parseTail(final String fullPath)
{
PointerParent parent = null;

// first char is the contextual slash, skip
int i = 1;
int end = fullPath.length();
final int end = fullPath.length();
int startOffset = 0;

while (i < end) {
char c = fullPath.charAt(i);
if (c == '/') { // common case, got a segment
parent = new PointerParent(parent, fullPath, fullPath.substring(1, i));
fullPath = fullPath.substring(i);
i = 1;
end = fullPath.length();
parent = new PointerParent(parent, startOffset, fullPath.substring(startOffset + 1, i));
startOffset = i;
++i;
continue;
}
++i;
Expand All @@ -591,10 +610,9 @@ protected static JsonPointer _parseTail(String fullPath)
if (i < 0) { // end!
return _buildPath(fullPath, segment, parent);
}
parent = new PointerParent(parent, fullPath, segment);
fullPath = fullPath.substring(i);
i = 1;
end = fullPath.length();
parent = new PointerParent(parent, startOffset, segment);
startOffset = i;
++i;
continue;
}
// otherwise, loop on
Expand All @@ -603,11 +621,11 @@ protected static JsonPointer _parseTail(String fullPath)
return _buildPath(fullPath, fullPath.substring(1), parent);
}

private static JsonPointer _buildPath(String fullPath, String segment,
private static JsonPointer _buildPath(final String fullPath, String segment,
PointerParent parent) {
JsonPointer curr = new JsonPointer(fullPath, segment, EMPTY);
JsonPointer curr = new JsonPointer(fullPath, 0, segment, EMPTY);
for (; parent != null; parent = parent.parent) {
curr = new JsonPointer(parent.fullPath, parent.segment, curr);
curr = new JsonPointer(fullPath, parent.fullPathOffset, parent.segment, curr);
}
return curr;
}
Expand Down Expand Up @@ -669,7 +687,9 @@ protected JsonPointer _constructHead()
// and from that, length of suffix to drop
int suffixLength = last._asString.length();
JsonPointer next = _nextSegment;
return new JsonPointer(_asString.substring(0, _asString.length() - suffixLength), _matchingPropertyName,
// !!! TODO 07-Oct-2022, tatu: change to iterative, not recursive
return new JsonPointer(_asString.substring(0, _asString.length() - suffixLength), 0,
_matchingPropertyName,
_matchingElementIndex, next._constructHead(suffixLength, last));
}

Expand All @@ -680,7 +700,9 @@ protected JsonPointer _constructHead(int suffixLength, JsonPointer last)
}
JsonPointer next = _nextSegment;
String str = _asString;
return new JsonPointer(str.substring(0, str.length() - suffixLength), _matchingPropertyName,
// !!! 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));
}

Expand All @@ -696,12 +718,12 @@ protected JsonPointer _constructHead(int suffixLength, JsonPointer last)
*/
private static class PointerParent {
public final PointerParent parent;
public final String fullPath;
public final int fullPathOffset;
public final String segment;

PointerParent(PointerParent pp, String fp, String sgm) {
PointerParent(PointerParent pp, int fpo, String sgm) {
parent = pp;
fullPath = fp;
fullPathOffset = fpo;
segment = sgm;
}
}
Expand Down
17 changes: 15 additions & 2 deletions src/test/java/com/fasterxml/jackson/core/fuzz/Fuzz51806JsonPointerParse818Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
// (reported as [core#818]
public class Fuzz51806JsonPointerParse818Test extends BaseTest
{
// Before fix, looks like this is enough to cause StackOverflowError
private final static int TOO_DEEP_PATH = 10_000;
// Before fix, StackOverflowError with 6_000 or so,
// and OOME with 20_000.
// After fixes will get progressively slower so limit size to
// keep test runs from making suite too slow
private final static int TOO_DEEP_PATH = 25_000;

// Verify that a very deep/long (by number of segments) JsonPointer
// may still be parsed ok, for "simple" case (no quoted chars)
Expand All @@ -28,6 +31,16 @@ private void _testJsonPointer(String pathExpr)
assertNotNull(p);
// But also verify it didn't change
assertEquals(pathExpr, p.toString());

// And then verify segment by segment, easiest way is to
// check that tail segment is proper substring
JsonPointer curr = p;

while ((curr = curr.tail()) != null) {
String act = curr.toString();
String exp = pathExpr.substring(pathExpr.length() - act.length());
assertEquals(exp, act);
}
}

private String _generatePath(int depth, boolean escaped) {
Expand Down