From c647bbf000d00a8b689c9d2d1032e4a0caf9cf0b Mon Sep 17 00:00:00 2001 From: mariofusco Date: Wed, 19 Jul 2023 15:22:47 +0200 Subject: [PATCH 01/29] Introducing BufferRecyclerPool --- .../fasterxml/jackson/core/JsonFactory.java | 3 ++ .../jackson/core/base/ParserBase.java | 1 + .../fasterxml/jackson/core/io/IOContext.java | 39 ++++++++++++++++--- .../fasterxml/jackson/core/io/UTF8Writer.java | 1 + .../jackson/core/json/JsonGeneratorImpl.java | 6 +++ .../jackson/core/util/BufferRecyclers.java | 2 + .../jackson/core/io/TestIOContext.java | 2 +- .../jackson/core/io/TestMergedStream.java | 2 + .../jackson/core/io/UTF8WriterTest.java | 1 + .../util/ReadConstrainedTextBufferTest.java | 6 +-- .../core/write/GeneratorDupHandlingTest.java | 4 ++ .../jackson/core/write/GeneratorMiscTest.java | 2 + 12 files changed, 60 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java index 07317f1fc6..99172aec8d 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java @@ -1873,6 +1873,7 @@ protected JsonParser _createParser(InputStream in, IOContext ctxt) throws IOExce e.addSuppressed(e2); } } + ctxt.close(); throw e; } } @@ -2148,7 +2149,9 @@ protected JsonGenerator _decorate(JsonGenerator g) { * Note: only public to give access for {@code ObjectMapper} * * @return Buffer recycler instance to use + * @deprecated use BufferRecyclerPool to get a pooled instance of a BufferRecycler */ + @Deprecated public BufferRecycler _getBufferRecycler() { return _getBufferRecyclerPool().acquireBufferRecycler(this); diff --git a/src/main/java/com/fasterxml/jackson/core/base/ParserBase.java b/src/main/java/com/fasterxml/jackson/core/base/ParserBase.java index 8976086676..37571fbb4b 100644 --- a/src/main/java/com/fasterxml/jackson/core/base/ParserBase.java +++ b/src/main/java/com/fasterxml/jackson/core/base/ParserBase.java @@ -394,6 +394,7 @@ protected void _checkStdFeatureChanges(int newFeatureFlags, int changedFeatures) // as per [JACKSON-324], do in finally block // Also, internal buffer(s) can now be released as well _releaseBuffers(); + _ioContext.close(); } } } diff --git a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java index 060446c9db..b18d848d33 100644 --- a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java +++ b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java @@ -7,6 +7,7 @@ import com.fasterxml.jackson.core.util.BufferRecycler; import com.fasterxml.jackson.core.util.ReadConstrainedTextBuffer; import com.fasterxml.jackson.core.util.TextBuffer; +import tools.jackson.core.util.BufferRecyclerPool; /** * To limit number of configuration and state objects to pass, all @@ -16,7 +17,7 @@ *

* NOTE: non-final since 2.4, to allow sub-classing. */ -public class IOContext +public class IOContext implements AutoCloseable { /* /********************************************************************** @@ -119,6 +120,8 @@ public class IOContext */ protected char[] _nameCopyBuffer; + private boolean closed = false; + /* /********************************************************************** /* Life-cycle @@ -130,18 +133,36 @@ public class IOContext * * @param src constraints for streaming reads * @param swc constraints for streaming writes - * @param br BufferRecycler to use, if any ({@code null} if none) + * @param erc Error report configuration to use * @param contentRef Input source reference for location reporting * @param managedResource Whether input source is managed (owned) by Jackson library + * + * @since 2.16 + */ + public IOContext(StreamReadConstraints src, StreamWriteConstraints swc, ErrorReportConfiguration erc, + ContentReference contentRef, boolean managedResource) + { + this(src, swc, erc, BufferRecyclerPool.acquireBufferRecycler(), contentRef, managedResource); + } + + /** + * @param src constraints for streaming reads + * @param swc constraints for streaming writes * @param erc Error report configuration to use + * @param br BufferRecycler to use, if any ({@code null} if none) + * @param contentRef Input source reference for location reporting + * @param managedResource Whether input source is managed (owned) by Jackson library * * @since 2.16 */ + @Deprecated public IOContext(StreamReadConstraints src, StreamWriteConstraints swc, ErrorReportConfiguration erc, - BufferRecycler br, ContentReference contentRef, boolean managedResource) + BufferRecycler br, ContentReference contentRef, boolean managedResource) { - _streamReadConstraints = src; - _streamWriteConstraints = swc; + _streamReadConstraints = (src == null) ? + StreamReadConstraints.defaults() : src; + _streamWriteConstraints = (swc == null) ? + StreamWriteConstraints.defaults() : swc; _errorReportConfiguration = erc; _bufferRecycler = br; _contentReference = contentRef; @@ -458,4 +479,12 @@ private IllegalArgumentException wrongBuf() { // sanity check failed; trying to return different, smaller buffer. return new IllegalArgumentException("Trying to release buffer smaller than original"); } + + @Override + public void close() { + if (!closed) { + BufferRecyclerPool.offerBufferRecycler(_bufferRecycler); + closed = true; + } + } } diff --git a/src/main/java/com/fasterxml/jackson/core/io/UTF8Writer.java b/src/main/java/com/fasterxml/jackson/core/io/UTF8Writer.java index b7a47fe7fd..b7561839d1 100644 --- a/src/main/java/com/fasterxml/jackson/core/io/UTF8Writer.java +++ b/src/main/java/com/fasterxml/jackson/core/io/UTF8Writer.java @@ -76,6 +76,7 @@ public void close() illegalSurrogate(code); } } + _context.close(); } @Override diff --git a/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java b/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java index 2bc37a116e..f561ca0757 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java +++ b/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java @@ -120,6 +120,12 @@ public abstract class JsonGeneratorImpl extends GeneratorBase /********************************************************** */ + @Override + public void close() throws IOException { + super.close(); + _ioContext.close(); + } + @SuppressWarnings("deprecation") public JsonGeneratorImpl(IOContext ctxt, int features, ObjectCodec codec) { diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java index c7cbfd025a..a2b24559f5 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java @@ -12,7 +12,9 @@ * @see BufferRecycler * * @since 2.9.2 + * @deprecated use BufferRecyclerPool to get a pooled instance of a BufferRecycler */ +@Deprecated public class BufferRecyclers { /** diff --git a/src/test/java/com/fasterxml/jackson/core/io/TestIOContext.java b/src/test/java/com/fasterxml/jackson/core/io/TestIOContext.java index e6ca0ce0bb..328a76ed37 100644 --- a/src/test/java/com/fasterxml/jackson/core/io/TestIOContext.java +++ b/src/test/java/com/fasterxml/jackson/core/io/TestIOContext.java @@ -13,7 +13,6 @@ public void testAllocations() throws Exception IOContext ctxt = new IOContext(StreamReadConstraints.defaults(), StreamWriteConstraints.defaults(), ErrorReportConfiguration.defaults(), - new BufferRecycler(), ContentReference.rawReference("N/A"), true); /* I/O Read buffer */ @@ -94,6 +93,7 @@ public void testAllocations() throws Exception verifyException(e, "smaller than original"); } ctxt.releaseNameCopyBuffer(null); + ctxt.close(); } } diff --git a/src/test/java/com/fasterxml/jackson/core/io/TestMergedStream.java b/src/test/java/com/fasterxml/jackson/core/io/TestMergedStream.java index 7707ac3d34..81a75ba1e7 100644 --- a/src/test/java/com/fasterxml/jackson/core/io/TestMergedStream.java +++ b/src/test/java/com/fasterxml/jackson/core/io/TestMergedStream.java @@ -20,6 +20,8 @@ public void testSimple() throws Exception ctxt.setEncoding(JsonEncoding.UTF8); MergedStream ms = new MergedStream(ctxt, new ByteArrayInputStream(second), first, 99, 99+5); + ctxt.close(); + // Ok, first, should have 5 bytes from first buffer: assertEquals(5, ms.available()); // not supported when there's buffered stuff... diff --git a/src/test/java/com/fasterxml/jackson/core/io/UTF8WriterTest.java b/src/test/java/com/fasterxml/jackson/core/io/UTF8WriterTest.java index 1907b9bf82..9b1881f601 100644 --- a/src/test/java/com/fasterxml/jackson/core/io/UTF8WriterTest.java +++ b/src/test/java/com/fasterxml/jackson/core/io/UTF8WriterTest.java @@ -2,6 +2,7 @@ import java.io.*; +import com.fasterxml.jackson.core.StreamWriteConstraints; import org.junit.Assert; public class UTF8WriterTest diff --git a/src/test/java/com/fasterxml/jackson/core/util/ReadConstrainedTextBufferTest.java b/src/test/java/com/fasterxml/jackson/core/util/ReadConstrainedTextBufferTest.java index 6c4211ab77..643380adff 100644 --- a/src/test/java/com/fasterxml/jackson/core/util/ReadConstrainedTextBufferTest.java +++ b/src/test/java/com/fasterxml/jackson/core/util/ReadConstrainedTextBufferTest.java @@ -45,12 +45,12 @@ private static TextBuffer makeConstrainedBuffer(int maxStringLen) { StreamReadConstraints constraints = StreamReadConstraints.builder() .maxStringLength(maxStringLen) .build(); - IOContext ioContext = new IOContext( + try (IOContext ioContext = new IOContext( constraints, StreamWriteConstraints.defaults(), ErrorReportConfiguration.defaults(), - new BufferRecycler(), ContentReference.rawReference("N/A"), true); - return ioContext.constructReadConstrainedTextBuffer(); + return ioContext.constructReadConstrainedTextBuffer(); + } } } \ No newline at end of file diff --git a/src/test/java/com/fasterxml/jackson/core/write/GeneratorDupHandlingTest.java b/src/test/java/com/fasterxml/jackson/core/write/GeneratorDupHandlingTest.java index 560bae3869..8a83ae38b0 100644 --- a/src/test/java/com/fasterxml/jackson/core/write/GeneratorDupHandlingTest.java +++ b/src/test/java/com/fasterxml/jackson/core/write/GeneratorDupHandlingTest.java @@ -52,6 +52,8 @@ protected void _testSimpleDups(boolean useStream, boolean lazySetting, JsonFacto fail("Should have gotten exception"); } catch (JsonGenerationException e) { verifyException(e, "duplicate field 'a'"); + } finally { + g1.close(); } JsonGenerator g2; @@ -66,6 +68,8 @@ protected void _testSimpleDups(boolean useStream, boolean lazySetting, JsonFacto fail("Should have gotten exception"); } catch (JsonGenerationException e) { verifyException(e, "duplicate field 'x'"); + } finally { + g2.close(); } } diff --git a/src/test/java/com/fasterxml/jackson/core/write/GeneratorMiscTest.java b/src/test/java/com/fasterxml/jackson/core/write/GeneratorMiscTest.java index 4b5a1bc480..c046d75af3 100644 --- a/src/test/java/com/fasterxml/jackson/core/write/GeneratorMiscTest.java +++ b/src/test/java/com/fasterxml/jackson/core/write/GeneratorMiscTest.java @@ -256,6 +256,8 @@ public void testAsEmbedded() throws Exception fail("Expected an exception"); } catch (JsonGenerationException e) { verifyException(e, "No native support for"); + } finally { + g.close(); } } } From b1eacd48217ebb36ad2a16c2e72915e881a838a5 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Thu, 20 Jul 2023 17:40:04 +0200 Subject: [PATCH 02/29] fix ReadConstrainedTextBufferTest to work with both a brand new BufferRecycler and one taken from the pool --- .../tools/jackson/core/util/ObjectPool.java | 68 +++++++++++++++++++ .../util/ReadConstrainedTextBufferTest.java | 51 ++++++++------ 2 files changed, 100 insertions(+), 19 deletions(-) create mode 100644 src/main/java/tools/jackson/core/util/ObjectPool.java diff --git a/src/main/java/tools/jackson/core/util/ObjectPool.java b/src/main/java/tools/jackson/core/util/ObjectPool.java new file mode 100644 index 0000000000..67ee7000a8 --- /dev/null +++ b/src/main/java/tools/jackson/core/util/ObjectPool.java @@ -0,0 +1,68 @@ +package tools.jackson.core.util; + +import java.util.Queue; +import java.util.concurrent.LinkedTransferQueue; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; +import java.util.function.Supplier; + +public interface ObjectPool extends AutoCloseable { + + T borrow(); + void offer(T t); + + default void withPooledObject(Consumer objectConsumer) { + T t = borrow(); + try { + objectConsumer.accept(t); + } finally { + offer(t); + } + } + + static ObjectPool newLockFreePool(Supplier factory) { + return new LockFreeObjectPool<>(factory); + } + + static ObjectPool newLockFreePool(Supplier factory, Consumer destroyer) { + return new LockFreeObjectPool<>(factory, destroyer); + } + + class LockFreeObjectPool implements ObjectPool { + private final Supplier factory; + private final Consumer destroyer; + + private final Queue pool = new LinkedTransferQueue<>(); + + private final AtomicInteger counter = new AtomicInteger(0); + + public LockFreeObjectPool(Supplier factory) { + this(factory, null); + } + + public LockFreeObjectPool(Supplier factory, Consumer destroyer) { + this.factory = factory; + this.destroyer = destroyer; + } + + @Override + public T borrow() { +// System.out.println("Borrow: " + counter.incrementAndGet()); + T t = pool.poll(); + return t != null ? t : factory.get(); + } + + @Override + public void offer(T t) { + pool.offer(t); +// System.out.println("Offer: " + counter.decrementAndGet()); + } + + @Override + public void close() throws Exception { + if (destroyer != null) { + pool.forEach(destroyer); + } + } + } +} diff --git a/src/test/java/com/fasterxml/jackson/core/util/ReadConstrainedTextBufferTest.java b/src/test/java/com/fasterxml/jackson/core/util/ReadConstrainedTextBufferTest.java index 643380adff..0801b1fdaf 100644 --- a/src/test/java/com/fasterxml/jackson/core/util/ReadConstrainedTextBufferTest.java +++ b/src/test/java/com/fasterxml/jackson/core/util/ReadConstrainedTextBufferTest.java @@ -16,41 +16,54 @@ class ReadConstrainedTextBufferTest { @Test public void appendCharArray() throws Exception { - TextBuffer constrained = makeConstrainedBuffer(SEGMENT_SIZE); - char[] chars = new char[SEGMENT_SIZE]; - Arrays.fill(chars, 'A'); - constrained.append(chars, 0, SEGMENT_SIZE); - Assertions.assertThrows(IOException.class, () -> constrained.append(chars, 0, SEGMENT_SIZE)); + try (IOContext ioContext = makeConstrainedContext(SEGMENT_SIZE)) { + TextBuffer constrained = ioContext.constructReadConstrainedTextBuffer(); + char[] chars = new char[SEGMENT_SIZE]; + Arrays.fill(chars, 'A'); + constrained.append(chars, 0, SEGMENT_SIZE); + Assertions.assertThrows(IOException.class, () -> { + constrained.append(chars, 0, SEGMENT_SIZE); + constrained.contentsAsString(); + }); + } } @Test public void appendString() throws Exception { - TextBuffer constrained = makeConstrainedBuffer(SEGMENT_SIZE); - char[] chars = new char[SEGMENT_SIZE]; - Arrays.fill(chars, 'A'); - constrained.append(new String(chars), 0, SEGMENT_SIZE); - Assertions.assertThrows(IOException.class, () -> constrained.append(new String(chars), 0, SEGMENT_SIZE)); + try (IOContext ioContext = makeConstrainedContext(SEGMENT_SIZE)) { + TextBuffer constrained = ioContext.constructReadConstrainedTextBuffer(); + char[] chars = new char[SEGMENT_SIZE]; + Arrays.fill(chars, 'A'); + constrained.append(new String(chars), 0, SEGMENT_SIZE); + Assertions.assertThrows(IOException.class, () -> { + constrained.append(new String(chars), 0, SEGMENT_SIZE); + constrained.contentsAsString(); + }); + } } @Test public void appendSingle() throws Exception { - TextBuffer constrained = makeConstrainedBuffer(SEGMENT_SIZE); - char[] chars = new char[SEGMENT_SIZE]; - Arrays.fill(chars, 'A'); - constrained.append(chars, 0, SEGMENT_SIZE); - Assertions.assertThrows(IOException.class, () -> constrained.append('x')); + try (IOContext ioContext = makeConstrainedContext(SEGMENT_SIZE)) { + TextBuffer constrained = ioContext.constructReadConstrainedTextBuffer(); + char[] chars = new char[SEGMENT_SIZE]; + Arrays.fill(chars, 'A'); + constrained.append(chars, 0, SEGMENT_SIZE); + Assertions.assertThrows(IOException.class, () -> { + constrained.append('x'); + constrained.contentsAsString(); + }); + } } - private static TextBuffer makeConstrainedBuffer(int maxStringLen) { + private static IOContext makeConstrainedContext(int maxStringLen) { StreamReadConstraints constraints = StreamReadConstraints.builder() .maxStringLength(maxStringLen) .build(); - try (IOContext ioContext = new IOContext( + return new IOContext( constraints, StreamWriteConstraints.defaults(), ErrorReportConfiguration.defaults(), ContentReference.rawReference("N/A"), true); - return ioContext.constructReadConstrainedTextBuffer(); - } } } \ No newline at end of file From d5d3a147890c318241a21eb4e039f27595d3fdb6 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Fri, 21 Jul 2023 08:52:52 +0200 Subject: [PATCH 03/29] minor refactor --- src/main/java/com/fasterxml/jackson/core/io/IOContext.java | 2 +- .../{tools => com/fasterxml}/jackson/core/util/ObjectPool.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename src/main/java/{tools => com/fasterxml}/jackson/core/util/ObjectPool.java (97%) diff --git a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java index b18d848d33..b97b51ef44 100644 --- a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java +++ b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java @@ -7,7 +7,7 @@ import com.fasterxml.jackson.core.util.BufferRecycler; import com.fasterxml.jackson.core.util.ReadConstrainedTextBuffer; import com.fasterxml.jackson.core.util.TextBuffer; -import tools.jackson.core.util.BufferRecyclerPool; +import com.fasterxml.jackson.core.util.BufferRecyclerPool; /** * To limit number of configuration and state objects to pass, all diff --git a/src/main/java/tools/jackson/core/util/ObjectPool.java b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java similarity index 97% rename from src/main/java/tools/jackson/core/util/ObjectPool.java rename to src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java index 67ee7000a8..7617db2ad4 100644 --- a/src/main/java/tools/jackson/core/util/ObjectPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java @@ -1,4 +1,4 @@ -package tools.jackson.core.util; +package com.fasterxml.jackson.core.util; import java.util.Queue; import java.util.concurrent.LinkedTransferQueue; From 67459da8cb25a920600c8a8452e2175e1d917581 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Mon, 24 Jul 2023 09:08:59 +0200 Subject: [PATCH 04/29] check performances of different object pool implementations --- pom.xml | 5 + .../jackson/core/util/ObjectPool.java | 280 +++++++++++++++++- 2 files changed, 278 insertions(+), 7 deletions(-) diff --git a/pom.xml b/pom.xml index eced5d43c4..86f31170fd 100644 --- a/pom.xml +++ b/pom.xml @@ -306,6 +306,11 @@ com.fasterxml.jackson.core.*;version=${project.version} fastdoubleparser 0.9.0 + + org.jctools + jctools-core + 4.0.1 + org.junit.vintage diff --git a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java index 7617db2ad4..05106d1176 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java @@ -1,8 +1,15 @@ package com.fasterxml.jackson.core.util; +import org.jctools.queues.MpmcArrayQueue; + import java.util.Queue; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; import java.util.concurrent.LinkedTransferQueue; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.atomic.AtomicReferenceArray; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; import java.util.function.Supplier; @@ -20,15 +27,43 @@ default void withPooledObject(Consumer objectConsumer) { } } - static ObjectPool newLockFreePool(Supplier factory) { - return new LockFreeObjectPool<>(factory); + enum Strategy { + DUMMY, BUFFER_RECYCLERS, LINKED_QUEUE, + ATOMIC_REF_ARRAY_16, ATOMIC_REF_ARRAY_64, + SYNCED_ARRAY_16, SYNCED_ARRAY_64, + SHARDED_SYNCED_ARRAY_4_64, SHARDED_SYNCED_ARRAY_16_16, + LOCK_FREE, LOCK_FREE_ASYNC_OFFER, + JCTOOLS_16, JCTOOLS_64 + } + + class StrategyHolder { + private static Strategy strategy = Strategy.SYNCED_ARRAY_16; + + public static void setStrategy(String name) { + strategy = Strategy.valueOf(name); + } } - static ObjectPool newLockFreePool(Supplier factory, Consumer destroyer) { - return new LockFreeObjectPool<>(factory, destroyer); + static ObjectPool newLockFreePool(Supplier factory) { + switch (StrategyHolder.strategy) { + case DUMMY: return new DummyPool<>(factory); + case BUFFER_RECYCLERS: return new DummyPool<>(() -> (T) BufferRecyclers.getBufferRecycler()); + case LINKED_QUEUE: return new LinkedQueuePool<>(factory); + case ATOMIC_REF_ARRAY_16: return new AtomicReferenceArrayPool<>(factory, 16); + case ATOMIC_REF_ARRAY_64: return new AtomicReferenceArrayPool<>(factory, 64); + case SYNCED_ARRAY_16: return new SyncedArrayPool<>(factory, 16); + case SYNCED_ARRAY_64: return new SyncedArrayPool<>(factory, 64); + case SHARDED_SYNCED_ARRAY_4_64: return new ShardedSyncedArrayPool<>(factory, 4, 64); + case SHARDED_SYNCED_ARRAY_16_16: return new ShardedSyncedArrayPool<>(factory, 16, 16); + case LOCK_FREE: return new LockFreePool<>(factory); + case LOCK_FREE_ASYNC_OFFER: return new LockFreeAsyncOfferPool<>(factory); + case JCTOOLS_16: return new JCToolsPool<>(factory, 16); + case JCTOOLS_64: return new JCToolsPool<>(factory, 64); + } + throw new UnsupportedOperationException(); } - class LockFreeObjectPool implements ObjectPool { + class LinkedQueuePool implements ObjectPool { private final Supplier factory; private final Consumer destroyer; @@ -36,11 +71,11 @@ class LockFreeObjectPool implements ObjectPool { private final AtomicInteger counter = new AtomicInteger(0); - public LockFreeObjectPool(Supplier factory) { + public LinkedQueuePool(Supplier factory) { this(factory, null); } - public LockFreeObjectPool(Supplier factory, Consumer destroyer) { + public LinkedQueuePool(Supplier factory, Consumer destroyer) { this.factory = factory; this.destroyer = destroyer; } @@ -65,4 +100,235 @@ public void close() throws Exception { } } } + + class AtomicReferenceArrayPool implements ObjectPool { + private final Supplier factory; + + private final AtomicInteger counter = new AtomicInteger(0); + + private final int size; + + private final AtomicReferenceArray pool; + + public AtomicReferenceArrayPool(Supplier factory, int size) { + this.factory = factory; + this.size = size; + this.pool = new AtomicReferenceArray<>(size); + } + + @Override + public T borrow() { + int pos = counter.getAndUpdate(i -> i > 0 ? i-1 : i); + if (pos <= 0) { + return factory.get(); + } + T t = pool.getAndSet(pos-1, null); + return t == null ? borrow() : t; + } + + @Override + public void offer(T t) { + int pos = counter.getAndUpdate(i -> i < 0 ? 1 : i+1); + if (pos < 0) { + pool.set(0, t); + } else if (pos < size) { + pool.set(pos, t); + } + } + + @Override + public void close() throws Exception { + + } + } + + class SyncedArrayPool implements ObjectPool { + private final Supplier factory; + + private final int size; + + private final Object[] pool; + + private final ReentrantLock lock = new ReentrantLock(); + + private volatile int counter = 0; + + public SyncedArrayPool(Supplier factory, int size) { + this.factory = factory; + this.size = size; + this.pool = new Object[size]; + } + + @Override + public T borrow() { + lock.lock(); + try { + if (counter == 0) { + return factory.get(); + } + T t = (T) pool[--counter]; + pool[counter] = null; + return t; + } finally { + lock.unlock(); + } + } + + @Override + public void offer(T t) { + lock.lock(); + try { + if (counter < size) { + pool[counter++] = t; + } + } finally { + lock.unlock(); + } + } + + @Override + public void close() throws Exception { + + } + } + + class ShardedSyncedArrayPool implements ObjectPool { + private final SyncedArrayPool[] pools; + + public ShardedSyncedArrayPool(Supplier factory, int shardsNr, int shardSize) { + this.pools = new SyncedArrayPool[shardsNr]; + for (int i = 0; i < pools.length; i++) { + pools[i] = new SyncedArrayPool(factory, shardSize); + } + } + + @Override + public T borrow() { + return pools[Thread.currentThread().hashCode() % pools.length].borrow(); + } + + @Override + public void offer(T t) { + pools[Thread.currentThread().hashCode() % pools.length].offer(t); + } + + @Override + public void close() throws Exception { + + } + } + + class DummyPool implements ObjectPool { + private final Supplier factory; + + + public DummyPool(Supplier factory) { + this.factory = factory; + } + + @Override + public T borrow() { + return factory.get(); + } + + @Override + public void offer(T t) { + + } + + @Override + public void close() throws Exception { + + } + } + + class JCToolsPool implements ObjectPool { + private final MpmcArrayQueue queue; + + private final Supplier factory; + + public JCToolsPool(Supplier factory, int size) { + this.factory = factory; + this.queue = new MpmcArrayQueue<>(size); + } + + @Override + public T borrow() { + T t = queue.poll(); + return t == null ? factory.get() : t; + } + + @Override + public void offer(T t) { + queue.offer(t); + } + + @Override + public void close() throws Exception { + + } + } + + class LockFreePool implements ObjectPool { + private final AtomicReference> head = new AtomicReference<>(); + + private final Supplier factory; + + public LockFreePool(Supplier factory) { + this.factory = factory; + } + + @Override + public T borrow() { + for (int i = 0; i < 3; i++) { + Node currentHead = head.get(); + if (currentHead == null) { + return factory.get(); + } + if (head.compareAndSet(currentHead, currentHead.next)) { + return currentHead.value; + } + } + return factory.get(); + } + + @Override + public void offer(T object) { + Node newHead = new Node<>(object); + for (int i = 0; i < 3; i++) { + newHead.next = head.get(); + if (head.compareAndSet(newHead.next, newHead)) { + return; + } + } + } + + @Override + public void close() throws Exception { + + } + } + + class LockFreeAsyncOfferPool extends LockFreePool { + + private static final Executor asyncOffer = Executors.newSingleThreadExecutor(); + + public LockFreeAsyncOfferPool(Supplier factory) { + super(factory); + } + + @Override + public void offer(T object) { + asyncOffer.execute( () -> super.offer(object) ); + } + } + + class Node { + final T value; + Node next; + + Node(T value) { + this.value = value; + } + } } From 7099315289b4545e7b7b2b8e7719dfb8496e01fe Mon Sep 17 00:00:00 2001 From: mariofusco Date: Tue, 25 Jul 2023 17:05:48 +0200 Subject: [PATCH 05/29] ensure that all parsers and generators are properly closed in tests --- .../java/com/fasterxml/jackson/core/util/ObjectPool.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java index 05106d1176..5fbd1652e0 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java @@ -37,7 +37,7 @@ enum Strategy { } class StrategyHolder { - private static Strategy strategy = Strategy.SYNCED_ARRAY_16; + private static Strategy strategy = Strategy.LINKED_QUEUE; public static void setStrategy(String name) { strategy = Strategy.valueOf(name); @@ -69,8 +69,6 @@ class LinkedQueuePool implements ObjectPool { private final Queue pool = new LinkedTransferQueue<>(); - private final AtomicInteger counter = new AtomicInteger(0); - public LinkedQueuePool(Supplier factory) { this(factory, null); } @@ -82,7 +80,6 @@ public LinkedQueuePool(Supplier factory, Consumer destroyer) { @Override public T borrow() { -// System.out.println("Borrow: " + counter.incrementAndGet()); T t = pool.poll(); return t != null ? t : factory.get(); } @@ -90,7 +87,6 @@ public T borrow() { @Override public void offer(T t) { pool.offer(t); -// System.out.println("Offer: " + counter.decrementAndGet()); } @Override From b404daf6d72cea7d9e3559675719dcaaded1925f Mon Sep 17 00:00:00 2001 From: mariofusco Date: Thu, 27 Jul 2023 10:02:17 +0200 Subject: [PATCH 06/29] cleanup pool implementations --- pom.xml | 5 - .../fasterxml/jackson/core/io/IOContext.java | 3 +- .../jackson/core/util/ObjectPool.java | 256 +++--------------- 3 files changed, 40 insertions(+), 224 deletions(-) diff --git a/pom.xml b/pom.xml index 86f31170fd..eced5d43c4 100644 --- a/pom.xml +++ b/pom.xml @@ -306,11 +306,6 @@ com.fasterxml.jackson.core.*;version=${project.version} fastdoubleparser 0.9.0 - - org.jctools - jctools-core - 4.0.1 - org.junit.vintage diff --git a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java index b97b51ef44..e009f48375 100644 --- a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java +++ b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java @@ -169,7 +169,6 @@ public IOContext(StreamReadConstraints src, StreamWriteConstraints swc, ErrorRep _sourceRef = contentRef.getRawContent(); _managedResource = managedResource; } - /** * Deprecated legacy constructor. * @@ -483,7 +482,7 @@ private IllegalArgumentException wrongBuf() { @Override public void close() { if (!closed) { - BufferRecyclerPool.offerBufferRecycler(_bufferRecycler); + BufferRecyclerPool.releaseBufferRecycler(_bufferRecycler); closed = true; } } diff --git a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java index 5fbd1652e0..1e934e0cae 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java @@ -1,219 +1,47 @@ package com.fasterxml.jackson.core.util; -import org.jctools.queues.MpmcArrayQueue; - -import java.util.Queue; -import java.util.concurrent.Executor; -import java.util.concurrent.Executors; -import java.util.concurrent.LinkedTransferQueue; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.Deque; +import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.atomic.AtomicReference; -import java.util.concurrent.atomic.AtomicReferenceArray; -import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; import java.util.function.Supplier; public interface ObjectPool extends AutoCloseable { - T borrow(); - void offer(T t); + T acquire(); + void release(T t); default void withPooledObject(Consumer objectConsumer) { - T t = borrow(); + T t = acquire(); try { objectConsumer.accept(t); } finally { - offer(t); + release(t); } } enum Strategy { - DUMMY, BUFFER_RECYCLERS, LINKED_QUEUE, - ATOMIC_REF_ARRAY_16, ATOMIC_REF_ARRAY_64, - SYNCED_ARRAY_16, SYNCED_ARRAY_64, - SHARDED_SYNCED_ARRAY_4_64, SHARDED_SYNCED_ARRAY_16_16, - LOCK_FREE, LOCK_FREE_ASYNC_OFFER, - JCTOOLS_16, JCTOOLS_64 + DUMMY, BUFFER_RECYCLERS, CONCURRENT_DEQUEUE, LOCK_FREE } class StrategyHolder { - private static Strategy strategy = Strategy.LINKED_QUEUE; + private static Strategy strategy = Strategy.LOCK_FREE; public static void setStrategy(String name) { strategy = Strategy.valueOf(name); } } - static ObjectPool newLockFreePool(Supplier factory) { + static ObjectPool newObjectPool(Supplier factory) { switch (StrategyHolder.strategy) { case DUMMY: return new DummyPool<>(factory); case BUFFER_RECYCLERS: return new DummyPool<>(() -> (T) BufferRecyclers.getBufferRecycler()); - case LINKED_QUEUE: return new LinkedQueuePool<>(factory); - case ATOMIC_REF_ARRAY_16: return new AtomicReferenceArrayPool<>(factory, 16); - case ATOMIC_REF_ARRAY_64: return new AtomicReferenceArrayPool<>(factory, 64); - case SYNCED_ARRAY_16: return new SyncedArrayPool<>(factory, 16); - case SYNCED_ARRAY_64: return new SyncedArrayPool<>(factory, 64); - case SHARDED_SYNCED_ARRAY_4_64: return new ShardedSyncedArrayPool<>(factory, 4, 64); - case SHARDED_SYNCED_ARRAY_16_16: return new ShardedSyncedArrayPool<>(factory, 16, 16); + case CONCURRENT_DEQUEUE: return new ConcurrentDequePool<>(factory); case LOCK_FREE: return new LockFreePool<>(factory); - case LOCK_FREE_ASYNC_OFFER: return new LockFreeAsyncOfferPool<>(factory); - case JCTOOLS_16: return new JCToolsPool<>(factory, 16); - case JCTOOLS_64: return new JCToolsPool<>(factory, 64); } throw new UnsupportedOperationException(); } - class LinkedQueuePool implements ObjectPool { - private final Supplier factory; - private final Consumer destroyer; - - private final Queue pool = new LinkedTransferQueue<>(); - - public LinkedQueuePool(Supplier factory) { - this(factory, null); - } - - public LinkedQueuePool(Supplier factory, Consumer destroyer) { - this.factory = factory; - this.destroyer = destroyer; - } - - @Override - public T borrow() { - T t = pool.poll(); - return t != null ? t : factory.get(); - } - - @Override - public void offer(T t) { - pool.offer(t); - } - - @Override - public void close() throws Exception { - if (destroyer != null) { - pool.forEach(destroyer); - } - } - } - - class AtomicReferenceArrayPool implements ObjectPool { - private final Supplier factory; - - private final AtomicInteger counter = new AtomicInteger(0); - - private final int size; - - private final AtomicReferenceArray pool; - - public AtomicReferenceArrayPool(Supplier factory, int size) { - this.factory = factory; - this.size = size; - this.pool = new AtomicReferenceArray<>(size); - } - - @Override - public T borrow() { - int pos = counter.getAndUpdate(i -> i > 0 ? i-1 : i); - if (pos <= 0) { - return factory.get(); - } - T t = pool.getAndSet(pos-1, null); - return t == null ? borrow() : t; - } - - @Override - public void offer(T t) { - int pos = counter.getAndUpdate(i -> i < 0 ? 1 : i+1); - if (pos < 0) { - pool.set(0, t); - } else if (pos < size) { - pool.set(pos, t); - } - } - - @Override - public void close() throws Exception { - - } - } - - class SyncedArrayPool implements ObjectPool { - private final Supplier factory; - - private final int size; - - private final Object[] pool; - - private final ReentrantLock lock = new ReentrantLock(); - - private volatile int counter = 0; - - public SyncedArrayPool(Supplier factory, int size) { - this.factory = factory; - this.size = size; - this.pool = new Object[size]; - } - - @Override - public T borrow() { - lock.lock(); - try { - if (counter == 0) { - return factory.get(); - } - T t = (T) pool[--counter]; - pool[counter] = null; - return t; - } finally { - lock.unlock(); - } - } - - @Override - public void offer(T t) { - lock.lock(); - try { - if (counter < size) { - pool[counter++] = t; - } - } finally { - lock.unlock(); - } - } - - @Override - public void close() throws Exception { - - } - } - - class ShardedSyncedArrayPool implements ObjectPool { - private final SyncedArrayPool[] pools; - - public ShardedSyncedArrayPool(Supplier factory, int shardsNr, int shardSize) { - this.pools = new SyncedArrayPool[shardsNr]; - for (int i = 0; i < pools.length; i++) { - pools[i] = new SyncedArrayPool(factory, shardSize); - } - } - - @Override - public T borrow() { - return pools[Thread.currentThread().hashCode() % pools.length].borrow(); - } - - @Override - public void offer(T t) { - pools[Thread.currentThread().hashCode() % pools.length].offer(t); - } - - @Override - public void close() throws Exception { - - } - } - class DummyPool implements ObjectPool { private final Supplier factory; @@ -223,12 +51,12 @@ public DummyPool(Supplier factory) { } @Override - public T borrow() { + public T acquire() { return factory.get(); } @Override - public void offer(T t) { + public void release(T t) { } @@ -238,30 +66,37 @@ public void close() throws Exception { } } - class JCToolsPool implements ObjectPool { - private final MpmcArrayQueue queue; - + class ConcurrentDequePool implements ObjectPool { private final Supplier factory; + private final Consumer destroyer; + + private final Deque pool = new ConcurrentLinkedDeque<>(); - public JCToolsPool(Supplier factory, int size) { + public ConcurrentDequePool(Supplier factory) { + this(factory, null); + } + + public ConcurrentDequePool(Supplier factory, Consumer destroyer) { this.factory = factory; - this.queue = new MpmcArrayQueue<>(size); + this.destroyer = destroyer; } @Override - public T borrow() { - T t = queue.poll(); - return t == null ? factory.get() : t; + public T acquire() { + T t = pool.pollFirst(); + return t != null ? t : factory.get(); } @Override - public void offer(T t) { - queue.offer(t); + public void release(T t) { + pool.offerLast(t); } @Override public void close() throws Exception { - + if (destroyer != null) { + pool.forEach(destroyer); + } } } @@ -275,13 +110,14 @@ public LockFreePool(Supplier factory) { } @Override - public T borrow() { + public T acquire() { for (int i = 0; i < 3; i++) { Node currentHead = head.get(); if (currentHead == null) { return factory.get(); } if (head.compareAndSet(currentHead, currentHead.next)) { + currentHead.next = null; return currentHead.value; } } @@ -289,7 +125,7 @@ public T borrow() { } @Override - public void offer(T object) { + public void release(T object) { Node newHead = new Node<>(object); for (int i = 0; i < 3; i++) { newHead.next = head.get(); @@ -303,28 +139,14 @@ public void offer(T object) { public void close() throws Exception { } - } - - class LockFreeAsyncOfferPool extends LockFreePool { - - private static final Executor asyncOffer = Executors.newSingleThreadExecutor(); - - public LockFreeAsyncOfferPool(Supplier factory) { - super(factory); - } - @Override - public void offer(T object) { - asyncOffer.execute( () -> super.offer(object) ); - } - } - - class Node { - final T value; - Node next; + static class Node { + final T value; + Node next; - Node(T value) { - this.value = value; + Node(T value) { + this.value = value; + } } } } From 30dc6c39db59b9b8ca0f7e3c374cbc94175dd82a Mon Sep 17 00:00:00 2001 From: mariofusco Date: Thu, 27 Jul 2023 16:59:53 +0200 Subject: [PATCH 07/29] make BufferRecycler closeable and disable the new ObjectPool by default --- .../fasterxml/jackson/core/JsonFactory.java | 30 +++++------- .../fasterxml/jackson/core/io/IOContext.java | 9 ++-- .../jackson/core/json/JsonGeneratorImpl.java | 6 ++- .../jackson/core/util/BufferRecycler.java | 16 ++++++- .../jackson/core/util/BufferRecyclers.java | 2 - .../jackson/core/util/ObjectPool.java | 46 ++++--------------- 6 files changed, 46 insertions(+), 63 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java index 99172aec8d..b732b835d9 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java @@ -131,6 +131,18 @@ public enum Feature */ USE_THREAD_LOCAL_FOR_BUFFER_RECYCLING(true), + /** + * Feature that determines whether we will use {@link BufferRecycler} with + * an object pool not making use of a {@link ThreadLocal}, for efficient reuse of + * underlying input/output buffers. + * This is suggested especially in environments making use of virtual threads. + *

+ * This setting is disabled by default. If enabled it overrides USE_THREAD_LOCAL_FOR_BUFFER_RECYCLING. + * + * @since 2.16 + */ + USE_OBJECT_POOL_FOR_BUFFER_RECYCLING(false), + /** * Feature to control charset detection for byte-based inputs ({@code byte[]}, {@link InputStream}...). * When this feature is enabled (the default), the factory will allow UTF-16 and UTF-32 inputs and try to detect @@ -2149,30 +2161,12 @@ protected JsonGenerator _decorate(JsonGenerator g) { * Note: only public to give access for {@code ObjectMapper} * * @return Buffer recycler instance to use - * @deprecated use BufferRecyclerPool to get a pooled instance of a BufferRecycler */ - @Deprecated public BufferRecycler _getBufferRecycler() { return _getBufferRecyclerPool().acquireBufferRecycler(this); } - /** - * Accessor for getting access to {@link BufferRecyclerPool} for getting - * {@link BufferRecycler} instance to use. - * - * @since 2.16 - */ - public BufferRecyclerPool _getBufferRecyclerPool() { - // 23-Apr-2015, tatu: Let's allow disabling of buffer recycling - // scheme, for cases where it is considered harmful (possibly - // on Android, for example) - if (!Feature.USE_THREAD_LOCAL_FOR_BUFFER_RECYCLING.enabledIn(_factoryFeatures)) { - return BufferRecyclers.nopRecyclerPool(); - } - return _bufferRecyclerPool; - } - /** * Overridable factory method that actually instantiates desired * context object. diff --git a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java index e009f48375..49709cd32b 100644 --- a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java +++ b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java @@ -120,7 +120,7 @@ public class IOContext implements AutoCloseable */ protected char[] _nameCopyBuffer; - private boolean closed = false; + private boolean _closed = false; /* /********************************************************************** @@ -169,6 +169,7 @@ public IOContext(StreamReadConstraints src, StreamWriteConstraints swc, ErrorRep _sourceRef = contentRef.getRawContent(); _managedResource = managedResource; } + /** * Deprecated legacy constructor. * @@ -481,9 +482,9 @@ private IllegalArgumentException wrongBuf() { @Override public void close() { - if (!closed) { - BufferRecyclerPool.releaseBufferRecycler(_bufferRecycler); - closed = true; + if (!_closed) { + _bufferRecycler.close(); + _closed = true; } } } diff --git a/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java b/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java index f561ca0757..487b41decc 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java +++ b/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java @@ -122,8 +122,10 @@ public abstract class JsonGeneratorImpl extends GeneratorBase @Override public void close() throws IOException { - super.close(); - _ioContext.close(); + if (!isClosed()) { + super.close(); + _ioContext.close(); + } } @SuppressWarnings("deprecation") diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java index a17532a683..1f729a7d85 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java @@ -13,7 +13,7 @@ * Rewritten in 2.10 to be thread-safe (see [jackson-core#479] for details), * to not rely on {@code ThreadLocal} access. */ -public class BufferRecycler +public class BufferRecycler implements AutoCloseable { /** * Buffer used for reading byte-based input. @@ -82,6 +82,8 @@ public class BufferRecycler // Note: changed from simple array in 2.10: protected final AtomicReferenceArray _charBuffers; + private ObjectPool _pool; + /* /********************************************************** /* Construction @@ -189,4 +191,16 @@ protected int charBufferLength(int ix) { protected byte[] balloc(int size) { return new byte[size]; } protected char[] calloc(int size) { return new char[size]; } + + BufferRecycler withPool(ObjectPool pool) { + this._pool = pool; + return this; + } + + @Override + public void close() { + if (_pool != null) { + _pool.release(this); + } + } } diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java index a2b24559f5..c7cbfd025a 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java @@ -12,9 +12,7 @@ * @see BufferRecycler * * @since 2.9.2 - * @deprecated use BufferRecyclerPool to get a pooled instance of a BufferRecycler */ -@Deprecated public class BufferRecyclers { /** diff --git a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java index 1e934e0cae..3973112aca 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java @@ -4,7 +4,7 @@ import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; -import java.util.function.Supplier; +import java.util.function.Function; public interface ObjectPool extends AutoCloseable { @@ -32,51 +32,25 @@ public static void setStrategy(String name) { } } - static ObjectPool newObjectPool(Supplier factory) { + static ObjectPool newObjectPool(Function, T> factory) { switch (StrategyHolder.strategy) { - case DUMMY: return new DummyPool<>(factory); - case BUFFER_RECYCLERS: return new DummyPool<>(() -> (T) BufferRecyclers.getBufferRecycler()); case CONCURRENT_DEQUEUE: return new ConcurrentDequePool<>(factory); case LOCK_FREE: return new LockFreePool<>(factory); } throw new UnsupportedOperationException(); } - class DummyPool implements ObjectPool { - private final Supplier factory; - - - public DummyPool(Supplier factory) { - this.factory = factory; - } - - @Override - public T acquire() { - return factory.get(); - } - - @Override - public void release(T t) { - - } - - @Override - public void close() throws Exception { - - } - } - class ConcurrentDequePool implements ObjectPool { - private final Supplier factory; + private final Function, T> factory; private final Consumer destroyer; private final Deque pool = new ConcurrentLinkedDeque<>(); - public ConcurrentDequePool(Supplier factory) { + public ConcurrentDequePool(Function, T> factory) { this(factory, null); } - public ConcurrentDequePool(Supplier factory, Consumer destroyer) { + public ConcurrentDequePool(Function, T> factory, Consumer destroyer) { this.factory = factory; this.destroyer = destroyer; } @@ -84,7 +58,7 @@ public ConcurrentDequePool(Supplier factory, Consumer destroyer) { @Override public T acquire() { T t = pool.pollFirst(); - return t != null ? t : factory.get(); + return t != null ? t : factory.apply(this); } @Override @@ -103,9 +77,9 @@ public void close() throws Exception { class LockFreePool implements ObjectPool { private final AtomicReference> head = new AtomicReference<>(); - private final Supplier factory; + private final Function, T> factory; - public LockFreePool(Supplier factory) { + public LockFreePool(Function, T> factory) { this.factory = factory; } @@ -114,14 +88,14 @@ public T acquire() { for (int i = 0; i < 3; i++) { Node currentHead = head.get(); if (currentHead == null) { - return factory.get(); + return factory.apply(this); } if (head.compareAndSet(currentHead, currentHead.next)) { currentHead.next = null; return currentHead.value; } } - return factory.get(); + return factory.apply(this); } @Override From 0f8bd70d20fdfc30ea8e633fc8df99889fc82697 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Thu, 27 Jul 2023 17:09:35 +0200 Subject: [PATCH 08/29] wip --- src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java index 3973112aca..3b01b63693 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java @@ -21,7 +21,7 @@ default void withPooledObject(Consumer objectConsumer) { } enum Strategy { - DUMMY, BUFFER_RECYCLERS, CONCURRENT_DEQUEUE, LOCK_FREE + CONCURRENT_DEQUEUE, LOCK_FREE } class StrategyHolder { From b09fd6ee682f50fbd2262196f059900ad51370c0 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Thu, 27 Jul 2023 18:01:10 +0200 Subject: [PATCH 09/29] wip --- .../java/com/fasterxml/jackson/core/util/BufferRecycler.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java index 1f729a7d85..26516ec233 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java @@ -201,6 +201,7 @@ BufferRecycler withPool(ObjectPool pool) { public void close() { if (_pool != null) { _pool.release(this); + _pool = null; } } } From d9490430797f87d6578fecde4793c50e1c37c722 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Fri, 28 Jul 2023 09:58:59 +0200 Subject: [PATCH 10/29] javadoc + better pool management + pool decorator for debug --- .../jackson/core/util/BufferRecycler.java | 6 +- .../jackson/core/util/ObjectPool.java | 91 +++++++++++++++---- 2 files changed, 80 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java index 26516ec233..4c8843610f 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java @@ -193,6 +193,9 @@ protected int charBufferLength(int ix) { protected char[] calloc(int size) { return new char[size]; } BufferRecycler withPool(ObjectPool pool) { + if (this._pool != null) { + throw new IllegalStateException(); + } this._pool = pool; return this; } @@ -200,8 +203,9 @@ BufferRecycler withPool(ObjectPool pool) { @Override public void close() { if (_pool != null) { - _pool.release(this); + ObjectPool tempPool = _pool; _pool = null; + tempPool.release(this); } } } diff --git a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java index 3b01b63693..c17d78cc2a 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java @@ -3,9 +3,19 @@ import java.util.Deque; import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.atomic.LongAdder; import java.util.function.Consumer; import java.util.function.Function; - +import java.util.function.Supplier; + +/** + * This is a utility class, whose main functionality is pooling object + * with a huge memory footprint and that are costly to be recreated at + * every usage like the {@link BufferRecycler}. It is intended for + * internal use only. + * + * @since 2.16 + */ public interface ObjectPool extends AutoCloseable { T acquire(); @@ -21,7 +31,22 @@ default void withPooledObject(Consumer objectConsumer) { } enum Strategy { - CONCURRENT_DEQUEUE, LOCK_FREE + CONCURRENT_DEQUEUE(ConcurrentDequePool::new, false), LOCK_FREE(LockFreePool::new, false), + DEBUG_CONCURRENT_DEQUEUE(ConcurrentDequePool::new, true), DEBUG_LOCK_FREE(LockFreePool::new, true); + + private final Function constructor; + + private final boolean debug; + + Strategy(Function constructor, boolean debug) { + this.constructor = constructor; + this.debug = debug; + } + + ObjectPool newObjectPool(Supplier factory) { + ObjectPool pool = constructor.apply(factory); + return debug ? new DebugPoolDecorator<>(pool) : pool; + } } class StrategyHolder { @@ -32,25 +57,21 @@ public static void setStrategy(String name) { } } - static ObjectPool newObjectPool(Function, T> factory) { - switch (StrategyHolder.strategy) { - case CONCURRENT_DEQUEUE: return new ConcurrentDequePool<>(factory); - case LOCK_FREE: return new LockFreePool<>(factory); - } - throw new UnsupportedOperationException(); + static ObjectPool newObjectPool(Supplier factory) { + return StrategyHolder.strategy.newObjectPool(factory); } class ConcurrentDequePool implements ObjectPool { - private final Function, T> factory; + private final Supplier factory; private final Consumer destroyer; private final Deque pool = new ConcurrentLinkedDeque<>(); - public ConcurrentDequePool(Function, T> factory) { + public ConcurrentDequePool(Supplier factory) { this(factory, null); } - public ConcurrentDequePool(Function, T> factory, Consumer destroyer) { + public ConcurrentDequePool(Supplier factory, Consumer destroyer) { this.factory = factory; this.destroyer = destroyer; } @@ -58,7 +79,7 @@ public ConcurrentDequePool(Function, T> factory, Consumer destr @Override public T acquire() { T t = pool.pollFirst(); - return t != null ? t : factory.apply(this); + return t != null ? t : factory.get(); } @Override @@ -77,9 +98,9 @@ public void close() throws Exception { class LockFreePool implements ObjectPool { private final AtomicReference> head = new AtomicReference<>(); - private final Function, T> factory; + private final Supplier factory; - public LockFreePool(Function, T> factory) { + public LockFreePool(Supplier factory) { this.factory = factory; } @@ -88,14 +109,14 @@ public T acquire() { for (int i = 0; i < 3; i++) { Node currentHead = head.get(); if (currentHead == null) { - return factory.apply(this); + return factory.get(); } if (head.compareAndSet(currentHead, currentHead.next)) { currentHead.next = null; return currentHead.value; } } - return factory.apply(this); + return factory.get(); } @Override @@ -123,4 +144,42 @@ static class Node { } } } + + class DebugPoolDecorator implements ObjectPool { + + private final ObjectPool pool; + + private final LongAdder acquireCounter = new LongAdder(); + private final LongAdder releaseCounter = new LongAdder(); + + public DebugPoolDecorator(ObjectPool pool) { + this.pool = pool; + } + + @Override + public T acquire() { + acquireCounter.increment(); + return pool.acquire(); + } + + @Override + public void release(T t) { + releaseCounter.increment(); + pool.release(t); + } + + @Override + public void close() throws Exception { + System.out.println("Closing " + this); + pool.close(); + } + + @Override + public String toString() { + return "DebugPoolDecorator{" + + "acquires = " + acquireCounter.sum() + + ", releases = " + releaseCounter.sum() + + '}'; + } + } } From b9c24c271c581efa5538e3f39517babf6492a455 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Tue, 22 Aug 2023 16:19:50 +0200 Subject: [PATCH 11/29] fix compilation problems after rebase --- .../fasterxml/jackson/core/JsonFactory.java | 23 ++++++++++++++++++- .../fasterxml/jackson/core/io/IOContext.java | 14 +++++------ .../jackson/core/util/BufferRecyclerPool.java | 4 +--- .../jackson/core/util/BufferRecyclers.java | 4 ++-- .../write/WriterBasedJsonGeneratorTest.java | 5 ++-- 5 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java index b732b835d9..8648022611 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java @@ -2164,7 +2164,28 @@ protected JsonGenerator _decorate(JsonGenerator g) { */ public BufferRecycler _getBufferRecycler() { - return _getBufferRecyclerPool().acquireBufferRecycler(this); + return _getBufferRecyclerPool().acquireBufferRecycler(); + } + + public static BufferRecycler _getDefaultBufferRecycler() + { + return BufferRecyclers.defaultRecyclerPool().acquireBufferRecycler(); + } + + /** + * Accessor for getting access to {@link BufferRecyclerPool} for getting + * {@link BufferRecycler} instance to use. + * + * @since 2.16 + */ + public BufferRecyclerPool _getBufferRecyclerPool() { + // 23-Apr-2015, tatu: Let's allow disabling of buffer recycling + // scheme, for cases where it is considered harmful (possibly + // on Android, for example) + if (!Feature.USE_THREAD_LOCAL_FOR_BUFFER_RECYCLING.enabledIn(_factoryFeatures)) { + return BufferRecyclers.nopRecyclerPool(); + } + return _bufferRecyclerPool; } /** diff --git a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java index 49709cd32b..4bcaf8d961 100644 --- a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java +++ b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java @@ -2,6 +2,7 @@ import com.fasterxml.jackson.core.ErrorReportConfiguration; import com.fasterxml.jackson.core.JsonEncoding; +import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.StreamReadConstraints; import com.fasterxml.jackson.core.StreamWriteConstraints; import com.fasterxml.jackson.core.util.BufferRecycler; @@ -142,27 +143,26 @@ public class IOContext implements AutoCloseable public IOContext(StreamReadConstraints src, StreamWriteConstraints swc, ErrorReportConfiguration erc, ContentReference contentRef, boolean managedResource) { - this(src, swc, erc, BufferRecyclerPool.acquireBufferRecycler(), contentRef, managedResource); + this(src, swc, erc, JsonFactory._getDefaultBufferRecycler(), contentRef, managedResource); } /** + * Main constructor to use. + * * @param src constraints for streaming reads * @param swc constraints for streaming writes - * @param erc Error report configuration to use * @param br BufferRecycler to use, if any ({@code null} if none) * @param contentRef Input source reference for location reporting * @param managedResource Whether input source is managed (owned) by Jackson library + * @param erc Error report configuration to use * * @since 2.16 */ - @Deprecated public IOContext(StreamReadConstraints src, StreamWriteConstraints swc, ErrorReportConfiguration erc, BufferRecycler br, ContentReference contentRef, boolean managedResource) { - _streamReadConstraints = (src == null) ? - StreamReadConstraints.defaults() : src; - _streamWriteConstraints = (swc == null) ? - StreamWriteConstraints.defaults() : swc; + _streamReadConstraints = src; + _streamWriteConstraints = swc; _errorReportConfiguration = erc; _bufferRecycler = br; _contentReference = contentRef; diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java index cd2276b0c2..2f372e711f 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java @@ -1,7 +1,5 @@ package com.fasterxml.jackson.core.util; -import com.fasterxml.jackson.core.TokenStreamFactory; - /** * Interface for entity that controls creation and possible reuse of {@link BufferRecycler} * instances used for recycling of underlying input/output buffers. @@ -11,7 +9,7 @@ public interface BufferRecyclerPool extends java.io.Serializable { - public BufferRecycler acquireBufferRecycler(TokenStreamFactory forFactory); + public BufferRecycler acquireBufferRecycler(); public void releaseBufferRecycler(BufferRecycler recycler); } diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java index c7cbfd025a..62fcecf025 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java @@ -214,7 +214,7 @@ public static class ThreadLocalRecyclerPool public final static ThreadLocalRecyclerPool INSTANCE = new ThreadLocalRecyclerPool(); @Override - public BufferRecycler acquireBufferRecycler(TokenStreamFactory forFactory) { + public BufferRecycler acquireBufferRecycler() { return getBufferRecycler(); } @@ -238,7 +238,7 @@ public static class NonRecyclingRecyclerPool public final static ThreadLocalRecyclerPool INSTANCE = new ThreadLocalRecyclerPool(); @Override - public BufferRecycler acquireBufferRecycler(TokenStreamFactory forFactory) { + public BufferRecycler acquireBufferRecycler() { return new BufferRecycler(); } diff --git a/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java b/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java index 3da2dd4d56..47fdc9035c 100644 --- a/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java +++ b/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java @@ -1,7 +1,5 @@ package com.fasterxml.jackson.core.write; -import java.io.StringWriter; - import com.fasterxml.jackson.core.BaseTest; import com.fasterxml.jackson.core.ErrorReportConfiguration; import com.fasterxml.jackson.core.JsonGenerator; @@ -11,7 +9,8 @@ import com.fasterxml.jackson.core.io.ContentReference; import com.fasterxml.jackson.core.io.IOContext; import com.fasterxml.jackson.core.json.WriterBasedJsonGenerator; -import com.fasterxml.jackson.core.util.BufferRecycler; + +import java.io.StringWriter; public class WriterBasedJsonGeneratorTest extends BaseTest { From 44e446bcd12ce70d7b7de721d1d806202267f99f Mon Sep 17 00:00:00 2001 From: mariofusco Date: Tue, 22 Aug 2023 18:29:26 +0200 Subject: [PATCH 12/29] readd virtual threads friendly BufferRecyclerPool implementations --- .../fasterxml/jackson/core/JsonFactory.java | 59 +++--- .../fasterxml/jackson/core/TSFBuilder.java | 2 +- .../fasterxml/jackson/core/io/IOContext.java | 2 +- .../jackson/core/util/BufferRecycler.java | 13 +- .../jackson/core/util/BufferRecyclerPool.java | 152 +++++++++++++- .../jackson/core/util/BufferRecyclers.java | 62 ------ .../jackson/core/util/ObjectPool.java | 185 ------------------ 7 files changed, 189 insertions(+), 286 deletions(-) delete mode 100644 src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java diff --git a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java index 8648022611..52ff6d4461 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java @@ -4,28 +4,49 @@ */ package com.fasterxml.jackson.core; -import java.io.*; -import java.lang.ref.SoftReference; -import java.net.URL; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; - import com.fasterxml.jackson.core.format.InputAccessor; import com.fasterxml.jackson.core.format.MatchStrength; -import com.fasterxml.jackson.core.io.*; -import com.fasterxml.jackson.core.json.*; +import com.fasterxml.jackson.core.io.CharacterEscapes; +import com.fasterxml.jackson.core.io.ContentReference; +import com.fasterxml.jackson.core.io.IOContext; +import com.fasterxml.jackson.core.io.InputDecorator; +import com.fasterxml.jackson.core.io.OutputDecorator; +import com.fasterxml.jackson.core.io.SerializedString; +import com.fasterxml.jackson.core.io.UTF8Writer; +import com.fasterxml.jackson.core.json.ByteSourceJsonBootstrapper; +import com.fasterxml.jackson.core.json.JsonWriteFeature; +import com.fasterxml.jackson.core.json.PackageVersion; +import com.fasterxml.jackson.core.json.ReaderBasedJsonParser; +import com.fasterxml.jackson.core.json.UTF8DataInputJsonParser; +import com.fasterxml.jackson.core.json.UTF8JsonGenerator; +import com.fasterxml.jackson.core.json.WriterBasedJsonGenerator; import com.fasterxml.jackson.core.json.async.NonBlockingByteBufferJsonParser; import com.fasterxml.jackson.core.json.async.NonBlockingJsonParser; import com.fasterxml.jackson.core.sym.ByteQuadsCanonicalizer; import com.fasterxml.jackson.core.sym.CharsToNameCanonicalizer; import com.fasterxml.jackson.core.util.BufferRecycler; import com.fasterxml.jackson.core.util.BufferRecyclerPool; -import com.fasterxml.jackson.core.util.BufferRecyclers; import com.fasterxml.jackson.core.util.JacksonFeature; import com.fasterxml.jackson.core.util.JsonGeneratorDecorator; import com.fasterxml.jackson.core.util.Separators; +import java.io.CharArrayReader; +import java.io.DataInput; +import java.io.DataOutput; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.io.Reader; +import java.io.StringReader; +import java.io.Writer; +import java.lang.ref.SoftReference; +import java.net.URL; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; + /** * The main factory class of Jackson package, used to configure and * construct @@ -131,18 +152,6 @@ public enum Feature */ USE_THREAD_LOCAL_FOR_BUFFER_RECYCLING(true), - /** - * Feature that determines whether we will use {@link BufferRecycler} with - * an object pool not making use of a {@link ThreadLocal}, for efficient reuse of - * underlying input/output buffers. - * This is suggested especially in environments making use of virtual threads. - *

- * This setting is disabled by default. If enabled it overrides USE_THREAD_LOCAL_FOR_BUFFER_RECYCLING. - * - * @since 2.16 - */ - USE_OBJECT_POOL_FOR_BUFFER_RECYCLING(false), - /** * Feature to control charset detection for byte-based inputs ({@code byte[]}, {@link InputStream}...). * When this feature is enabled (the default), the factory will allow UTF-16 and UTF-32 inputs and try to detect @@ -382,7 +391,7 @@ public static int collectDefaults() { public JsonFactory() { this((ObjectCodec) null); } public JsonFactory(ObjectCodec oc) { - _bufferRecyclerPool = BufferRecyclers.defaultRecyclerPool(); + _bufferRecyclerPool = BufferRecyclerPool.defaultRecyclerPool(); _objectCodec = oc; _quoteChar = DEFAULT_QUOTE_CHAR; _streamReadConstraints = StreamReadConstraints.defaults(); @@ -2169,7 +2178,7 @@ public BufferRecycler _getBufferRecycler() public static BufferRecycler _getDefaultBufferRecycler() { - return BufferRecyclers.defaultRecyclerPool().acquireBufferRecycler(); + return BufferRecyclerPool.defaultRecyclerPool().acquireBufferRecycler(); } /** @@ -2183,7 +2192,7 @@ public BufferRecyclerPool _getBufferRecyclerPool() { // scheme, for cases where it is considered harmful (possibly // on Android, for example) if (!Feature.USE_THREAD_LOCAL_FOR_BUFFER_RECYCLING.enabledIn(_factoryFeatures)) { - return BufferRecyclers.nopRecyclerPool(); + return BufferRecyclerPool.PoolStrategy.NO_OP.getPool(); } return _bufferRecyclerPool; } diff --git a/src/main/java/com/fasterxml/jackson/core/TSFBuilder.java b/src/main/java/com/fasterxml/jackson/core/TSFBuilder.java index f34c655c31..02cf0d7f29 100644 --- a/src/main/java/com/fasterxml/jackson/core/TSFBuilder.java +++ b/src/main/java/com/fasterxml/jackson/core/TSFBuilder.java @@ -142,7 +142,7 @@ protected TSFBuilder(JsonFactory base) protected TSFBuilder(int factoryFeatures, int parserFeatures, int generatorFeatures) { - _bufferRecyclerPool = BufferRecyclers.defaultRecyclerPool(); + _bufferRecyclerPool = BufferRecyclerPool.defaultRecyclerPool(); _factoryFeatures = factoryFeatures; _streamReadFeatures = parserFeatures; diff --git a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java index 4bcaf8d961..e3aefb23ff 100644 --- a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java +++ b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java @@ -483,7 +483,7 @@ private IllegalArgumentException wrongBuf() { @Override public void close() { if (!_closed) { - _bufferRecycler.close(); + _bufferRecycler.release(); _closed = true; } } diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java index 4c8843610f..3c1ef0cc76 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java @@ -13,7 +13,7 @@ * Rewritten in 2.10 to be thread-safe (see [jackson-core#479] for details), * to not rely on {@code ThreadLocal} access. */ -public class BufferRecycler implements AutoCloseable +public class BufferRecycler { /** * Buffer used for reading byte-based input. @@ -82,7 +82,7 @@ public class BufferRecycler implements AutoCloseable // Note: changed from simple array in 2.10: protected final AtomicReferenceArray _charBuffers; - private ObjectPool _pool; + private BufferRecyclerPool _pool; /* /********************************************************** @@ -192,7 +192,7 @@ protected int charBufferLength(int ix) { protected byte[] balloc(int size) { return new byte[size]; } protected char[] calloc(int size) { return new char[size]; } - BufferRecycler withPool(ObjectPool pool) { + BufferRecycler withPool(BufferRecyclerPool pool) { if (this._pool != null) { throw new IllegalStateException(); } @@ -200,12 +200,11 @@ BufferRecycler withPool(ObjectPool pool) { return this; } - @Override - public void close() { + public void release() { if (_pool != null) { - ObjectPool tempPool = _pool; + BufferRecyclerPool tempPool = _pool; _pool = null; - tempPool.release(this); + tempPool.releaseBufferRecycler(this); } } } diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java index 2f372e711f..e6112028ab 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java @@ -1,15 +1,157 @@ package com.fasterxml.jackson.core.util; +import java.io.Serializable; +import java.util.Deque; +import java.util.concurrent.ConcurrentLinkedDeque; +import java.util.concurrent.atomic.AtomicReference; + /** * Interface for entity that controls creation and possible reuse of {@link BufferRecycler} * instances used for recycling of underlying input/output buffers. * * @since 2.16 */ -public interface BufferRecyclerPool - extends java.io.Serializable -{ - public BufferRecycler acquireBufferRecycler(); +public interface BufferRecyclerPool extends Serializable { + + BufferRecycler acquireBufferRecycler(); + + void releaseBufferRecycler(BufferRecycler recycler); + + /** + * Returns the default BufferRecyclerPool implementation which is the thread local based one. + */ + static BufferRecyclerPool defaultRecyclerPool() { + return PoolStrategy.THREAD_LOCAL.getPool(); + } + + enum PoolStrategy { + NO_OP(new NonRecyclingRecyclerPool()), + THREAD_LOCAL(new ThreadLocalRecyclerPool()), + CONCURRENT_DEQUEUE(new ConcurrentDequePool()), + LOCK_FREE(new LockFreePool()); + + private final BufferRecyclerPool pool; + + PoolStrategy(BufferRecyclerPool pool) { + this.pool = pool; + } + + public BufferRecyclerPool getPool() { + return pool; + } + } + + /** + * Default {@link BufferRecyclerPool} implementation that uses + * {@link ThreadLocal} for recycling instances. + * + * @since 2.16 + */ + class ThreadLocalRecyclerPool implements BufferRecyclerPool { + private static final long serialVersionUID = 1L; + + @Override + public BufferRecycler acquireBufferRecycler() { + return BufferRecyclers.getBufferRecycler(); + } + + @Override + public void releaseBufferRecycler(BufferRecycler recycler) { + ; // nothing to do, relies on ThreadLocal + } + } + + /** + * {@link BufferRecyclerPool} implementation that does not use + * any pool but simply creates new instances when necessary. + * + * @since 2.16 + */ + class NonRecyclingRecyclerPool implements BufferRecyclerPool { + private static final long serialVersionUID = 1L; + + @Override + public BufferRecycler acquireBufferRecycler() { + return new BufferRecycler(); + } + + @Override + public void releaseBufferRecycler(BufferRecycler recycler) { + ; // nothing to do, relies on ThreadLocal + } + } + + /** + * {@link BufferRecyclerPool} implementation that uses + * {@link ConcurrentLinkedDeque} for recycling instances. + * + * @since 2.16 + */ + class ConcurrentDequePool implements BufferRecyclerPool { + private final Deque pool = new ConcurrentLinkedDeque<>(); + + @Override + public BufferRecycler acquireBufferRecycler() { + return getBufferRecycler().withPool(this); + } + + private BufferRecycler getBufferRecycler() { + BufferRecycler bufferRecycler = pool.pollFirst(); + return bufferRecycler != null ? bufferRecycler : new BufferRecycler(); + } + + @Override + public void releaseBufferRecycler(BufferRecycler bufferRecycler) { + pool.offerLast(bufferRecycler); + } + } + + /** + * {@link BufferRecyclerPool} implementation that uses + * a lock free linked list for recycling instances. + * + * @since 2.16 + */ + class LockFreePool implements BufferRecyclerPool { + private final AtomicReference> head = new AtomicReference<>(); + + @Override + public BufferRecycler acquireBufferRecycler() { + return getBufferRecycler().withPool(this); + } + + private BufferRecycler getBufferRecycler() { + for (int i = 0; i < 3; i++) { + Node currentHead = head.get(); + if (currentHead == null) { + return new BufferRecycler(); + } + if (head.compareAndSet(currentHead, currentHead.next)) { + currentHead.next = null; + return currentHead.value; + } + } + return new BufferRecycler(); + } + + @Override + public void releaseBufferRecycler(BufferRecycler bufferRecycler) { + LockFreePool.Node newHead = new LockFreePool.Node<>(bufferRecycler); + for (int i = 0; i < 3; i++) { + newHead.next = head.get(); + if (head.compareAndSet(newHead.next, newHead)) { + return; + } + } + } + + static class Node { + final T value; + LockFreePool.Node next; - public void releaseBufferRecycler(BufferRecycler recycler); + Node(T value) { + this.value = value; + } + } + } } diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java index 62fcecf025..994f4a6e4d 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java @@ -185,66 +185,4 @@ public static void quoteAsJsonText(CharSequence input, StringBuilder output) { public static byte[] quoteAsJsonUTF8(String rawText) { return JsonStringEncoder.getInstance().quoteAsUTF8(rawText); } - - /* - /********************************************************************** - /* Default BufferRecyclerPool implementations - /********************************************************************** - */ - - public static BufferRecyclerPool defaultRecyclerPool() { - return ThreadLocalRecyclerPool.INSTANCE; - } - - public static BufferRecyclerPool nopRecyclerPool() { - return NonRecyclingRecyclerPool.INSTANCE; - } - - /** - * Default {@link BufferRecyclerPool} implementation that uses - * {@link ThreadLocal} for recycling instances. - * - * @since 2.16 - */ - public static class ThreadLocalRecyclerPool - implements BufferRecyclerPool - { - private static final long serialVersionUID = 1L; - - public final static ThreadLocalRecyclerPool INSTANCE = new ThreadLocalRecyclerPool(); - - @Override - public BufferRecycler acquireBufferRecycler() { - return getBufferRecycler(); - } - - @Override - public void releaseBufferRecycler(BufferRecycler recycler) { - ; // nothing to do, relies on ThreadLocal - } - } - - /** - * {@link BufferRecyclerPool} implementation that does not use - * any pool but simply creates new instances when necessary. - * - * @since 2.16 - */ - public static class NonRecyclingRecyclerPool - implements BufferRecyclerPool - { - private static final long serialVersionUID = 1L; - - public final static ThreadLocalRecyclerPool INSTANCE = new ThreadLocalRecyclerPool(); - - @Override - public BufferRecycler acquireBufferRecycler() { - return new BufferRecycler(); - } - - @Override - public void releaseBufferRecycler(BufferRecycler recycler) { - ; // nothing to do, relies on ThreadLocal - } - } } diff --git a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java b/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java deleted file mode 100644 index c17d78cc2a..0000000000 --- a/src/main/java/com/fasterxml/jackson/core/util/ObjectPool.java +++ /dev/null @@ -1,185 +0,0 @@ -package com.fasterxml.jackson.core.util; - -import java.util.Deque; -import java.util.concurrent.ConcurrentLinkedDeque; -import java.util.concurrent.atomic.AtomicReference; -import java.util.concurrent.atomic.LongAdder; -import java.util.function.Consumer; -import java.util.function.Function; -import java.util.function.Supplier; - -/** - * This is a utility class, whose main functionality is pooling object - * with a huge memory footprint and that are costly to be recreated at - * every usage like the {@link BufferRecycler}. It is intended for - * internal use only. - * - * @since 2.16 - */ -public interface ObjectPool extends AutoCloseable { - - T acquire(); - void release(T t); - - default void withPooledObject(Consumer objectConsumer) { - T t = acquire(); - try { - objectConsumer.accept(t); - } finally { - release(t); - } - } - - enum Strategy { - CONCURRENT_DEQUEUE(ConcurrentDequePool::new, false), LOCK_FREE(LockFreePool::new, false), - DEBUG_CONCURRENT_DEQUEUE(ConcurrentDequePool::new, true), DEBUG_LOCK_FREE(LockFreePool::new, true); - - private final Function constructor; - - private final boolean debug; - - Strategy(Function constructor, boolean debug) { - this.constructor = constructor; - this.debug = debug; - } - - ObjectPool newObjectPool(Supplier factory) { - ObjectPool pool = constructor.apply(factory); - return debug ? new DebugPoolDecorator<>(pool) : pool; - } - } - - class StrategyHolder { - private static Strategy strategy = Strategy.LOCK_FREE; - - public static void setStrategy(String name) { - strategy = Strategy.valueOf(name); - } - } - - static ObjectPool newObjectPool(Supplier factory) { - return StrategyHolder.strategy.newObjectPool(factory); - } - - class ConcurrentDequePool implements ObjectPool { - private final Supplier factory; - private final Consumer destroyer; - - private final Deque pool = new ConcurrentLinkedDeque<>(); - - public ConcurrentDequePool(Supplier factory) { - this(factory, null); - } - - public ConcurrentDequePool(Supplier factory, Consumer destroyer) { - this.factory = factory; - this.destroyer = destroyer; - } - - @Override - public T acquire() { - T t = pool.pollFirst(); - return t != null ? t : factory.get(); - } - - @Override - public void release(T t) { - pool.offerLast(t); - } - - @Override - public void close() throws Exception { - if (destroyer != null) { - pool.forEach(destroyer); - } - } - } - - class LockFreePool implements ObjectPool { - private final AtomicReference> head = new AtomicReference<>(); - - private final Supplier factory; - - public LockFreePool(Supplier factory) { - this.factory = factory; - } - - @Override - public T acquire() { - for (int i = 0; i < 3; i++) { - Node currentHead = head.get(); - if (currentHead == null) { - return factory.get(); - } - if (head.compareAndSet(currentHead, currentHead.next)) { - currentHead.next = null; - return currentHead.value; - } - } - return factory.get(); - } - - @Override - public void release(T object) { - Node newHead = new Node<>(object); - for (int i = 0; i < 3; i++) { - newHead.next = head.get(); - if (head.compareAndSet(newHead.next, newHead)) { - return; - } - } - } - - @Override - public void close() throws Exception { - - } - - static class Node { - final T value; - Node next; - - Node(T value) { - this.value = value; - } - } - } - - class DebugPoolDecorator implements ObjectPool { - - private final ObjectPool pool; - - private final LongAdder acquireCounter = new LongAdder(); - private final LongAdder releaseCounter = new LongAdder(); - - public DebugPoolDecorator(ObjectPool pool) { - this.pool = pool; - } - - @Override - public T acquire() { - acquireCounter.increment(); - return pool.acquire(); - } - - @Override - public void release(T t) { - releaseCounter.increment(); - pool.release(t); - } - - @Override - public void close() throws Exception { - System.out.println("Closing " + this); - pool.close(); - } - - @Override - public String toString() { - return "DebugPoolDecorator{" + - "acquires = " + acquireCounter.sum() + - ", releases = " + releaseCounter.sum() + - '}'; - } - } -} From 835076c5e924a9c3e649740220c4fd05999ddfd0 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Tue, 22 Aug 2023 18:55:13 +0200 Subject: [PATCH 13/29] add test class --- .../core/write/BufferRecyclerPoolTest.java | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java diff --git a/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java b/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java new file mode 100644 index 0000000000..1906f47e22 --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java @@ -0,0 +1,64 @@ +package com.fasterxml.jackson.core.write; + +import com.fasterxml.jackson.core.BaseTest; +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.util.BufferRecyclerPool; + +import java.io.IOException; +import java.io.OutputStream; + +public class BufferRecyclerPoolTest extends BaseTest { + + public void testNoOp() { + checkBufferRecyclerPoolImpl(BufferRecyclerPool.PoolStrategy.NO_OP); + } + + public void testThreadLocal() { + checkBufferRecyclerPoolImpl(BufferRecyclerPool.PoolStrategy.THREAD_LOCAL); + } + + public void testLockFree() { + checkBufferRecyclerPoolImpl(BufferRecyclerPool.PoolStrategy.LOCK_FREE); + } + + public void testConcurrentDequeue() { + checkBufferRecyclerPoolImpl(BufferRecyclerPool.PoolStrategy.CONCURRENT_DEQUEUE); + } + + private void checkBufferRecyclerPoolImpl(BufferRecyclerPool.PoolStrategy poolStrategy) { + JsonFactory jsonFactory = new JsonFactory().setBufferRecyclerPool(poolStrategy.getPool()); + assertEquals(6, write("test", jsonFactory)); + } + + protected final int write(Object value, JsonFactory jsonFactory) { + NopOutputStream out = new NopOutputStream(); + try (JsonGenerator gen = jsonFactory.createGenerator(out)) { + gen.writeObject(value); + } catch (IOException e) { + throw new RuntimeException(e); + } + return out.size(); + } + + public class NopOutputStream extends OutputStream { + protected int size = 0; + + public NopOutputStream() { } + + @Override + public void write(int b) throws IOException { ++size; } + + @Override + public void write(byte[] b) throws IOException { size += b.length; } + + @Override + public void write(byte[] b, int offset, int len) throws IOException { size += len; } + + public NopOutputStream reset() { + size = 0; + return this; + } + public int size() { return size; } + } +} From 843648b33fe3cd11db9f8262ca46def054b44104 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sat, 26 Aug 2023 23:44:42 -0700 Subject: [PATCH 14/29] ... --- .../jackson/core/write/WriterBasedJsonGeneratorTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java b/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java index 47fdc9035c..acc72f11fb 100644 --- a/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java +++ b/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java @@ -1,5 +1,7 @@ package com.fasterxml.jackson.core.write; +import java.io.StringWriter; + import com.fasterxml.jackson.core.BaseTest; import com.fasterxml.jackson.core.ErrorReportConfiguration; import com.fasterxml.jackson.core.JsonGenerator; @@ -10,8 +12,6 @@ import com.fasterxml.jackson.core.io.IOContext; import com.fasterxml.jackson.core.json.WriterBasedJsonGenerator; -import java.io.StringWriter; - public class WriterBasedJsonGeneratorTest extends BaseTest { public void testNestingDepthWithSmallLimit() throws Exception From 98691fc6edae1a26bc77a54744e85a7084623caf Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sat, 26 Aug 2023 23:50:52 -0700 Subject: [PATCH 15/29] Fix a merge issue --- .../com/fasterxml/jackson/core/write/GeneratorMiscTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/com/fasterxml/jackson/core/write/GeneratorMiscTest.java b/src/test/java/com/fasterxml/jackson/core/write/GeneratorMiscTest.java index c046d75af3..4b5a1bc480 100644 --- a/src/test/java/com/fasterxml/jackson/core/write/GeneratorMiscTest.java +++ b/src/test/java/com/fasterxml/jackson/core/write/GeneratorMiscTest.java @@ -256,8 +256,6 @@ public void testAsEmbedded() throws Exception fail("Expected an exception"); } catch (JsonGenerationException e) { verifyException(e, "No native support for"); - } finally { - g.close(); } } } From 5cf738dd516681e427ac50bd5ab07f74efbed19f Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sun, 27 Aug 2023 00:03:21 -0700 Subject: [PATCH 16/29] ... --- .../jackson/core/write/GeneratorDupHandlingTest.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/test/java/com/fasterxml/jackson/core/write/GeneratorDupHandlingTest.java b/src/test/java/com/fasterxml/jackson/core/write/GeneratorDupHandlingTest.java index 8a83ae38b0..560bae3869 100644 --- a/src/test/java/com/fasterxml/jackson/core/write/GeneratorDupHandlingTest.java +++ b/src/test/java/com/fasterxml/jackson/core/write/GeneratorDupHandlingTest.java @@ -52,8 +52,6 @@ protected void _testSimpleDups(boolean useStream, boolean lazySetting, JsonFacto fail("Should have gotten exception"); } catch (JsonGenerationException e) { verifyException(e, "duplicate field 'a'"); - } finally { - g1.close(); } JsonGenerator g2; @@ -68,8 +66,6 @@ protected void _testSimpleDups(boolean useStream, boolean lazySetting, JsonFacto fail("Should have gotten exception"); } catch (JsonGenerationException e) { verifyException(e, "duplicate field 'x'"); - } finally { - g2.close(); } } From 2e643343b92a00c90dd900e03954d4fca4cef563 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Mon, 28 Aug 2023 10:34:51 +0200 Subject: [PATCH 17/29] wip --- .../jackson/core/write/WriterBasedJsonGeneratorTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java b/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java index acc72f11fb..7724205502 100644 --- a/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java +++ b/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java @@ -48,7 +48,6 @@ private IOContext _ioContext(StreamWriteConstraints swc) { return new IOContext(StreamReadConstraints.defaults(), swc, ErrorReportConfiguration.defaults(), - new BufferRecycler(), ContentReference.unknown(), true); } } From 3375a0bf3fd53c6c9442f54168006cabea5f48e2 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Mon, 28 Aug 2023 11:47:00 +0200 Subject: [PATCH 18/29] fix tests --- .../jackson/core/util/BufferRecyclerPool.java | 16 +++++++--------- .../jackson/core/testsupport/TestSupport.java | 2 +- .../failing/ParserErrorHandling105Test.java | 6 ++---- .../failing/ParserErrorHandling679Test.java | 6 ++---- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java index e6112028ab..84f6f355e7 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java @@ -48,7 +48,6 @@ public BufferRecyclerPool getPool() { * @since 2.16 */ class ThreadLocalRecyclerPool implements BufferRecyclerPool { - private static final long serialVersionUID = 1L; @Override public BufferRecycler acquireBufferRecycler() { @@ -68,7 +67,6 @@ public void releaseBufferRecycler(BufferRecycler recycler) { * @since 2.16 */ class NonRecyclingRecyclerPool implements BufferRecyclerPool { - private static final long serialVersionUID = 1L; @Override public BufferRecycler acquireBufferRecycler() { @@ -113,7 +111,7 @@ public void releaseBufferRecycler(BufferRecycler bufferRecycler) { * @since 2.16 */ class LockFreePool implements BufferRecyclerPool { - private final AtomicReference> head = new AtomicReference<>(); + private final AtomicReference head = new AtomicReference<>(); @Override public BufferRecycler acquireBufferRecycler() { @@ -122,7 +120,7 @@ public BufferRecycler acquireBufferRecycler() { private BufferRecycler getBufferRecycler() { for (int i = 0; i < 3; i++) { - Node currentHead = head.get(); + Node currentHead = head.get(); if (currentHead == null) { return new BufferRecycler(); } @@ -136,7 +134,7 @@ private BufferRecycler getBufferRecycler() { @Override public void releaseBufferRecycler(BufferRecycler bufferRecycler) { - LockFreePool.Node newHead = new LockFreePool.Node<>(bufferRecycler); + LockFreePool.Node newHead = new LockFreePool.Node(bufferRecycler); for (int i = 0; i < 3; i++) { newHead.next = head.get(); if (head.compareAndSet(newHead.next, newHead)) { @@ -145,11 +143,11 @@ public void releaseBufferRecycler(BufferRecycler bufferRecycler) { } } - static class Node { - final T value; - LockFreePool.Node next; + static class Node implements Serializable { + final BufferRecycler value; + LockFreePool.Node next; - Node(T value) { + Node(BufferRecycler value) { this.value = value; } } diff --git a/src/test/java/com/fasterxml/jackson/core/testsupport/TestSupport.java b/src/test/java/com/fasterxml/jackson/core/testsupport/TestSupport.java index e1a9e74dcc..614451b2c8 100644 --- a/src/test/java/com/fasterxml/jackson/core/testsupport/TestSupport.java +++ b/src/test/java/com/fasterxml/jackson/core/testsupport/TestSupport.java @@ -45,6 +45,6 @@ private static IOContext testIOContext(StreamReadConstraints src, StreamWriteConstraints swc, ErrorReportConfiguration erc) { return new IOContext(src, swc, erc, - new BufferRecycler(), ContentReference.unknown(), false); + ContentReference.unknown(), false); } } diff --git a/src/test/java/com/fasterxml/jackson/failing/ParserErrorHandling105Test.java b/src/test/java/com/fasterxml/jackson/failing/ParserErrorHandling105Test.java index f1d924258f..8be962bd80 100644 --- a/src/test/java/com/fasterxml/jackson/failing/ParserErrorHandling105Test.java +++ b/src/test/java/com/fasterxml/jackson/failing/ParserErrorHandling105Test.java @@ -37,15 +37,13 @@ public void testMangledFloatsChars() throws Exception { private void _testMangledNonRootInts(int mode) throws Exception { - JsonParser p = createParser(mode, "[ 123true ]"); - assertToken(JsonToken.START_ARRAY, p.nextToken()); - try { + try (JsonParser p = createParser(mode, "[ 123true ]")) { + assertToken(JsonToken.START_ARRAY, p.nextToken()); JsonToken t = p.nextToken(); fail("Should have gotten an exception; instead got token: "+t); } catch (JsonParseException e) { verifyException(e, "expected space"); } - p.close(); } private void _testMangledNonRootFloats(int mode) throws Exception diff --git a/src/test/java/com/fasterxml/jackson/failing/ParserErrorHandling679Test.java b/src/test/java/com/fasterxml/jackson/failing/ParserErrorHandling679Test.java index 62d37f0f23..617ee38bc5 100644 --- a/src/test/java/com/fasterxml/jackson/failing/ParserErrorHandling679Test.java +++ b/src/test/java/com/fasterxml/jackson/failing/ParserErrorHandling679Test.java @@ -41,16 +41,14 @@ private void _testNonRootMangledFloats679(int mode) throws Exception { private void _testNonRootMangledFloats679(int mode, String value) throws Exception { // Also test with floats - JsonParser p = createParser(mode, "[ "+value+" ]"); - assertEquals(JsonToken.START_ARRAY, p.nextToken()); - try { + try (JsonParser p = createParser(mode, "[ "+value+" ]")) { + assertEquals(JsonToken.START_ARRAY, p.nextToken()); JsonToken t = p.nextToken(); Double v = p.getDoubleValue(); fail("Should have gotten an exception for '"+value+"'; instead got ("+t+") number: "+v); } catch (JsonParseException e) { verifyException(e, "expected "); } - p.close(); } private void _testNonRootMangledInts(int mode) throws Exception { From 643e5e68bd3cd52ff2b552531496492180238af7 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Mon, 28 Aug 2023 12:33:31 +0200 Subject: [PATCH 19/29] add comments --- .../com/fasterxml/jackson/core/util/BufferRecycler.java | 4 ++++ .../com/fasterxml/jackson/core/util/BufferRecyclerPool.java | 6 +++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java index 3c1ef0cc76..d752c49cd0 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java @@ -196,6 +196,8 @@ BufferRecycler withPool(BufferRecyclerPool pool) { if (this._pool != null) { throw new IllegalStateException(); } + // assign to pool to which this BufferRecycler belongs in order to release it + // to the same pool when the work will be completed this._pool = pool; return this; } @@ -203,6 +205,8 @@ BufferRecycler withPool(BufferRecyclerPool pool) { public void release() { if (_pool != null) { BufferRecyclerPool tempPool = _pool; + // nullify the reference to the pool in order to avoid the risk of releasing + // the same BufferRecycler more than once, thus compromising the pool integrity _pool = null; tempPool.releaseBufferRecycler(this); } diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java index 84f6f355e7..baba0bacab 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java @@ -75,7 +75,7 @@ public BufferRecycler acquireBufferRecycler() { @Override public void releaseBufferRecycler(BufferRecycler recycler) { - ; // nothing to do, relies on ThreadLocal + ; // nothing to do, there is no underlying pool } } @@ -119,6 +119,10 @@ public BufferRecycler acquireBufferRecycler() { } private BufferRecycler getBufferRecycler() { + // This simple lock free algorithm uses an optimistic compareAndSet strategy to + // populate the underlying linked list in a thread-safe way. However, under very + // heavy contention, the compareAndSet could fail multiple times, so it seems a + // reasonable heuristic to limit the number of retries in this situation. for (int i = 0; i < 3; i++) { Node currentHead = head.get(); if (currentHead == null) { From 454018cee2e96ea33cbabff1c83a0e67f187b119 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Mon, 28 Aug 2023 15:21:27 +0200 Subject: [PATCH 20/29] remove enum of pool implementations --- .../fasterxml/jackson/core/JsonFactory.java | 2 +- .../jackson/core/util/BufferRecyclerPool.java | 29 +++++++------------ .../core/write/BufferRecyclerPoolTest.java | 12 ++++---- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java index 52ff6d4461..177a91bfb7 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java @@ -2192,7 +2192,7 @@ public BufferRecyclerPool _getBufferRecyclerPool() { // scheme, for cases where it is considered harmful (possibly // on Android, for example) if (!Feature.USE_THREAD_LOCAL_FOR_BUFFER_RECYCLING.enabledIn(_factoryFeatures)) { - return BufferRecyclerPool.PoolStrategy.NO_OP.getPool(); + return BufferRecyclerPool.NonRecyclingRecyclerPool.INSTANCE; } return _bufferRecyclerPool; } diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java index baba0bacab..c4e999fe2b 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java @@ -21,24 +21,7 @@ public interface BufferRecyclerPool extends Serializable { * Returns the default BufferRecyclerPool implementation which is the thread local based one. */ static BufferRecyclerPool defaultRecyclerPool() { - return PoolStrategy.THREAD_LOCAL.getPool(); - } - - enum PoolStrategy { - NO_OP(new NonRecyclingRecyclerPool()), - THREAD_LOCAL(new ThreadLocalRecyclerPool()), - CONCURRENT_DEQUEUE(new ConcurrentDequePool()), - LOCK_FREE(new LockFreePool()); - - private final BufferRecyclerPool pool; - - PoolStrategy(BufferRecyclerPool pool) { - this.pool = pool; - } - - public BufferRecyclerPool getPool() { - return pool; - } + return ThreadLocalRecyclerPool.INSTANCE; } /** @@ -49,6 +32,8 @@ public BufferRecyclerPool getPool() { */ class ThreadLocalRecyclerPool implements BufferRecyclerPool { + public static final BufferRecyclerPool INSTANCE = new ThreadLocalRecyclerPool(); + @Override public BufferRecycler acquireBufferRecycler() { return BufferRecyclers.getBufferRecycler(); @@ -68,6 +53,8 @@ public void releaseBufferRecycler(BufferRecycler recycler) { */ class NonRecyclingRecyclerPool implements BufferRecyclerPool { + public static final BufferRecyclerPool INSTANCE = new NonRecyclingRecyclerPool(); + @Override public BufferRecycler acquireBufferRecycler() { return new BufferRecycler(); @@ -86,6 +73,9 @@ public void releaseBufferRecycler(BufferRecycler recycler) { * @since 2.16 */ class ConcurrentDequePool implements BufferRecyclerPool { + + public static final BufferRecyclerPool INSTANCE = new ConcurrentDequePool(); + private final Deque pool = new ConcurrentLinkedDeque<>(); @Override @@ -111,6 +101,9 @@ public void releaseBufferRecycler(BufferRecycler bufferRecycler) { * @since 2.16 */ class LockFreePool implements BufferRecyclerPool { + + public static final BufferRecyclerPool INSTANCE = new LockFreePool(); + private final AtomicReference head = new AtomicReference<>(); @Override diff --git a/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java b/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java index 1906f47e22..2ee072bd7e 100644 --- a/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java +++ b/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java @@ -11,23 +11,23 @@ public class BufferRecyclerPoolTest extends BaseTest { public void testNoOp() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.PoolStrategy.NO_OP); + checkBufferRecyclerPoolImpl(BufferRecyclerPool.NonRecyclingRecyclerPool.INSTANCE); } public void testThreadLocal() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.PoolStrategy.THREAD_LOCAL); + checkBufferRecyclerPoolImpl(BufferRecyclerPool.ThreadLocalRecyclerPool.INSTANCE); } public void testLockFree() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.PoolStrategy.LOCK_FREE); + checkBufferRecyclerPoolImpl(BufferRecyclerPool.LockFreePool.INSTANCE); } public void testConcurrentDequeue() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.PoolStrategy.CONCURRENT_DEQUEUE); + checkBufferRecyclerPoolImpl(BufferRecyclerPool.ConcurrentDequePool.INSTANCE); } - private void checkBufferRecyclerPoolImpl(BufferRecyclerPool.PoolStrategy poolStrategy) { - JsonFactory jsonFactory = new JsonFactory().setBufferRecyclerPool(poolStrategy.getPool()); + private void checkBufferRecyclerPoolImpl(BufferRecyclerPool pool) { + JsonFactory jsonFactory = new JsonFactory().setBufferRecyclerPool(pool); assertEquals(6, write("test", jsonFactory)); } From 559413605ae70c1803a8d1d890fc11ca421faec4 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 29 Aug 2023 17:57:30 -0700 Subject: [PATCH 21/29] Minor tweaks: add javadoc, reduce diff from 2.16 --- .../fasterxml/jackson/core/JsonFactory.java | 46 ++++--------------- .../fasterxml/jackson/core/io/IOContext.java | 5 +- .../jackson/core/util/BufferRecycler.java | 34 ++++++++++---- 3 files changed, 37 insertions(+), 48 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java index 177a91bfb7..8662e3b4a4 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java @@ -4,48 +4,22 @@ */ package com.fasterxml.jackson.core; +import java.io.*; +import java.lang.ref.SoftReference; +import java.net.URL; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; + import com.fasterxml.jackson.core.format.InputAccessor; import com.fasterxml.jackson.core.format.MatchStrength; -import com.fasterxml.jackson.core.io.CharacterEscapes; -import com.fasterxml.jackson.core.io.ContentReference; -import com.fasterxml.jackson.core.io.IOContext; -import com.fasterxml.jackson.core.io.InputDecorator; -import com.fasterxml.jackson.core.io.OutputDecorator; -import com.fasterxml.jackson.core.io.SerializedString; -import com.fasterxml.jackson.core.io.UTF8Writer; -import com.fasterxml.jackson.core.json.ByteSourceJsonBootstrapper; -import com.fasterxml.jackson.core.json.JsonWriteFeature; -import com.fasterxml.jackson.core.json.PackageVersion; -import com.fasterxml.jackson.core.json.ReaderBasedJsonParser; -import com.fasterxml.jackson.core.json.UTF8DataInputJsonParser; -import com.fasterxml.jackson.core.json.UTF8JsonGenerator; -import com.fasterxml.jackson.core.json.WriterBasedJsonGenerator; +import com.fasterxml.jackson.core.io.*; +import com.fasterxml.jackson.core.json.*; import com.fasterxml.jackson.core.json.async.NonBlockingByteBufferJsonParser; import com.fasterxml.jackson.core.json.async.NonBlockingJsonParser; import com.fasterxml.jackson.core.sym.ByteQuadsCanonicalizer; import com.fasterxml.jackson.core.sym.CharsToNameCanonicalizer; -import com.fasterxml.jackson.core.util.BufferRecycler; -import com.fasterxml.jackson.core.util.BufferRecyclerPool; -import com.fasterxml.jackson.core.util.JacksonFeature; -import com.fasterxml.jackson.core.util.JsonGeneratorDecorator; -import com.fasterxml.jackson.core.util.Separators; - -import java.io.CharArrayReader; -import java.io.DataInput; -import java.io.DataOutput; -import java.io.File; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.io.OutputStreamWriter; -import java.io.Reader; -import java.io.StringReader; -import java.io.Writer; -import java.lang.ref.SoftReference; -import java.net.URL; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; +import com.fasterxml.jackson.core.util.*; /** * The main factory class of Jackson package, used to configure and diff --git a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java index e3aefb23ff..40a94b3480 100644 --- a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java +++ b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java @@ -8,7 +8,6 @@ import com.fasterxml.jackson.core.util.BufferRecycler; import com.fasterxml.jackson.core.util.ReadConstrainedTextBuffer; import com.fasterxml.jackson.core.util.TextBuffer; -import com.fasterxml.jackson.core.util.BufferRecyclerPool; /** * To limit number of configuration and state objects to pass, all @@ -159,7 +158,7 @@ public IOContext(StreamReadConstraints src, StreamWriteConstraints swc, ErrorRep * @since 2.16 */ public IOContext(StreamReadConstraints src, StreamWriteConstraints swc, ErrorReportConfiguration erc, - BufferRecycler br, ContentReference contentRef, boolean managedResource) + BufferRecycler br, ContentReference contentRef, boolean managedResource) { _streamReadConstraints = src; _streamWriteConstraints = swc; @@ -169,7 +168,7 @@ public IOContext(StreamReadConstraints src, StreamWriteConstraints swc, ErrorRep _sourceRef = contentRef.getRawContent(); _managedResource = managedResource; } - + /** * Deprecated legacy constructor. * diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java index d752c49cd0..136d9498df 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java @@ -1,17 +1,21 @@ package com.fasterxml.jackson.core.util; +import java.util.Objects; import java.util.concurrent.atomic.AtomicReferenceArray; /** * This is a small utility class, whose main functionality is to allow - * simple reuse of raw byte/char buffers. It is usually used through - * ThreadLocal member of the owning class pointing to - * instance of this class through a SoftReference. The - * end result is a low-overhead GC-cleanable recycling: hopefully + * simple reuse of raw byte/char buffers. It is usually allocated through + * {@link BufferRecyclerPool} (starting with 2.16): multiple pool + * implementations exists. + * The default pool implementation uses + * {@code ThreadLocal} combined with {@code SoftReference}. + * The end result is a low-overhead GC-cleanable recycling: hopefully * ideal for use by stream readers. *

* Rewritten in 2.10 to be thread-safe (see [jackson-core#479] for details), - * to not rely on {@code ThreadLocal} access. + * to not rely on {@code ThreadLocal} access.
+ * Rewritten in 2.16 to work with {@link BufferRecyclerPool} abstraction. */ public class BufferRecycler { @@ -192,23 +196,35 @@ protected int charBufferLength(int ix) { protected byte[] balloc(int size) { return new byte[size]; } protected char[] calloc(int size) { return new char[size]; } + /** + * Method called by owner of this recycler instance, to provide reference to + * {@link BufferRecyclerPool} into which instance is to be released (if any) + * + * @since 2.16 + */ BufferRecycler withPool(BufferRecyclerPool pool) { if (this._pool != null) { - throw new IllegalStateException(); + throw new IllegalStateException("BufferRecycler already linked to pool: "+pool); } // assign to pool to which this BufferRecycler belongs in order to release it // to the same pool when the work will be completed - this._pool = pool; + _pool = Objects.requireNonNull(pool); return this; } + /** + * Method called when owner of this recycler no longer wishes use it; this should + * return it to pool passed via {@code withPool()} (if any). + * + * @since 2.16 + */ public void release() { if (_pool != null) { - BufferRecyclerPool tempPool = _pool; + BufferRecyclerPool tmpPool = _pool; // nullify the reference to the pool in order to avoid the risk of releasing // the same BufferRecycler more than once, thus compromising the pool integrity _pool = null; - tempPool.releaseBufferRecycler(this); + tmpPool.releaseBufferRecycler(this); } } } From e3a0971d223675b9b5d8a1ea426da7cec44e45fa Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 29 Aug 2023 18:05:57 -0700 Subject: [PATCH 22/29] Bit more cleanup; remove addition of default recycler pool access via JsonFactory --- .../fasterxml/jackson/core/JsonFactory.java | 15 ++++++++------- .../fasterxml/jackson/core/io/IOContext.java | 18 ------------------ .../jackson/core/io/TestIOContext.java | 1 + .../jackson/core/testsupport/TestSupport.java | 1 + .../util/ReadConstrainedTextBufferTest.java | 1 + .../write/WriterBasedJsonGeneratorTest.java | 2 ++ 6 files changed, 13 insertions(+), 25 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java index 8662e3b4a4..ec9f340ce2 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java @@ -110,8 +110,10 @@ public enum Feature FAIL_ON_SYMBOL_HASH_OVERFLOW(true), /** - * Feature that determines whether we will use {@link BufferRecycler} with - * {@link ThreadLocal} and {@link SoftReference}, for efficient reuse of + * Feature that determines whether we will use a {@link BufferRecyclerPool} + * for allocating and possibly recycling {@link BufferRecycler} or not. + * The default {@link BufferRecyclerPool} implementation uses + * {@link ThreadLocal} and {@link SoftReference} for efficient reuse of * underlying input/output buffers. * This usually makes sense on normal J2SE/J2EE server-side processing; * but may not make sense on platforms where {@link SoftReference} handling @@ -120,6 +122,10 @@ public enum Feature * jackson-core#189 * for a possible case) *

+ * Note that since 2.16 naming here is somewhat misleading as this is used + * to now enable or disable pooling; but the actual pooling implementation + * is configurable and may not be based on {@link ThreadLocal}. + *

* This setting is enabled by default. * * @since 2.6 @@ -2150,11 +2156,6 @@ public BufferRecycler _getBufferRecycler() return _getBufferRecyclerPool().acquireBufferRecycler(); } - public static BufferRecycler _getDefaultBufferRecycler() - { - return BufferRecyclerPool.defaultRecyclerPool().acquireBufferRecycler(); - } - /** * Accessor for getting access to {@link BufferRecyclerPool} for getting * {@link BufferRecycler} instance to use. diff --git a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java index 40a94b3480..56d0ec1eaf 100644 --- a/src/main/java/com/fasterxml/jackson/core/io/IOContext.java +++ b/src/main/java/com/fasterxml/jackson/core/io/IOContext.java @@ -2,7 +2,6 @@ import com.fasterxml.jackson.core.ErrorReportConfiguration; import com.fasterxml.jackson.core.JsonEncoding; -import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.StreamReadConstraints; import com.fasterxml.jackson.core.StreamWriteConstraints; import com.fasterxml.jackson.core.util.BufferRecycler; @@ -128,23 +127,6 @@ public class IOContext implements AutoCloseable /********************************************************************** */ - /** - * Main constructor to use. - * - * @param src constraints for streaming reads - * @param swc constraints for streaming writes - * @param erc Error report configuration to use - * @param contentRef Input source reference for location reporting - * @param managedResource Whether input source is managed (owned) by Jackson library - * - * @since 2.16 - */ - public IOContext(StreamReadConstraints src, StreamWriteConstraints swc, ErrorReportConfiguration erc, - ContentReference contentRef, boolean managedResource) - { - this(src, swc, erc, JsonFactory._getDefaultBufferRecycler(), contentRef, managedResource); - } - /** * Main constructor to use. * diff --git a/src/test/java/com/fasterxml/jackson/core/io/TestIOContext.java b/src/test/java/com/fasterxml/jackson/core/io/TestIOContext.java index 328a76ed37..ebd0b1d272 100644 --- a/src/test/java/com/fasterxml/jackson/core/io/TestIOContext.java +++ b/src/test/java/com/fasterxml/jackson/core/io/TestIOContext.java @@ -13,6 +13,7 @@ public void testAllocations() throws Exception IOContext ctxt = new IOContext(StreamReadConstraints.defaults(), StreamWriteConstraints.defaults(), ErrorReportConfiguration.defaults(), + new BufferRecycler(), ContentReference.rawReference("N/A"), true); /* I/O Read buffer */ diff --git a/src/test/java/com/fasterxml/jackson/core/testsupport/TestSupport.java b/src/test/java/com/fasterxml/jackson/core/testsupport/TestSupport.java index 614451b2c8..8b4afd0492 100644 --- a/src/test/java/com/fasterxml/jackson/core/testsupport/TestSupport.java +++ b/src/test/java/com/fasterxml/jackson/core/testsupport/TestSupport.java @@ -45,6 +45,7 @@ private static IOContext testIOContext(StreamReadConstraints src, StreamWriteConstraints swc, ErrorReportConfiguration erc) { return new IOContext(src, swc, erc, + new BufferRecycler(), ContentReference.unknown(), false); } } diff --git a/src/test/java/com/fasterxml/jackson/core/util/ReadConstrainedTextBufferTest.java b/src/test/java/com/fasterxml/jackson/core/util/ReadConstrainedTextBufferTest.java index 0801b1fdaf..cfb163cff1 100644 --- a/src/test/java/com/fasterxml/jackson/core/util/ReadConstrainedTextBufferTest.java +++ b/src/test/java/com/fasterxml/jackson/core/util/ReadConstrainedTextBufferTest.java @@ -64,6 +64,7 @@ private static IOContext makeConstrainedContext(int maxStringLen) { constraints, StreamWriteConstraints.defaults(), ErrorReportConfiguration.defaults(), + new BufferRecycler(), ContentReference.rawReference("N/A"), true); } } \ No newline at end of file diff --git a/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java b/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java index 7724205502..3da2dd4d56 100644 --- a/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java +++ b/src/test/java/com/fasterxml/jackson/core/write/WriterBasedJsonGeneratorTest.java @@ -11,6 +11,7 @@ import com.fasterxml.jackson.core.io.ContentReference; import com.fasterxml.jackson.core.io.IOContext; import com.fasterxml.jackson.core.json.WriterBasedJsonGenerator; +import com.fasterxml.jackson.core.util.BufferRecycler; public class WriterBasedJsonGeneratorTest extends BaseTest { @@ -48,6 +49,7 @@ private IOContext _ioContext(StreamWriteConstraints swc) { return new IOContext(StreamReadConstraints.defaults(), swc, ErrorReportConfiguration.defaults(), + new BufferRecycler(), ContentReference.unknown(), true); } } From 16add03729c6ba49d6258aa5186ea581527f05d1 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 29 Aug 2023 18:10:00 -0700 Subject: [PATCH 23/29] Warnings removal --- .../java/com/fasterxml/jackson/core/io/UTF8WriterTest.java | 1 - .../com/fasterxml/jackson/core/testsupport/TestSupport.java | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/test/java/com/fasterxml/jackson/core/io/UTF8WriterTest.java b/src/test/java/com/fasterxml/jackson/core/io/UTF8WriterTest.java index 9b1881f601..1907b9bf82 100644 --- a/src/test/java/com/fasterxml/jackson/core/io/UTF8WriterTest.java +++ b/src/test/java/com/fasterxml/jackson/core/io/UTF8WriterTest.java @@ -2,7 +2,6 @@ import java.io.*; -import com.fasterxml.jackson.core.StreamWriteConstraints; import org.junit.Assert; public class UTF8WriterTest diff --git a/src/test/java/com/fasterxml/jackson/core/testsupport/TestSupport.java b/src/test/java/com/fasterxml/jackson/core/testsupport/TestSupport.java index 8b4afd0492..e1a9e74dcc 100644 --- a/src/test/java/com/fasterxml/jackson/core/testsupport/TestSupport.java +++ b/src/test/java/com/fasterxml/jackson/core/testsupport/TestSupport.java @@ -45,7 +45,6 @@ private static IOContext testIOContext(StreamReadConstraints src, StreamWriteConstraints swc, ErrorReportConfiguration erc) { return new IOContext(src, swc, erc, - new BufferRecycler(), - ContentReference.unknown(), false); + new BufferRecycler(), ContentReference.unknown(), false); } } From 79b28858b5f7007303f36fa242626fad481f86a7 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 29 Aug 2023 19:58:03 -0700 Subject: [PATCH 24/29] Refactoring, adding more Javadocs --- .../fasterxml/jackson/core/JsonFactory.java | 4 +- .../fasterxml/jackson/core/TSFBuilder.java | 3 +- .../jackson/core/util/BufferRecyclerPool.java | 186 +++++++++++++++--- .../jackson/core/util/BufferRecyclers.java | 1 - .../jackson/core/TestJDKSerializability.java | 29 +++ .../core/write/BufferRecyclerPoolTest.java | 12 +- 6 files changed, 199 insertions(+), 36 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java index ec9f340ce2..7eaae033f3 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonFactory.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonFactory.java @@ -371,7 +371,7 @@ public static int collectDefaults() { public JsonFactory() { this((ObjectCodec) null); } public JsonFactory(ObjectCodec oc) { - _bufferRecyclerPool = BufferRecyclerPool.defaultRecyclerPool(); + _bufferRecyclerPool = BufferRecyclerPool.defaultPool(); _objectCodec = oc; _quoteChar = DEFAULT_QUOTE_CHAR; _streamReadConstraints = StreamReadConstraints.defaults(); @@ -2167,7 +2167,7 @@ public BufferRecyclerPool _getBufferRecyclerPool() { // scheme, for cases where it is considered harmful (possibly // on Android, for example) if (!Feature.USE_THREAD_LOCAL_FOR_BUFFER_RECYCLING.enabledIn(_factoryFeatures)) { - return BufferRecyclerPool.NonRecyclingRecyclerPool.INSTANCE; + return BufferRecyclerPool.nonRecyclingPool(); } return _bufferRecyclerPool; } diff --git a/src/main/java/com/fasterxml/jackson/core/TSFBuilder.java b/src/main/java/com/fasterxml/jackson/core/TSFBuilder.java index 02cf0d7f29..8ee09de195 100644 --- a/src/main/java/com/fasterxml/jackson/core/TSFBuilder.java +++ b/src/main/java/com/fasterxml/jackson/core/TSFBuilder.java @@ -9,7 +9,6 @@ import com.fasterxml.jackson.core.json.JsonReadFeature; import com.fasterxml.jackson.core.json.JsonWriteFeature; import com.fasterxml.jackson.core.util.BufferRecyclerPool; -import com.fasterxml.jackson.core.util.BufferRecyclers; import com.fasterxml.jackson.core.util.JsonGeneratorDecorator; /** @@ -142,7 +141,7 @@ protected TSFBuilder(JsonFactory base) protected TSFBuilder(int factoryFeatures, int parserFeatures, int generatorFeatures) { - _bufferRecyclerPool = BufferRecyclerPool.defaultRecyclerPool(); + _bufferRecyclerPool = BufferRecyclerPool.defaultPool(); _factoryFeatures = factoryFeatures; _streamReadFeatures = parserFeatures; diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java index c4e999fe2b..f1dcb89636 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java @@ -11,29 +11,69 @@ * * @since 2.16 */ -public interface BufferRecyclerPool extends Serializable { - +public interface BufferRecyclerPool extends Serializable +{ BufferRecycler acquireBufferRecycler(); void releaseBufferRecycler(BufferRecycler recycler); /** - * Returns the default BufferRecyclerPool implementation which is the thread local based one. + * @return the default {@link BufferRecyclerPool} implementation + * which is the thread local based one: + * basically alias to {@link #threadLocalPool()}). + */ + public static BufferRecyclerPool defaultPool() { + return threadLocalPool(); + } + + /** + * @return Shared instance of {@link ThreadLocalPool}; same as calling + * {@link ThreadLocalPool#shared()}. */ - static BufferRecyclerPool defaultRecyclerPool() { - return ThreadLocalRecyclerPool.INSTANCE; + public static BufferRecyclerPool threadLocalPool() { + return ThreadLocalPool.shared(); } + /** + * @return Shared instance of {@link NonRecyclingPool}; same as calling + * {@link NonRecyclingPool#shared()}. + */ + public static BufferRecyclerPool nonRecyclingPool() { + return NonRecyclingPool.shared(); + } + /** * Default {@link BufferRecyclerPool} implementation that uses - * {@link ThreadLocal} for recycling instances. - * - * @since 2.16 + * {@link ThreadLocal} for recycling instances. {@link BufferRecycler} + * instances are stored using {@link java.lang.ref.SoftReference}s so that + * they may be Garbage Collected as needed by JVM. + *

+ * Note that this implementation may not work well on platforms where + * {@link java.lang.ref.SoftReference}s are not well supported (like + * Android), or on platforms where {@link java.lang.Thread}s are not + * long-living or reused (like Project Loom). */ - class ThreadLocalRecyclerPool implements BufferRecyclerPool { + public static class ThreadLocalPool implements BufferRecyclerPool + { + private static final long serialVersionUID = 1L; + + private static final BufferRecyclerPool SHARED = new ThreadLocalPool(); - public static final BufferRecyclerPool INSTANCE = new ThreadLocalRecyclerPool(); + /** + * Accessor for the global, shared instance of {@link ThreadLocal}-based + * pool: due to its nature it is essentially Singleton as there can only + * be a single recycled {@link BufferRecycler} per thread. + * + * @return Shared pool instance + */ + public static BufferRecyclerPool shared() { + return SHARED; + } + + // No instances beyond shared one should be constructed + private ThreadLocalPool() { } + @SuppressWarnings("deprecation") @Override public BufferRecycler acquireBufferRecycler() { return BufferRecyclers.getBufferRecycler(); @@ -48,15 +88,31 @@ public void releaseBufferRecycler(BufferRecycler recycler) { /** * {@link BufferRecyclerPool} implementation that does not use * any pool but simply creates new instances when necessary. - * - * @since 2.16 */ - class NonRecyclingRecyclerPool implements BufferRecyclerPool { + public static class NonRecyclingPool implements BufferRecyclerPool + { + private static final long serialVersionUID = 1L; + + private static final BufferRecyclerPool SHARED = new NonRecyclingPool(); - public static final BufferRecyclerPool INSTANCE = new NonRecyclingRecyclerPool(); + // No instances beyond shared one should be constructed + private NonRecyclingPool() { } + + /** + * Accessor for the shared singleton instance; due to implementation having no state + * this is preferred over creating instances. + * + * @return Shared pool instance + */ + public static BufferRecyclerPool shared() { + return SHARED; + } + // // // Actual API implementation + @Override public BufferRecycler acquireBufferRecycler() { + // Could link back to this pool as marker? For now just leave back-ref empty return new BufferRecycler(); } @@ -69,15 +125,53 @@ public void releaseBufferRecycler(BufferRecycler recycler) { /** * {@link BufferRecyclerPool} implementation that uses * {@link ConcurrentLinkedDeque} for recycling instances. - * - * @since 2.16 */ - class ConcurrentDequePool implements BufferRecyclerPool { + public static class ConcurrentDequePool implements BufferRecyclerPool + { + private static final long serialVersionUID = 1L; - public static final BufferRecyclerPool INSTANCE = new ConcurrentDequePool(); + private static final BufferRecyclerPool SHARED = new ConcurrentDequePool(); - private final Deque pool = new ConcurrentLinkedDeque<>(); + private final transient Deque pool; + // // // Life-cycle (constructors, factory methods) + + protected ConcurrentDequePool() { + pool = new ConcurrentLinkedDeque<>(); + } + + /** + * Accessor for getting the globally shared singleton instance. + * Note that if you choose to use this instance, + * pool may be shared by many other {@code JsonFactory} instances. + * + * @return Shared pool instance + */ + public static BufferRecyclerPool shared() { + return SHARED; + } + + /** + * Accessor for creating and returning a new, non-shared pool instance. + * + * @return Newly constructed, non-shared pool instance + */ + public static BufferRecyclerPool nonShared() { + return new ConcurrentDequePool(); + } + + // // // JDK serialization support + + /** + * To avoid serializing pool contents we made {@code pool} {@code transient}; + * to compensate, need to re-create proper instance using constructor. + */ + protected Object readResolve() { + return new ConcurrentDequePool(); + } + + // // // Actual API implementation + @Override public BufferRecycler acquireBufferRecycler() { return getBufferRecycler().withPool(this); @@ -97,14 +191,56 @@ public void releaseBufferRecycler(BufferRecycler bufferRecycler) { /** * {@link BufferRecyclerPool} implementation that uses * a lock free linked list for recycling instances. - * - * @since 2.16 */ - class LockFreePool implements BufferRecyclerPool { + public static class LockFreePool implements BufferRecyclerPool + { + private static final long serialVersionUID = 1L; + + /** + * Globally shared pool instance. + */ + private static final BufferRecyclerPool SHARED = new LockFreePool(); + + // Needs to be transient to avoid JDK serialization from writing it out + private final transient AtomicReference head; + + // // // Life-cycle (constructors, factory methods) - public static final BufferRecyclerPool INSTANCE = new LockFreePool(); + private LockFreePool() { + head = new AtomicReference<>(); + } + + /** + * Accessor for getting the globally shared singleton instance. + * Note that if you choose to use this instance, + * pool may be shared by many other {@code JsonFactory} instances. + * + * @return Shared pool instance + */ + public static BufferRecyclerPool shared() { + return SHARED; + } + + /** + * Accessor for creating and returning a new, non-shared pool instance. + * + * @return Newly constructed, non-shared pool instance + */ + public static BufferRecyclerPool nonShared() { + return new LockFreePool(); + } + + // // // JDK serialization support + + /** + * To avoid serializing pool contents we made {@code head} {@code transient}; + * to compensate, need to re-create proper instance using constructor. + */ + protected Object readResolve() { + return new LockFreePool(); + } - private final AtomicReference head = new AtomicReference<>(); + // // // Actual API implementation @Override public BufferRecycler acquireBufferRecycler() { @@ -140,7 +276,7 @@ public void releaseBufferRecycler(BufferRecycler bufferRecycler) { } } - static class Node implements Serializable { + static class Node { final BufferRecycler value; LockFreePool.Node next; diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java index 994f4a6e4d..320b916659 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java @@ -2,7 +2,6 @@ import java.lang.ref.SoftReference; -import com.fasterxml.jackson.core.TokenStreamFactory; import com.fasterxml.jackson.core.io.JsonStringEncoder; /** diff --git a/src/test/java/com/fasterxml/jackson/core/TestJDKSerializability.java b/src/test/java/com/fasterxml/jackson/core/TestJDKSerializability.java index 951748ca62..3ea177f063 100644 --- a/src/test/java/com/fasterxml/jackson/core/TestJDKSerializability.java +++ b/src/test/java/com/fasterxml/jackson/core/TestJDKSerializability.java @@ -3,6 +3,7 @@ import java.io.*; import com.fasterxml.jackson.core.io.ContentReference; +import com.fasterxml.jackson.core.util.BufferRecyclerPool; import com.fasterxml.jackson.core.util.DefaultPrettyPrinter; /** @@ -105,6 +106,34 @@ public void testSourceReference() throws Exception assertSame(ref2, ContentReference.unknown()); } + /* + /********************************************************************** + /* Other entities + /********************************************************************** + */ + + public void testRecyclerPools() throws Exception + { + // First: shared/global pools that do not retain shared/global + // identity: + _testRecyclerPoolNonShared(BufferRecyclerPool.nonRecyclingPool()); + _testRecyclerPoolNonShared(BufferRecyclerPool.threadLocalPool()); + + // Then non-shared pool implementations + _testRecyclerPoolNonShared(BufferRecyclerPool.ConcurrentDequePool.nonShared()); + _testRecyclerPoolNonShared(BufferRecyclerPool.LockFreePool.nonShared()); + + // !!! TODO: Should test that shared/global singleton pools remained + // as global/shared too; but not yet implemented + } + + private void _testRecyclerPoolNonShared(BufferRecyclerPool pool) throws Exception { + byte[] stuff = jdkSerialize(pool); + BufferRecyclerPool result = jdkDeserialize(stuff); + assertNotNull(result); + assertEquals(pool.getClass(), result.getClass()); + } + /* /********************************************************************** /* Exception types diff --git a/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java b/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java index 2ee072bd7e..089b2b8059 100644 --- a/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java +++ b/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java @@ -8,22 +8,22 @@ import java.io.IOException; import java.io.OutputStream; -public class BufferRecyclerPoolTest extends BaseTest { - +public class BufferRecyclerPoolTest extends BaseTest +{ public void testNoOp() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.NonRecyclingRecyclerPool.INSTANCE); + checkBufferRecyclerPoolImpl(BufferRecyclerPool.NonRecyclingPool.shared()); } public void testThreadLocal() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.ThreadLocalRecyclerPool.INSTANCE); + checkBufferRecyclerPoolImpl(BufferRecyclerPool.ThreadLocalPool.shared()); } public void testLockFree() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.LockFreePool.INSTANCE); + checkBufferRecyclerPoolImpl(BufferRecyclerPool.LockFreePool.nonShared()); } public void testConcurrentDequeue() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.ConcurrentDequePool.INSTANCE); + checkBufferRecyclerPoolImpl(BufferRecyclerPool.ConcurrentDequePool.nonShared()); } private void checkBufferRecyclerPoolImpl(BufferRecyclerPool pool) { From 0f359fce2a4889d88ebe6194054fe90a7b8a45ec Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 29 Aug 2023 20:16:00 -0700 Subject: [PATCH 25/29] Minor change to test class --- .../fasterxml/jackson/core/write/BufferRecyclerPoolTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java b/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java index 089b2b8059..e629eb6ea2 100644 --- a/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java +++ b/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java @@ -27,7 +27,9 @@ public void testConcurrentDequeue() { } private void checkBufferRecyclerPoolImpl(BufferRecyclerPool pool) { - JsonFactory jsonFactory = new JsonFactory().setBufferRecyclerPool(pool); + JsonFactory jsonFactory = JsonFactory.builder() + .bufferRecyclerPool(pool) + .build(); assertEquals(6, write("test", jsonFactory)); } From 53941560446ffddec4ac17b245884de7a557e474 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Wed, 30 Aug 2023 14:45:34 +0200 Subject: [PATCH 26/29] improve tests checking that pooled BufferRecycler is properly acquired/released --- .../jackson/core/json/JsonGeneratorImpl.java | 4 +++ .../{write => io}/BufferRecyclerPoolTest.java | 33 ++++++++++++++----- 2 files changed, 28 insertions(+), 9 deletions(-) rename src/test/java/com/fasterxml/jackson/core/{write => io}/BufferRecyclerPoolTest.java (60%) diff --git a/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java b/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java index 487b41decc..3aaf01d3c6 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java +++ b/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java @@ -245,6 +245,10 @@ public JacksonFeatureSet getWriteCapabilities() { return JSON_WRITE_CAPABILITIES; } + public IOContext _getIoContext() { + return _ioContext; + } + /* /********************************************************** /* Shared helper methods diff --git a/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java b/src/test/java/com/fasterxml/jackson/core/io/BufferRecyclerPoolTest.java similarity index 60% rename from src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java rename to src/test/java/com/fasterxml/jackson/core/io/BufferRecyclerPoolTest.java index e629eb6ea2..9f6fe9d568 100644 --- a/src/test/java/com/fasterxml/jackson/core/write/BufferRecyclerPoolTest.java +++ b/src/test/java/com/fasterxml/jackson/core/io/BufferRecyclerPoolTest.java @@ -1,8 +1,10 @@ -package com.fasterxml.jackson.core.write; +package com.fasterxml.jackson.core.io; import com.fasterxml.jackson.core.BaseTest; import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.json.JsonGeneratorImpl; +import com.fasterxml.jackson.core.util.BufferRecycler; import com.fasterxml.jackson.core.util.BufferRecyclerPool; import java.io.IOException; @@ -11,36 +13,49 @@ public class BufferRecyclerPoolTest extends BaseTest { public void testNoOp() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.NonRecyclingPool.shared()); + // no-op pool doesn't actually pool anything, so avoid checking it + checkBufferRecyclerPoolImpl(BufferRecyclerPool.NonRecyclingPool.shared(), false); } public void testThreadLocal() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.ThreadLocalPool.shared()); + checkBufferRecyclerPoolImpl(BufferRecyclerPool.ThreadLocalPool.shared(), true); } public void testLockFree() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.LockFreePool.nonShared()); + checkBufferRecyclerPoolImpl(BufferRecyclerPool.LockFreePool.shared(), true); } public void testConcurrentDequeue() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.ConcurrentDequePool.nonShared()); + checkBufferRecyclerPoolImpl(BufferRecyclerPool.ConcurrentDequePool.shared(), true); } - private void checkBufferRecyclerPoolImpl(BufferRecyclerPool pool) { + private void checkBufferRecyclerPoolImpl(BufferRecyclerPool pool, boolean checkPooledResource) { JsonFactory jsonFactory = JsonFactory.builder() .bufferRecyclerPool(pool) .build(); - assertEquals(6, write("test", jsonFactory)); + BufferRecycler usedBufferRecycler = write("test", jsonFactory, 6); + if (checkPooledResource) { + // acquire the pooled BufferRecycler again and check if it is the same instance used before + BufferRecycler pooledBufferRecycler = pool.acquireBufferRecycler(); + try { + assertSame(usedBufferRecycler, pooledBufferRecycler); + } finally { + pooledBufferRecycler.release(); + } + } } - protected final int write(Object value, JsonFactory jsonFactory) { + protected final BufferRecycler write(Object value, JsonFactory jsonFactory, int expectedSize) { + BufferRecycler bufferRecycler; NopOutputStream out = new NopOutputStream(); try (JsonGenerator gen = jsonFactory.createGenerator(out)) { + bufferRecycler = ((JsonGeneratorImpl) gen)._getIoContext()._bufferRecycler; gen.writeObject(value); } catch (IOException e) { throw new RuntimeException(e); } - return out.size(); + assertEquals(expectedSize, out.size); + return bufferRecycler; } public class NopOutputStream extends OutputStream { From 9d53f003c5b2f40ec02a932d5c33f706e9de0029 Mon Sep 17 00:00:00 2001 From: mariofusco Date: Wed, 30 Aug 2023 15:25:44 +0200 Subject: [PATCH 27/29] add bounded pool implementation --- .../jackson/core/util/BufferRecyclerPool.java | 82 +++++++++++++++++-- .../core/io/BufferRecyclerPoolTest.java | 9 +- 2 files changed, 82 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java index f1dcb89636..183a357a9c 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java @@ -2,6 +2,8 @@ import java.io.Serializable; import java.util.Deque; +import java.util.Queue; +import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.atomic.AtomicReference; @@ -22,7 +24,7 @@ public interface BufferRecyclerPool extends Serializable * which is the thread local based one: * basically alias to {@link #threadLocalPool()}). */ - public static BufferRecyclerPool defaultPool() { + static BufferRecyclerPool defaultPool() { return threadLocalPool(); } @@ -30,7 +32,7 @@ public static BufferRecyclerPool defaultPool() { * @return Shared instance of {@link ThreadLocalPool}; same as calling * {@link ThreadLocalPool#shared()}. */ - public static BufferRecyclerPool threadLocalPool() { + static BufferRecyclerPool threadLocalPool() { return ThreadLocalPool.shared(); } @@ -38,7 +40,7 @@ public static BufferRecyclerPool threadLocalPool() { * @return Shared instance of {@link NonRecyclingPool}; same as calling * {@link NonRecyclingPool#shared()}. */ - public static BufferRecyclerPool nonRecyclingPool() { + static BufferRecyclerPool nonRecyclingPool() { return NonRecyclingPool.shared(); } @@ -53,7 +55,7 @@ public static BufferRecyclerPool nonRecyclingPool() { * Android), or on platforms where {@link java.lang.Thread}s are not * long-living or reused (like Project Loom). */ - public static class ThreadLocalPool implements BufferRecyclerPool + class ThreadLocalPool implements BufferRecyclerPool { private static final long serialVersionUID = 1L; @@ -89,7 +91,7 @@ public void releaseBufferRecycler(BufferRecycler recycler) { * {@link BufferRecyclerPool} implementation that does not use * any pool but simply creates new instances when necessary. */ - public static class NonRecyclingPool implements BufferRecyclerPool + class NonRecyclingPool implements BufferRecyclerPool { private static final long serialVersionUID = 1L; @@ -126,7 +128,7 @@ public void releaseBufferRecycler(BufferRecycler recycler) { * {@link BufferRecyclerPool} implementation that uses * {@link ConcurrentLinkedDeque} for recycling instances. */ - public static class ConcurrentDequePool implements BufferRecyclerPool + class ConcurrentDequePool implements BufferRecyclerPool { private static final long serialVersionUID = 1L; @@ -192,7 +194,7 @@ public void releaseBufferRecycler(BufferRecycler bufferRecycler) { * {@link BufferRecyclerPool} implementation that uses * a lock free linked list for recycling instances. */ - public static class LockFreePool implements BufferRecyclerPool + class LockFreePool implements BufferRecyclerPool { private static final long serialVersionUID = 1L; @@ -285,4 +287,70 @@ static class Node { } } } + + /** + * {@link BufferRecyclerPool} implementation that uses + * a bounded queue for recycling instances. + */ + class BoundedPool implements BufferRecyclerPool + { + private static final long serialVersionUID = 1L; + + private static final BufferRecyclerPool SHARED = new BoundedPool(16); + + private final transient Queue pool; + + // // // Life-cycle (constructors, factory methods) + + protected BoundedPool(int capacity) { + pool = new ArrayBlockingQueue<>(capacity); + } + + /** + * Accessor for getting the globally shared singleton instance. + * Note that if you choose to use this instance, + * pool may be shared by many other {@code JsonFactory} instances. + * + * @return Shared pool instance + */ + public static BufferRecyclerPool shared() { + return SHARED; + } + + /** + * Accessor for creating and returning a new, non-shared pool instance. + * + * @return Newly constructed, non-shared pool instance + */ + public static BufferRecyclerPool nonShared(int capacity) { + return new BoundedPool(capacity); + } + + // // // JDK serialization support + + /** + * To avoid serializing pool contents we made {@code pool} {@code transient}; + * to compensate, need to re-create proper instance using constructor. + */ + protected Object readResolve() { + return SHARED; + } + + // // // Actual API implementation + + @Override + public BufferRecycler acquireBufferRecycler() { + return getBufferRecycler().withPool(this); + } + + private BufferRecycler getBufferRecycler() { + BufferRecycler bufferRecycler = pool.poll(); + return bufferRecycler != null ? bufferRecycler : new BufferRecycler(); + } + + @Override + public void releaseBufferRecycler(BufferRecycler bufferRecycler) { + pool.offer(bufferRecycler); + } + } } diff --git a/src/test/java/com/fasterxml/jackson/core/io/BufferRecyclerPoolTest.java b/src/test/java/com/fasterxml/jackson/core/io/BufferRecyclerPoolTest.java index 9f6fe9d568..e76153006b 100644 --- a/src/test/java/com/fasterxml/jackson/core/io/BufferRecyclerPoolTest.java +++ b/src/test/java/com/fasterxml/jackson/core/io/BufferRecyclerPoolTest.java @@ -22,11 +22,15 @@ public void testThreadLocal() { } public void testLockFree() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.LockFreePool.shared(), true); + checkBufferRecyclerPoolImpl(BufferRecyclerPool.LockFreePool.nonShared(), true); } public void testConcurrentDequeue() { - checkBufferRecyclerPoolImpl(BufferRecyclerPool.ConcurrentDequePool.shared(), true); + checkBufferRecyclerPoolImpl(BufferRecyclerPool.ConcurrentDequePool.nonShared(), true); + } + + public void testBounded() { + checkBufferRecyclerPoolImpl(BufferRecyclerPool.BoundedPool.nonShared(1), true); } private void checkBufferRecyclerPoolImpl(BufferRecyclerPool pool, boolean checkPooledResource) { @@ -34,6 +38,7 @@ private void checkBufferRecyclerPoolImpl(BufferRecyclerPool pool, boolean checkP .bufferRecyclerPool(pool) .build(); BufferRecycler usedBufferRecycler = write("test", jsonFactory, 6); + if (checkPooledResource) { // acquire the pooled BufferRecycler again and check if it is the same instance used before BufferRecycler pooledBufferRecycler = pool.acquireBufferRecycler(); From 58e6f30f848602a84d685d6c641a127288d3939b Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 30 Aug 2023 14:59:09 -0700 Subject: [PATCH 28/29] Adding Javadocs, minor renaming --- .../jackson/core/json/JsonGeneratorImpl.java | 15 ++- .../jackson/core/util/BufferRecyclerPool.java | 96 ++++++++++++++++--- .../jackson/core/TestJDKSerializability.java | 15 ++- .../core/io/BufferRecyclerPoolTest.java | 2 +- 4 files changed, 107 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java b/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java index 3aaf01d3c6..ad2eaa3fa0 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java +++ b/src/main/java/com/fasterxml/jackson/core/json/JsonGeneratorImpl.java @@ -245,7 +245,20 @@ public JacksonFeatureSet getWriteCapabilities() { return JSON_WRITE_CAPABILITIES; } - public IOContext _getIoContext() { + /* + /********************************************************** + /* Misc other accessors + /********************************************************** + */ + + /** + * Accessor for use by {@code jackson-core} itself (tests in particular). + * + * @return {@link IOContext} in use by this generator + * + * @since 2.16 + */ + public IOContext ioContext() { return _ioContext; } diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java index 183a357a9c..016a80cb64 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java @@ -10,13 +10,50 @@ /** * Interface for entity that controls creation and possible reuse of {@link BufferRecycler} * instances used for recycling of underlying input/output buffers. + *

+ * Different pool implementations use different strategies on retaining + * recyclers for reuse. For example we have: + *

+ * + *

+ * Default implementations are also included as nested classes. * * @since 2.16 */ public interface BufferRecyclerPool extends Serializable { + /** + * Method called to acquire {@link BufferRecycler}; possibly + * (but necessarily) a pooled recycler instance (depends on implementation + * and pool state). + * + * @return {@link BufferRecycler} for caller to use; caller expected + * to call {@link #releaseBufferRecycler} after it is done using recycler. + */ BufferRecycler acquireBufferRecycler(); + /** + * Method that should be called when previously acquired (see {@link #acquireBufferRecycler}) + * recycler instances is no longer needed; this lets pool to take ownership + * for possible reuse. + * + * @param recycler + */ void releaseBufferRecycler(BufferRecycler recycler); /** @@ -29,7 +66,7 @@ static BufferRecyclerPool defaultPool() { } /** - * @return Shared instance of {@link ThreadLocalPool}; same as calling + * @return Globally shared instance of {@link ThreadLocalPool}; same as calling * {@link ThreadLocalPool#shared()}. */ static BufferRecyclerPool threadLocalPool() { @@ -37,13 +74,19 @@ static BufferRecyclerPool threadLocalPool() { } /** - * @return Shared instance of {@link NonRecyclingPool}; same as calling + * @return Globally shared instance of {@link NonRecyclingPool}; same as calling * {@link NonRecyclingPool#shared()}. */ static BufferRecyclerPool nonRecyclingPool() { return NonRecyclingPool.shared(); } - + + /* + /********************************************************************** + /* Default BufferRecyclerPool implementations + /********************************************************************** + */ + /** * Default {@link BufferRecyclerPool} implementation that uses * {@link ThreadLocal} for recycling instances. {@link BufferRecycler} @@ -59,7 +102,7 @@ class ThreadLocalPool implements BufferRecyclerPool { private static final long serialVersionUID = 1L; - private static final BufferRecyclerPool SHARED = new ThreadLocalPool(); + private static final BufferRecyclerPool GLOBAL = new ThreadLocalPool(); /** * Accessor for the global, shared instance of {@link ThreadLocal}-based @@ -69,12 +112,18 @@ class ThreadLocalPool implements BufferRecyclerPool * @return Shared pool instance */ public static BufferRecyclerPool shared() { - return SHARED; + return GLOBAL; } // No instances beyond shared one should be constructed private ThreadLocalPool() { } + // // // JDK serialization support + + protected Object readResolve() { return GLOBAL; } + + // // // JDK serialization support + @SuppressWarnings("deprecation") @Override public BufferRecycler acquireBufferRecycler() { @@ -95,7 +144,7 @@ class NonRecyclingPool implements BufferRecyclerPool { private static final long serialVersionUID = 1L; - private static final BufferRecyclerPool SHARED = new NonRecyclingPool(); + private static final BufferRecyclerPool GLOBAL = new NonRecyclingPool(); // No instances beyond shared one should be constructed private NonRecyclingPool() { } @@ -107,9 +156,13 @@ private NonRecyclingPool() { } * @return Shared pool instance */ public static BufferRecyclerPool shared() { - return SHARED; + return GLOBAL; } + // // // JDK serialization support + + protected Object readResolve() { return GLOBAL; } + // // // Actual API implementation @Override @@ -127,12 +180,14 @@ public void releaseBufferRecycler(BufferRecycler recycler) { /** * {@link BufferRecyclerPool} implementation that uses * {@link ConcurrentLinkedDeque} for recycling instances. + *

+ * Pool is unbounded: see {@link BufferRecyclerPool} what this means. */ class ConcurrentDequePool implements BufferRecyclerPool { private static final long serialVersionUID = 1L; - private static final BufferRecyclerPool SHARED = new ConcurrentDequePool(); + private static final BufferRecyclerPool GLOBAL = new ConcurrentDequePool(); private final transient Deque pool; @@ -150,7 +205,7 @@ protected ConcurrentDequePool() { * @return Shared pool instance */ public static BufferRecyclerPool shared() { - return SHARED; + return GLOBAL; } /** @@ -193,6 +248,8 @@ public void releaseBufferRecycler(BufferRecycler bufferRecycler) { /** * {@link BufferRecyclerPool} implementation that uses * a lock free linked list for recycling instances. + *

+ * Pool is unbounded: see {@link BufferRecyclerPool} what this means. */ class LockFreePool implements BufferRecyclerPool { @@ -201,7 +258,7 @@ class LockFreePool implements BufferRecyclerPool /** * Globally shared pool instance. */ - private static final BufferRecyclerPool SHARED = new LockFreePool(); + private static final BufferRecyclerPool GLOBAL = new LockFreePool(); // Needs to be transient to avoid JDK serialization from writing it out private final transient AtomicReference head; @@ -220,7 +277,7 @@ private LockFreePool() { * @return Shared pool instance */ public static BufferRecyclerPool shared() { - return SHARED; + return GLOBAL; } /** @@ -290,13 +347,22 @@ static class Node { /** * {@link BufferRecyclerPool} implementation that uses - * a bounded queue for recycling instances. + * a bounded queue ({@link ArrayBlockingQueue} for recycling instances. + * This is "bounded" pool since it will never hold on to more + * {@link BufferRecycler} instances than its size configuration: + * the default size is {@link BoundedPool#DEFAULT_CAPACITY}. */ class BoundedPool implements BufferRecyclerPool { private static final long serialVersionUID = 1L; - private static final BufferRecyclerPool SHARED = new BoundedPool(16); + /** + * Default capacity which limits number of recyclers that are ever + * retained for reuse. + */ + public final static int DEFAULT_CAPACITY = 100; + + private static final BufferRecyclerPool GLOBAL = new BoundedPool(DEFAULT_CAPACITY); private final transient Queue pool; @@ -314,7 +380,7 @@ protected BoundedPool(int capacity) { * @return Shared pool instance */ public static BufferRecyclerPool shared() { - return SHARED; + return GLOBAL; } /** @@ -333,7 +399,7 @@ public static BufferRecyclerPool nonShared(int capacity) { * to compensate, need to re-create proper instance using constructor. */ protected Object readResolve() { - return SHARED; + return GLOBAL; } // // // Actual API implementation diff --git a/src/test/java/com/fasterxml/jackson/core/TestJDKSerializability.java b/src/test/java/com/fasterxml/jackson/core/TestJDKSerializability.java index 3ea177f063..7185a8f5fe 100644 --- a/src/test/java/com/fasterxml/jackson/core/TestJDKSerializability.java +++ b/src/test/java/com/fasterxml/jackson/core/TestJDKSerializability.java @@ -114,10 +114,10 @@ public void testSourceReference() throws Exception public void testRecyclerPools() throws Exception { - // First: shared/global pools that do not retain shared/global - // identity: - _testRecyclerPoolNonShared(BufferRecyclerPool.nonRecyclingPool()); - _testRecyclerPoolNonShared(BufferRecyclerPool.threadLocalPool()); + // First: shared/global pools that will always remain/become globally + // shared instances + _testRecyclerPoolGlobal(BufferRecyclerPool.nonRecyclingPool()); + _testRecyclerPoolGlobal(BufferRecyclerPool.threadLocalPool()); // Then non-shared pool implementations _testRecyclerPoolNonShared(BufferRecyclerPool.ConcurrentDequePool.nonShared()); @@ -127,6 +127,13 @@ public void testRecyclerPools() throws Exception // as global/shared too; but not yet implemented } + private void _testRecyclerPoolGlobal(BufferRecyclerPool pool) throws Exception { + byte[] stuff = jdkSerialize(pool); + BufferRecyclerPool result = jdkDeserialize(stuff); + assertNotNull(result); + assertSame(pool.getClass(), result.getClass()); + } + private void _testRecyclerPoolNonShared(BufferRecyclerPool pool) throws Exception { byte[] stuff = jdkSerialize(pool); BufferRecyclerPool result = jdkDeserialize(stuff); diff --git a/src/test/java/com/fasterxml/jackson/core/io/BufferRecyclerPoolTest.java b/src/test/java/com/fasterxml/jackson/core/io/BufferRecyclerPoolTest.java index e76153006b..2381f520b1 100644 --- a/src/test/java/com/fasterxml/jackson/core/io/BufferRecyclerPoolTest.java +++ b/src/test/java/com/fasterxml/jackson/core/io/BufferRecyclerPoolTest.java @@ -54,7 +54,7 @@ protected final BufferRecycler write(Object value, JsonFactory jsonFactory, int BufferRecycler bufferRecycler; NopOutputStream out = new NopOutputStream(); try (JsonGenerator gen = jsonFactory.createGenerator(out)) { - bufferRecycler = ((JsonGeneratorImpl) gen)._getIoContext()._bufferRecycler; + bufferRecycler = ((JsonGeneratorImpl) gen).ioContext()._bufferRecycler; gen.writeObject(value); } catch (IOException e) { throw new RuntimeException(e); From eb2461bc187afc239eb3c3281ea638e118051647 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 30 Aug 2023 17:31:14 -0700 Subject: [PATCH 29/29] Implement sharing-retaining JDK serialization for BufferRecyclerPools. --- .../jackson/core/util/BufferRecyclerPool.java | 133 ++++++++++++------ .../jackson/core/util/BufferRecyclers.java | 2 +- .../jackson/core/TestJDKSerializability.java | 23 +-- 3 files changed, 104 insertions(+), 54 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java index 016a80cb64..672f5f352b 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerPool.java @@ -2,7 +2,7 @@ import java.io.Serializable; import java.util.Deque; -import java.util.Queue; +import java.util.Optional; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.atomic.AtomicReference; @@ -122,7 +122,7 @@ private ThreadLocalPool() { } protected Object readResolve() { return GLOBAL; } - // // // JDK serialization support + // // // Actual API implementation @SuppressWarnings("deprecation") @Override @@ -164,7 +164,7 @@ public static BufferRecyclerPool shared() { protected Object readResolve() { return GLOBAL; } // // // Actual API implementation - + @Override public BufferRecycler acquireBufferRecycler() { // Could link back to this pool as marker? For now just leave back-ref empty @@ -177,23 +177,55 @@ public void releaseBufferRecycler(BufferRecycler recycler) { } } + /** + * Intermediate base class for instances that are stateful and require + * special handling with respect to JDK serialization, to retain + * "global" reference distinct from non-shared ones. + */ + abstract class StatefulImplBase implements BufferRecyclerPool { + private static final long serialVersionUID = 1L; + + protected final static int SERIALIZATION_SHARED = -1; + + protected final static int SERIALIZATION_NON_SHARED = 1; + + /** + * Value that indicates basic aspects of pool for JDK serialization; + * either marker for shared/non-shared, or possibly bounded size; + * depends on sub-class. + */ + protected final int _serialization; + + protected StatefulImplBase(int serialization) { + _serialization = serialization; + } + + protected Optional _resolveToShared(BufferRecyclerPool shared) { + if (_serialization == SERIALIZATION_SHARED) { + return Optional.of(shared); + } + return Optional.empty(); + } + } + /** * {@link BufferRecyclerPool} implementation that uses * {@link ConcurrentLinkedDeque} for recycling instances. *

* Pool is unbounded: see {@link BufferRecyclerPool} what this means. */ - class ConcurrentDequePool implements BufferRecyclerPool + class ConcurrentDequePool extends StatefulImplBase { private static final long serialVersionUID = 1L; - private static final BufferRecyclerPool GLOBAL = new ConcurrentDequePool(); + private static final ConcurrentDequePool GLOBAL = new ConcurrentDequePool(SERIALIZATION_SHARED); private final transient Deque pool; // // // Life-cycle (constructors, factory methods) - protected ConcurrentDequePool() { + protected ConcurrentDequePool(int serialization) { + super(serialization); pool = new ConcurrentLinkedDeque<>(); } @@ -204,7 +236,7 @@ protected ConcurrentDequePool() { * * @return Shared pool instance */ - public static BufferRecyclerPool shared() { + public static ConcurrentDequePool shared() { return GLOBAL; } @@ -213,30 +245,28 @@ public static BufferRecyclerPool shared() { * * @return Newly constructed, non-shared pool instance */ - public static BufferRecyclerPool nonShared() { - return new ConcurrentDequePool(); + public static ConcurrentDequePool nonShared() { + return new ConcurrentDequePool(SERIALIZATION_NON_SHARED); } // // // JDK serialization support /** - * To avoid serializing pool contents we made {@code pool} {@code transient}; - * to compensate, need to re-create proper instance using constructor. + * Make sure to re-link to global/shared or non-shared. */ protected Object readResolve() { - return new ConcurrentDequePool(); + return _resolveToShared(GLOBAL).orElseGet(() -> nonShared()); } // // // Actual API implementation @Override public BufferRecycler acquireBufferRecycler() { - return getBufferRecycler().withPool(this); - } - - private BufferRecycler getBufferRecycler() { BufferRecycler bufferRecycler = pool.pollFirst(); - return bufferRecycler != null ? bufferRecycler : new BufferRecycler(); + if (bufferRecycler == null) { + bufferRecycler = new BufferRecycler(); + } + return bufferRecycler.withPool(this); } @Override @@ -248,24 +278,25 @@ public void releaseBufferRecycler(BufferRecycler bufferRecycler) { /** * {@link BufferRecyclerPool} implementation that uses * a lock free linked list for recycling instances. - *

- * Pool is unbounded: see {@link BufferRecyclerPool} what this means. + * Pool is unbounded: see {@link BufferRecyclerPool} for + * details on what this means. */ - class LockFreePool implements BufferRecyclerPool + class LockFreePool extends StatefulImplBase { private static final long serialVersionUID = 1L; /** * Globally shared pool instance. */ - private static final BufferRecyclerPool GLOBAL = new LockFreePool(); + private static final LockFreePool GLOBAL = new LockFreePool(SERIALIZATION_SHARED); // Needs to be transient to avoid JDK serialization from writing it out private final transient AtomicReference head; // // // Life-cycle (constructors, factory methods) - private LockFreePool() { + private LockFreePool(int serialization) { + super(serialization); head = new AtomicReference<>(); } @@ -276,7 +307,7 @@ private LockFreePool() { * * @return Shared pool instance */ - public static BufferRecyclerPool shared() { + public static LockFreePool shared() { return GLOBAL; } @@ -285,28 +316,27 @@ public static BufferRecyclerPool shared() { * * @return Newly constructed, non-shared pool instance */ - public static BufferRecyclerPool nonShared() { - return new LockFreePool(); + public static LockFreePool nonShared() { + return new LockFreePool(SERIALIZATION_NON_SHARED); } // // // JDK serialization support /** - * To avoid serializing pool contents we made {@code head} {@code transient}; - * to compensate, need to re-create proper instance using constructor. + * Make sure to re-link to global/shared or non-shared. */ protected Object readResolve() { - return new LockFreePool(); + return _resolveToShared(GLOBAL).orElseGet(() -> nonShared()); } // // // Actual API implementation @Override public BufferRecycler acquireBufferRecycler() { - return getBufferRecycler().withPool(this); + return _getRecycler().withPool(this); } - private BufferRecycler getBufferRecycler() { + private BufferRecycler _getRecycler() { // This simple lock free algorithm uses an optimistic compareAndSet strategy to // populate the underlying linked list in a thread-safe way. However, under very // heavy contention, the compareAndSet could fail multiple times, so it seems a @@ -335,7 +365,7 @@ public void releaseBufferRecycler(BufferRecycler bufferRecycler) { } } - static class Node { + private static class Node { final BufferRecycler value; LockFreePool.Node next; @@ -352,7 +382,7 @@ static class Node { * {@link BufferRecycler} instances than its size configuration: * the default size is {@link BoundedPool#DEFAULT_CAPACITY}. */ - class BoundedPool implements BufferRecyclerPool + class BoundedPool extends StatefulImplBase { private static final long serialVersionUID = 1L; @@ -362,13 +392,17 @@ class BoundedPool implements BufferRecyclerPool */ public final static int DEFAULT_CAPACITY = 100; - private static final BufferRecyclerPool GLOBAL = new BoundedPool(DEFAULT_CAPACITY); + private static final BoundedPool GLOBAL = new BoundedPool(SERIALIZATION_SHARED); - private final transient Queue pool; + private final transient ArrayBlockingQueue pool; + + private final transient int capacity; // // // Life-cycle (constructors, factory methods) - protected BoundedPool(int capacity) { + protected BoundedPool(int capacityAsId) { + super(capacityAsId); + capacity = (capacityAsId <= 0) ? DEFAULT_CAPACITY : capacityAsId; pool = new ArrayBlockingQueue<>(capacity); } @@ -379,44 +413,53 @@ protected BoundedPool(int capacity) { * * @return Shared pool instance */ - public static BufferRecyclerPool shared() { + public static BoundedPool shared() { return GLOBAL; } /** * Accessor for creating and returning a new, non-shared pool instance. * + * @param capacity Maximum capacity of the pool: must be positive number above zero. + * * @return Newly constructed, non-shared pool instance */ - public static BufferRecyclerPool nonShared(int capacity) { + public static BoundedPool nonShared(int capacity) { + if (capacity <= 0) { + throw new IllegalArgumentException("capacity must be > 0, was: "+capacity); + } return new BoundedPool(capacity); } // // // JDK serialization support /** - * To avoid serializing pool contents we made {@code pool} {@code transient}; - * to compensate, need to re-create proper instance using constructor. + * Make sure to re-link to global/shared or non-shared. */ protected Object readResolve() { - return GLOBAL; + return _resolveToShared(GLOBAL).orElseGet(() -> nonShared(_serialization)); } // // // Actual API implementation @Override public BufferRecycler acquireBufferRecycler() { - return getBufferRecycler().withPool(this); - } - - private BufferRecycler getBufferRecycler() { BufferRecycler bufferRecycler = pool.poll(); - return bufferRecycler != null ? bufferRecycler : new BufferRecycler(); + if (bufferRecycler == null) { + bufferRecycler = new BufferRecycler(); + } + return bufferRecycler.withPool(this); } @Override public void releaseBufferRecycler(BufferRecycler bufferRecycler) { pool.offer(bufferRecycler); } + + // // // Other methods + + public int capacity() { + return capacity; + } } } diff --git a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java index 320b916659..0e8fffc14b 100644 --- a/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java +++ b/src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java @@ -17,7 +17,7 @@ public class BufferRecyclers /** * System property that is checked to see if recycled buffers (see {@link BufferRecycler}) * should be tracked, for purpose of forcing release of all such buffers, typically - * during major classloading. + * during major garbage-collection. * * @since 2.9.6 */ diff --git a/src/test/java/com/fasterxml/jackson/core/TestJDKSerializability.java b/src/test/java/com/fasterxml/jackson/core/TestJDKSerializability.java index 7185a8f5fe..d03740d152 100644 --- a/src/test/java/com/fasterxml/jackson/core/TestJDKSerializability.java +++ b/src/test/java/com/fasterxml/jackson/core/TestJDKSerializability.java @@ -119,26 +119,33 @@ public void testRecyclerPools() throws Exception _testRecyclerPoolGlobal(BufferRecyclerPool.nonRecyclingPool()); _testRecyclerPoolGlobal(BufferRecyclerPool.threadLocalPool()); - // Then non-shared pool implementations + _testRecyclerPoolGlobal(BufferRecyclerPool.ConcurrentDequePool.shared()); + _testRecyclerPoolGlobal(BufferRecyclerPool.LockFreePool.shared()); + BufferRecyclerPool.BoundedPool bounded = + _testRecyclerPoolGlobal(BufferRecyclerPool.BoundedPool.shared()); + assertEquals(BufferRecyclerPool.BoundedPool.DEFAULT_CAPACITY, bounded.capacity()); + _testRecyclerPoolNonShared(BufferRecyclerPool.ConcurrentDequePool.nonShared()); _testRecyclerPoolNonShared(BufferRecyclerPool.LockFreePool.nonShared()); - - // !!! TODO: Should test that shared/global singleton pools remained - // as global/shared too; but not yet implemented + bounded = _testRecyclerPoolNonShared(BufferRecyclerPool.BoundedPool.nonShared(250)); + assertEquals(250, bounded.capacity()); } - private void _testRecyclerPoolGlobal(BufferRecyclerPool pool) throws Exception { + private T _testRecyclerPoolGlobal(T pool) throws Exception { byte[] stuff = jdkSerialize(pool); - BufferRecyclerPool result = jdkDeserialize(stuff); + T result = jdkDeserialize(stuff); assertNotNull(result); assertSame(pool.getClass(), result.getClass()); + return result; } - private void _testRecyclerPoolNonShared(BufferRecyclerPool pool) throws Exception { + private T _testRecyclerPoolNonShared(T pool) throws Exception { byte[] stuff = jdkSerialize(pool); - BufferRecyclerPool result = jdkDeserialize(stuff); + T result = jdkDeserialize(stuff); assertNotNull(result); assertEquals(pool.getClass(), result.getClass()); + assertNotSame(pool, result); + return result; } /*