Skip to content

Add a way to configure TypeFactory Jackson uses #4115

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 6 commits into from
Sep 26, 2023
Merged

Add a way to configure TypeFactory Jackson uses #4115

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
47164ab
Implement all
JooHyukKim Sep 12, 2023
fe64525
Add deprecate "note" in constructor
JooHyukKim Sep 12, 2023
7bf1c4b
Merge remote-tracking branch 'upstream/2.16' into 2502-Add-TypeFactor…
JooHyukKim Sep 23, 2023
ae11701
Clean up test
JooHyukKim Sep 23, 2023
eed6b6f
Update DefaultCacheProvider.java
JooHyukKim Sep 24, 2023
ce900a8
Update DefaultCacheProvider.java
JooHyukKim Sep 24, 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
1 change: 1 addition & 0 deletions src/main/java/com/fasterxml/jackson/databind/ObjectMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2297,6 +2297,7 @@ public ObjectMapper setCacheProvider(CacheProvider cacheProvider) {
_serializationConfig = _serializationConfig.with(cacheProvider);
_deserializationContext = _deserializationContext.withCaches(cacheProvider);
_serializerProvider = _serializerProvider.withCaches(cacheProvider);
_typeFactory = _typeFactory.withCaches(cacheProvider);
return this;
}

Expand Down
10 changes: 9 additions & 1 deletion src/main/java/com/fasterxml/jackson/databind/cfg/CacheProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,19 @@ public interface CacheProvider
* @return {@link LookupCache} instance for constructing {@link DeserializerCache}.
*/
LookupCache<JavaType, JsonDeserializer<Object>> forDeserializerCache(DeserializationConfig config);

/**
* Method to provide a {@link LookupCache} instance for constructing {@link com.fasterxml.jackson.databind.ser.SerializerCache}.
*
* @return {@link LookupCache} instance for constructing {@link com.fasterxml.jackson.databind.ser.SerializerCache}.
*/
LookupCache<TypeKey, JsonSerializer<Object>> forSerializerCache(SerializationConfig config);

/**
* Method to provide a {@link LookupCache} instance for constructing {@link com.fasterxml.jackson.databind.type.TypeFactory}.
*
* @return {@link LookupCache} instance for constructing {@link com.fasterxml.jackson.databind.type.TypeFactory}.
*/
LookupCache<Object, JavaType> forTypeFactory();

}
44 changes: 40 additions & 4 deletions src/main/java/com/fasterxml/jackson/databind/cfg/DefaultCacheProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.deser.DeserializerCache;
import com.fasterxml.jackson.databind.ser.SerializerCache;
import com.fasterxml.jackson.databind.type.TypeFactory;
import com.fasterxml.jackson.databind.util.LRUMap;
import com.fasterxml.jackson.databind.util.LookupCache;
import com.fasterxml.jackson.databind.util.TypeKey;
Expand All @@ -21,7 +22,7 @@ public class DefaultCacheProvider
private static final long serialVersionUID = 1L;

private final static DefaultCacheProvider DEFAULT
= new DefaultCacheProvider(DeserializerCache.DEFAULT_MAX_CACHE_SIZE, SerializerCache.DEFAULT_MAX_CACHE_SIZE);
= new DefaultCacheProvider(DeserializerCache.DEFAULT_MAX_CACHE_SIZE, SerializerCache.DEFAULT_MAX_CACHE_SIZE, TypeFactory.DEFAULT_MAX_CACHE_SIZE);

/**
* Maximum size of the {@link LookupCache} instance constructed by {@link #forDeserializerCache(DeserializationConfig)}.
Expand All @@ -36,17 +37,25 @@ public class DefaultCacheProvider
* @see Builder#maxSerializerCacheSize(int)
*/
protected final int _maxSerializerCacheSize;


/**
* Maximum size of the {@link LookupCache} instance constructed by {@link #forTypeFactory()}.
*
* @see Builder#maxTypeFactoryCacheSize(int)
*/
protected final int _maxTypeFactoryCacheSize;

/*
/**********************************************************************
/* Life cycle
/**********************************************************************
*/

protected DefaultCacheProvider(int maxDeserializerCacheSize, int maxSerializerCacheSize)
protected DefaultCacheProvider(int maxDeserializerCacheSize, int maxSerializerCacheSize, int maxTypeFactoryCacheSize)
{
_maxDeserializerCacheSize = maxDeserializerCacheSize;
_maxSerializerCacheSize = maxSerializerCacheSize;
_maxTypeFactoryCacheSize = maxTypeFactoryCacheSize;
}

/*
Expand Down Expand Up @@ -84,6 +93,11 @@ public LookupCache<TypeKey, JsonSerializer<Object>> forSerializerCache(Serializa
return _buildCache(_maxSerializerCacheSize);
}

@Override
public LookupCache<Object, JavaType> forTypeFactory() {
return _buildCache(_maxTypeFactoryCacheSize);
}

/*
/**********************************************************
/* Overridable factory methods
Expand Down Expand Up @@ -127,6 +141,12 @@ public static class Builder {
*/
private int _maxSerializerCacheSize;

/**
* Maximum Size of the {@link LookupCache} instance created by {@link #forTypeFactory()}.
* Corresponds to {@link DefaultCacheProvider#_maxTypeFactoryCacheSize}.
*/
private int _maxTypeFactoryCacheSize;

Builder() { }

/**
Expand Down Expand Up @@ -163,13 +183,29 @@ public Builder maxSerializerCacheSize(int maxSerializerCacheSize) {
return this;
}

/**
* Define the maximum size of the {@link LookupCache} instance constructed by {@link #forTypeFactory()}.
* The cache is instantiated as:
*
* @param maxTypeFactoryCacheSize Size for the {@link LookupCache} to use within {@link com.fasterxml.jackson.databind.type.TypeFactory}
* @return this builder
* @throws IllegalArgumentException if {@code maxTypeFactoryCacheSize} is negative
*/
public Builder maxTypeFactoryCacheSize(int maxTypeFactoryCacheSize) {
if (maxTypeFactoryCacheSize < 0) {
throw new IllegalArgumentException("Cannot set maxTypeFactoryCacheSize to a negative value");
}
_maxTypeFactoryCacheSize = maxTypeFactoryCacheSize;
return this;
}

/**
* Constructs a {@link DefaultCacheProvider} with the provided configuration values, using defaults where not specified.
*
* @return A {@link DefaultCacheProvider} instance with the specified configuration
*/
public DefaultCacheProvider build() {
return new DefaultCacheProvider(_maxDeserializerCacheSize, _maxSerializerCacheSize);
return new DefaultCacheProvider(_maxDeserializerCacheSize, _maxSerializerCacheSize, _maxTypeFactoryCacheSize);
}
}
}
26 changes: 24 additions & 2 deletions src/main/java/com/fasterxml/jackson/databind/type/TypeFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.cfg.CacheProvider;
import com.fasterxml.jackson.databind.util.ArrayBuilders;
import com.fasterxml.jackson.databind.util.ClassUtil;
import com.fasterxml.jackson.databind.util.LRUMap;
Expand Down Expand Up @@ -70,6 +71,15 @@ public class TypeFactory // note: was final in 2.9, removed from 2.10
{
private static final long serialVersionUID = 1L;

/**
* Default size used to construct {@link #_typeCache}.
*
* Used to be passed inline.
*
* @since 2.16
*/
public static final int DEFAULT_MAX_CACHE_SIZE = 200;

private final static JavaType[] NO_TYPES = new JavaType[0];

/**
Expand Down Expand Up @@ -178,7 +188,7 @@ public class TypeFactory // note: was final in 2.9, removed from 2.10
*/

private TypeFactory() {
this(new LRUMap<>(16, 200));
this(new LRUMap<>(16, DEFAULT_MAX_CACHE_SIZE));
}

/**
Expand All @@ -198,7 +208,7 @@ protected TypeFactory(LookupCache<Object,JavaType> typeCache, TypeParser p,
TypeModifier[] mods, ClassLoader classLoader)
{
if (typeCache == null) {
typeCache = new LRUMap<>(16, 200);
typeCache = new LRUMap<>(16, DEFAULT_MAX_CACHE_SIZE);
}
_typeCache = typeCache;
// As per [databind#894] must ensure we have back-linkage from TypeFactory:
Expand Down Expand Up @@ -260,11 +270,23 @@ public TypeFactory withCache(LRUMap<Object,JavaType> cache) {
* bigger maximum size.
*
* @since 2.12
* @deprecated Since 2.16. Use {@link #withCaches(CacheProvider)} instead.
*/
public TypeFactory withCache(LookupCache<Object,JavaType> cache) {
return new TypeFactory(cache, _parser, _modifiers, _classLoader);
}

/**
* Mutant factory method that will construct a new {@link TypeFactory}
* with cache instances provided by {@link CacheProvider}.
*
* @since 2.16
*/
public TypeFactory withCaches(CacheProvider cacheProvider) {
Copy link
Member

Choose a reason for hiding this comment

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

I think I'll change this so we won't need a new method -- caller can just construct LookupCache needed and pass that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure! Ill check your commit later👍🏼👍🏼

return new TypeFactory(cacheProvider.forTypeFactory(),
_parser, _modifiers, _classLoader);
}

/**
* Method used to access the globally shared instance, which has
* no custom configuration. Used by {@code ObjectMapper} to
Expand Down
69 changes: 68 additions & 1 deletion src/test/java/com/fasterxml/jackson/databind/cfg/CacheProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.fasterxml.jackson.databind.util.LRUMap;
import com.fasterxml.jackson.databind.util.LookupCache;
import com.fasterxml.jackson.databind.util.TypeKey;
import java.util.List;
import org.junit.Test;

import java.util.HashMap;
Expand Down Expand Up @@ -90,6 +91,11 @@ public LookupCache<JavaType, JsonDeserializer<Object>> forDeserializerCache(Dese
return _cache;
}

@Override
public LookupCache<Object, JavaType> forTypeFactory() {
return new LRUMap<>(16, 64);
}

@Override
public LookupCache<TypeKey, JsonSerializer<Object>> forSerializerCache(SerializationConfig config) {
return new LRUMap<>(8, 64);
Expand All @@ -110,6 +116,10 @@ public LookupCache<JavaType, JsonDeserializer<Object>> forDeserializerCache(Dese
}

@Override
public LookupCache<Object, JavaType> forTypeFactory() {
return new LRUMap<>(16, 64);
}

public LookupCache<TypeKey, JsonSerializer<Object>> forSerializerCache(SerializationConfig config) {
return _cache;
}
Expand All @@ -128,6 +138,40 @@ public JsonSerializer<Object> put(TypeKey key, JsonSerializer<Object> value) {
return super.put(key, value);
}
}

static class CustomTypeFactoryCacheProvider implements CacheProvider {

final CustomTestTypeFactoryCache _cache = new CustomTestTypeFactoryCache();

@Override
public LookupCache<JavaType, JsonDeserializer<Object>> forDeserializerCache(DeserializationConfig config) {
return new LRUMap<>(16, 64);
}

@Override
public LookupCache<Object, JavaType> forTypeFactory() {
return _cache;
}

public LookupCache<TypeKey, JsonSerializer<Object>> forSerializerCache(SerializationConfig config) {
return new LRUMap<>(16, 64);
}
}

static class CustomTestTypeFactoryCache extends LRUMap<Object,JavaType> {

public boolean _isInvoked = false;

public CustomTestTypeFactoryCache() {
super(8, 16);
}

@Override
public JavaType putIfAbsent(Object key, JavaType value) {
_isInvoked = true;
return super.putIfAbsent(key, value);
}
}

/*
/**********************************************************************
Expand Down Expand Up @@ -186,7 +230,7 @@ public void testDefaultCacheProviderSharesCache() throws Exception
.cacheProvider(cacheProvider)
.build();

// Act
// Act
// 3. Add two different types to each mapper cache
mapper1.readValue("{\"point\":24}", RandomBean.class);
mapper2.readValue("{\"height\":24}", AnotherBean.class);
Expand All @@ -205,10 +249,12 @@ public void testBuilderValueValidation() throws Exception
DefaultCacheProvider.builder()
.maxDeserializerCacheSize(0)
.maxSerializerCacheSize(0)
.maxTypeFactoryCacheSize(0)
Copy link
Member

@cowtowncoder cowtowncoder Sep 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

One thing I wonder: I think that most cache implementations impose a minimum size, so 0 (for example) won't do what user might want -- that is, disabling of caching.

Then again, it is now possibly to add custom cache instances too which can be "no-op" (do nothing) so maybe this doesn't matter.

I also haven't tested out if backing LRUMap honors 0 size or not: JDK Maps do have minimum size but since 2.14 LRUMap uses integrated version of a more optimized caching library (ConcurrentLinkedHashMap).

Copy link
Contributor

Choose a reason for hiding this comment

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

LRUMap does honor max size of 0 (a write occurs, but is immediately evicted). LRUMap is based off ConcurrentLinkedHashMap, which is a predecessor to Caffeine

Caffeine, on the other hand, batches evictions, so the zero maximum size constraint can be temporarily violated.

In addition, Cache2k, Android LruCache, and Ehcache all throw IllegalArgumentException when trying to create a cache with a maximum size of zero.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you @iProdigy . Sounds like we are good then. So 0 size for 2.14+ Jackson implementation will not retain anything in cache, and while there is write overhead that should be negligible.

It'd be nice to test this invariant, but no further work should be necessary it sounds.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm, I am trying to come up with JavaDoc like...

 * Note that setting value of {@code ...} to zero will not .... unlike some Cache providers.

....but idk what to exactly say 🥲 any help?

Copy link
Member Author

@JooHyukKim JooHyukKim Sep 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Added JavaDoc and tagged you there.

.build();
DefaultCacheProvider.builder()
.maxDeserializerCacheSize(Integer.MAX_VALUE)
.maxSerializerCacheSize(Integer.MAX_VALUE)
.maxTypeFactoryCacheSize(Integer.MAX_VALUE)
.build();

// fail cases
Expand All @@ -224,6 +270,12 @@ public void testBuilderValueValidation() throws Exception
} catch (IllegalArgumentException e) {
assertTrue(e.getMessage().contains("Cannot set maxSerializerCacheSize to a negative value"));
}
try {
DefaultCacheProvider.builder().maxTypeFactoryCacheSize(-1);
fail("Should not reach here");
} catch (IllegalArgumentException e) {
assertTrue(e.getMessage().contains("Cannot set maxTypeFactoryCacheSize to a negative value"));
}
}

/**
Expand Down Expand Up @@ -258,4 +310,19 @@ private void _verifySerializeSuccess(CacheProvider cacheProvider) throws Excepti
assertEquals("{\"slide\":123}",
mapper.writeValueAsString(new SerBean()));
}

/**
* Sanity test for serialization with {@link CacheProvider#forTypeFactory()}
*/
@Test
public void sanityCheckTypeFactoryCacheSize() throws Exception
{
// custom
CustomTypeFactoryCacheProvider customProvider = new CustomTypeFactoryCacheProvider();
ObjectMapper mapper = JsonMapper.builder()
.cacheProvider(customProvider)
.build();
mapper.getTypeFactory().constructParametricType(List.class, HashMap.class);
assertTrue(customProvider._cache._isInvoked);
}
}