Skip to content

Add BufferRecyclerProvider configuration #1083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 22, 2023
Merged

Add BufferRecyclerProvider configuration #1083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
751f51e
Add `BufferRecyclerProvider` configuration
cowtowncoder Aug 21, 2023
11247b8
Change BufferRecyclerProvider to interface; remove (for now) unnecess…
cowtowncoder Aug 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions src/main/java/com/fasterxml/jackson/core/JsonFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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.BufferRecyclerProvider;
import com.fasterxml.jackson.core.util.BufferRecyclers;
import com.fasterxml.jackson.core.util.JacksonFeature;
import com.fasterxml.jackson.core.util.JsonGeneratorDecorator;
Expand Down Expand Up @@ -260,6 +261,11 @@ public static int collectDefaults() {
/**********************************************************
*/

/**
* @since 2.16
*/
protected BufferRecyclerProvider _bufferRecyclerProvider;

/**
* Object that implements conversion functionality between
* Java objects and JSON content. For base JsonFactory implementation
Expand Down Expand Up @@ -364,6 +370,7 @@ public static int collectDefaults() {
public JsonFactory() { this((ObjectCodec) null); }

public JsonFactory(ObjectCodec oc) {
_bufferRecyclerProvider = BufferRecyclers.defaultProvider();
_objectCodec = oc;
_quoteChar = DEFAULT_QUOTE_CHAR;
_streamReadConstraints = StreamReadConstraints.defaults();
Expand All @@ -382,6 +389,7 @@ public JsonFactory(ObjectCodec oc) {
*/
protected JsonFactory(JsonFactory src, ObjectCodec codec)
{
_bufferRecyclerProvider = src._bufferRecyclerProvider;
_objectCodec = codec;

// General
Expand Down Expand Up @@ -410,6 +418,7 @@ protected JsonFactory(JsonFactory src, ObjectCodec codec)
* @since 2.10
*/
public JsonFactory(JsonFactoryBuilder b) {
_bufferRecyclerProvider = b._bufferRecyclerProvider;
_objectCodec = null;

// General
Expand Down Expand Up @@ -439,6 +448,7 @@ public JsonFactory(JsonFactoryBuilder b) {
* @param bogus Argument only needed to separate constructor signature; ignored
*/
protected JsonFactory(TSFBuilder<?,?> b, boolean bogus) {
_bufferRecyclerProvider = b._bufferRecyclerProvider;
_objectCodec = null;

_factoryFeatures = b._factoryFeatures;
Expand Down Expand Up @@ -1131,6 +1141,11 @@ public String getRootValueSeparator() {
/**********************************************************
*/

public JsonFactory setBufferRecyclerProvider(BufferRecyclerProvider p) {
_bufferRecyclerProvider = Objects.requireNonNull(p);
return this;
}

/**
* Method for associating a {@link ObjectCodec} (typically
* a <code>com.fasterxml.jackson.databind.ObjectMapper</code>)
Expand Down Expand Up @@ -2126,10 +2141,10 @@ public BufferRecycler _getBufferRecycler()
// 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.getBufferRecycler();
if (!Feature.USE_THREAD_LOCAL_FOR_BUFFER_RECYCLING.enabledIn(_factoryFeatures)) {
return new BufferRecycler();
}
return new BufferRecycler();
return _bufferRecyclerProvider.acquireBufferRecycler(this);
}

/**
Expand Down
29 changes: 28 additions & 1 deletion src/main/java/com/fasterxml/jackson/core/TSFBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import com.fasterxml.jackson.core.io.OutputDecorator;
import com.fasterxml.jackson.core.json.JsonReadFeature;
import com.fasterxml.jackson.core.json.JsonWriteFeature;
import com.fasterxml.jackson.core.util.BufferRecyclerProvider;
import com.fasterxml.jackson.core.util.BufferRecyclers;
import com.fasterxml.jackson.core.util.JsonGeneratorDecorator;

/**
Expand Down Expand Up @@ -70,6 +72,11 @@ public abstract class TSFBuilder<F extends JsonFactory,
/**********************************************************************
*/

/**
* @since 2.16
*/
protected BufferRecyclerProvider _bufferRecyclerProvider;

/**
* Optional helper object that may decorate input sources, to do
* additional processing on input during parsing.
Expand Down Expand Up @@ -108,7 +115,7 @@ public abstract class TSFBuilder<F extends JsonFactory,
*/
protected List<JsonGeneratorDecorator> _generatorDecorators;


/*
/**********************************************************************
/* Construction
/**********************************************************************
Expand All @@ -135,6 +142,8 @@ protected TSFBuilder(JsonFactory base)
protected TSFBuilder(int factoryFeatures,
int parserFeatures, int generatorFeatures)
{
_bufferRecyclerProvider = BufferRecyclers.defaultProvider();

_factoryFeatures = factoryFeatures;
_streamReadFeatures = parserFeatures;
_streamWriteFeatures = generatorFeatures;
Expand All @@ -161,6 +170,10 @@ protected static <T> List<T> _copy(List<T> src) {
public int streamReadFeatures() { return _streamReadFeatures; }
public int streamWriteFeatures() { return _streamWriteFeatures; }

public BufferRecyclerProvider bufferRecyclerProvider() {
return _bufferRecyclerProvider;
}

public InputDecorator inputDecorator() { return _inputDecorator; }
public OutputDecorator outputDecorator() { return _outputDecorator; }

Expand Down Expand Up @@ -300,6 +313,20 @@ public B configure(JsonWriteFeature f, boolean state) {
return _failNonJSON(f);
}

// // // Other configuration, helper objects

/**
* @param p BufferRecyclerProvider to use for buffer allocation
*
* @return this builder (for call chaining)
*
* @since 2.16
*/
public B bufferRecyclerProvider(BufferRecyclerProvider p) {
_bufferRecyclerProvider = Objects.requireNonNull(p);
return _this();
}

// // // Other configuration, decorators

public B inputDecorator(InputDecorator dec) {
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/fasterxml/jackson/core/base/ParserBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import java.util.Arrays;

import com.fasterxml.jackson.core.*;
import com.fasterxml.jackson.core.exc.StreamConstraintsException;
import com.fasterxml.jackson.core.io.IOContext;
import com.fasterxml.jackson.core.io.ContentReference;
import com.fasterxml.jackson.core.io.NumberInput;
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/fasterxml/jackson/core/json/UTF8DataInputJsonParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.fasterxml.jackson.core.json;

import java.io.*;
import java.util.Arrays;

import com.fasterxml.jackson.core.*;
import com.fasterxml.jackson.core.base.ParserBase;
Expand Down
15 changes: 15 additions & 0 deletions src/main/java/com/fasterxml/jackson/core/util/BufferRecyclerProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.fasterxml.jackson.core.util;

import com.fasterxml.jackson.core.TokenStreamFactory;

/**
* Provider for {@link BufferRecycler}s to allow pluggable and optional
* recycling of underlying input/output buffers.
*
* @since 2.16
*/
public interface BufferRecyclerProvider
extends java.io.Serializable
{
public abstract BufferRecycler acquireBufferRecycler(TokenStreamFactory forFactory);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't sure of naming for get/return etc, open to different names than acquire/release.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming wise I believe that what's wrong here is the name of the class. The fact that is a pool should be reflected in its name, so we should call it BufferRecyclerPool. Once you do this acquire and release will be the most natural choices, but of course I'm open to other suggestions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmh. Ok. Not sure how I managed to miss this comment before merging.

I agree that if there's life-cycle, "provider" is not quite the right choice.
Not sure I like "pool" but maybe that'd be better...
Let me first merge things as they are and the consider renaming.

}
34 changes: 34 additions & 0 deletions src/main/java/com/fasterxml/jackson/core/util/BufferRecyclers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.lang.ref.SoftReference;

import com.fasterxml.jackson.core.TokenStreamFactory;
import com.fasterxml.jackson.core.io.JsonStringEncoder;

/**
Expand Down Expand Up @@ -61,7 +62,11 @@ public class BufferRecyclers
* Main accessor to call for accessing possibly recycled {@link BufferRecycler} instance.
*
* @return {@link BufferRecycler} to use
*
* @deprecated Since 2.16 should use {@link BufferRecyclerProvider} abstraction instead
* of calling static methods of this class
*/
@Deprecated // since 2.16
public static BufferRecycler getBufferRecycler()
{
SoftReference<BufferRecycler> ref = _recyclerRef.get();
Expand Down Expand Up @@ -180,4 +185,33 @@ public static void quoteAsJsonText(CharSequence input, StringBuilder output) {
public static byte[] quoteAsJsonUTF8(String rawText) {
return JsonStringEncoder.getInstance().quoteAsUTF8(rawText);
}

/*
/**********************************************************************
/* Default BufferRecyclerProvider implementations
/**********************************************************************
*/

public static BufferRecyclerProvider defaultProvider() {
return ThreadLocalBufferRecyclerProvider.INSTANCE;
}

/**
* Default {@link BufferRecyclerProvider} implementation that uses
* {@link ThreadLocal} for recycling instances.
*
* @since 2.16
*/
public static class ThreadLocalBufferRecyclerProvider
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was going back and forth between creating a main-level class vs nested here; mostly located here to keep close to defaultProvider() defined above.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it's better to keep all default implementations in one place and have them internal to the main level class. By the way we will also need another implementation not pooling anything but simply returning a new BufferRecycler every time that acquire is called. I believe that this "dummy" implementation should be also placed here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about "no-op" implementation but for now it did not seem necessary. But maybe I am wrong.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. The reason -- maybe not great one -- was that the check for "do not recycle" is using a simple Feature, and by-passes provider/pool altogether.

Maybe using and passing no-op instance would make more sense, nonetheless, instead of using null provider/pool.

implements BufferRecyclerProvider
{
private static final long serialVersionUID = 1L;

public final static ThreadLocalBufferRecyclerProvider INSTANCE = new ThreadLocalBufferRecyclerProvider();

@Override
public BufferRecycler acquireBufferRecycler(TokenStreamFactory forFactory) {
return getBufferRecycler();
}
}
}