From e41f969ead5cf6e0105d975e1a7df20f37c109ce Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Tue, 13 Feb 2024 13:18:13 +0100 Subject: [PATCH] ensure all buffers get released again in JsonStreamParserImpl --- .../johnzon/core/JsonStreamParserImpl.java | 9 ++- .../apache/johnzon/core/HugeStringTest.java | 47 ------------ .../core/JsonStreamParserImplTest.java | 74 +++++++++++++++++++ 3 files changed, 81 insertions(+), 49 deletions(-) delete mode 100644 johnzon-core/src/test/java/org/apache/johnzon/core/HugeStringTest.java diff --git a/johnzon-core/src/main/java/org/apache/johnzon/core/JsonStreamParserImpl.java b/johnzon-core/src/main/java/org/apache/johnzon/core/JsonStreamParserImpl.java index dede892d..b4eb5542 100644 --- a/johnzon-core/src/main/java/org/apache/johnzon/core/JsonStreamParserImpl.java +++ b/johnzon-core/src/main/java/org/apache/johnzon/core/JsonStreamParserImpl.java @@ -74,7 +74,7 @@ public class JsonStreamParserImpl extends JohnzonJsonParserImpl implements JsonC //this buffer is used to store current String or Number value in case that //within the value a buffer boundary is crossed or the string contains escaped characters private char[] fallBackCopyBuffer; - private boolean releaseFallBackCopyBufferLength = true; + private boolean releaseFallBackCopyBuffer = true; private int fallBackCopyBufferLength; // when boundaries of fallBackCopyBuffer have been reached private List previousFallBackCopyBuffers; @@ -942,6 +942,11 @@ private void combinePreviousFallbackBuffersToCurrent() { index += fallBackCopyBufferLength; releasePreviousFallBackCopyBuffers(); + if (releaseFallBackCopyBuffer) { + valueProvider.release(fallBackCopyBuffer); + releaseFallBackCopyBuffer = false; + } + fallBackCopyBuffer = newBuffer; fallBackCopyBufferLength = index; } @@ -1044,7 +1049,7 @@ public void close() { } bufferProvider.release(buffer); - if (releaseFallBackCopyBufferLength) { + if (releaseFallBackCopyBuffer) { valueProvider.release(fallBackCopyBuffer); } releasePreviousFallBackCopyBuffers(); diff --git a/johnzon-core/src/test/java/org/apache/johnzon/core/HugeStringTest.java b/johnzon-core/src/test/java/org/apache/johnzon/core/HugeStringTest.java deleted file mode 100644 index 72ce072a..00000000 --- a/johnzon-core/src/test/java/org/apache/johnzon/core/HugeStringTest.java +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.johnzon.core; - -import jakarta.json.Json; -import jakarta.json.JsonReader; -import org.junit.Ignore; -import org.junit.Test; - -import java.io.StringReader; - -@Ignore -public class HugeStringTest { - @Test - public void test() { - String json = "{\"data\":\"" + "a".repeat(50 * 1024 * 1024 + 1) + "\"}"; - - // Warmup - for (int i = 0; i < 10; i++) { - try (JsonReader reader = Json.createReader(new StringReader(json))) { - reader.readObject(); - } - } - - long start = System.currentTimeMillis(); - try (JsonReader reader = Json.createReader(new StringReader(json))) { - reader.readObject(); - } - System.err.println("Took " + (System.currentTimeMillis() - start) + "ms"); - } -} diff --git a/johnzon-core/src/test/java/org/apache/johnzon/core/JsonStreamParserImplTest.java b/johnzon-core/src/test/java/org/apache/johnzon/core/JsonStreamParserImplTest.java index bf767ab4..965d6e96 100644 --- a/johnzon-core/src/test/java/org/apache/johnzon/core/JsonStreamParserImplTest.java +++ b/johnzon-core/src/test/java/org/apache/johnzon/core/JsonStreamParserImplTest.java @@ -18,18 +18,26 @@ */ package org.apache.johnzon.core; +import jakarta.json.Json; +import jakarta.json.JsonReader; +import jakarta.json.JsonReaderFactory; +import jakarta.json.spi.JsonProvider; +import org.junit.Ignore; import org.junit.Test; import jakarta.json.stream.JsonParser; import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.StringReader; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; +import java.util.Map; import static java.util.Arrays.asList; import static java.util.Collections.emptyMap; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; public class JsonStreamParserImplTest { @Test @@ -94,4 +102,70 @@ public void ensureNoArrayBoundErrorWhenOverflow() throws IOException { asList("START_OBJECT", "KEY_NAME", "VALUE_STRING", "{\"foo\":\"barbar\\barbarbar\"}", "END_OBJECT"), events); } + + @Test + @Ignore("No real test, just run directly from your IDE") + public void largeStringPerformance() { + String json = "{\"data\":\"" + "a".repeat(50 * 1024 * 1024 + 1) + "\"}"; + + // Warmup + for (int i = 0; i < 10; i++) { + try (JsonReader reader = Json.createReader(new StringReader(json))) { + reader.readObject(); + } + } + + long start = System.currentTimeMillis(); + try (JsonReader reader = Json.createReader(new StringReader(json))) { + reader.readObject(); + } + System.err.println("Took " + (System.currentTimeMillis() - start) + "ms"); + } + + @Test + public void allBuffersReleased() { + String json = "{\"data\":\"" + "a".repeat(JsonParserFactoryImpl.DEFAULT_MAX_STRING_LENGTH * 2) + "\"}"; + + JsonReaderFactory readerFactory = JsonProvider.provider().createReaderFactory(Map.of( + JsonParserFactoryImpl.BUFFER_STRATEGY, TrackingBufferStrategy.class.getName())); + + try (JsonReader reader = readerFactory.createReader(new StringReader(json))) { + reader.readObject(); + } + + assertTrue(TrackingBufferStrategy.TrackingBufferProvider.borrowed.isEmpty()); + } + + public static class TrackingBufferStrategy implements BufferStrategy { + private final BufferStrategy delegate = BufferStrategyFactory.valueOf("BY_INSTANCE"); + + @Override + public BufferProvider newCharProvider(int size) { + return new TrackingBufferProvider(delegate.newCharProvider(size)); + } + + public static class TrackingBufferProvider implements BufferStrategy.BufferProvider { + protected static List borrowed = new ArrayList<>(); + + private final BufferStrategy.BufferProvider delegate; + + public TrackingBufferProvider(BufferStrategy.BufferProvider delegate) { + this.delegate = delegate; + } + + @Override + public char[] newBuffer() { + char[] result = delegate.newBuffer(); + borrowed.add(result); + + return result; + } + + @Override + public void release(char[] value) { + borrowed.remove(value); + delegate.release(value); + } + } + } }