diff --git a/conjure-java-jackson-serialization/build.gradle b/conjure-java-jackson-serialization/build.gradle index 308b018b5..d8e39b80b 100644 --- a/conjure-java-jackson-serialization/build.gradle +++ b/conjure-java-jackson-serialization/build.gradle @@ -14,7 +14,6 @@ dependencies { api "com.palantir.safe-logging:preconditions" implementation "com.palantir.safe-logging:logger" implementation 'com.palantir.tritium:tritium-registry' - implementation 'com.github.ben-manes.caffeine:caffeine' testImplementation "junit:junit" testImplementation "org.assertj:assertj-core" diff --git a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/CaffeineCachingTypeFactory.java b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/CaffeineCachingTypeFactory.java deleted file mode 100644 index adc9ce5dc..000000000 --- a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/CaffeineCachingTypeFactory.java +++ /dev/null @@ -1,217 +0,0 @@ -/* - * (c) Copyright 2023 Palantir Technologies Inc. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.palantir.conjure.java.serialization; - -import com.codahale.metrics.Meter; -import com.fasterxml.jackson.databind.JavaType; -import com.fasterxml.jackson.databind.type.TypeFactory; -import com.fasterxml.jackson.databind.type.TypeModifier; -import com.fasterxml.jackson.databind.type.TypeParser; -import com.fasterxml.jackson.databind.util.ArrayBuilders; -import com.fasterxml.jackson.databind.util.LRUMap; -import com.fasterxml.jackson.databind.util.LookupCache; -import com.github.benmanes.caffeine.cache.Cache; -import com.github.benmanes.caffeine.cache.Caffeine; -import com.github.benmanes.caffeine.cache.RemovalCause; -import com.github.benmanes.caffeine.cache.stats.CacheStats; -import com.github.benmanes.caffeine.cache.stats.StatsCounter; -import com.google.common.primitives.Ints; -import com.palantir.logsafe.SafeArg; -import com.palantir.logsafe.logger.SafeLogger; -import com.palantir.logsafe.logger.SafeLoggerFactory; -import com.palantir.tritium.metrics.registry.SharedTaggedMetricRegistries; -import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; -import java.util.Locale; -import java.util.function.Supplier; -import javax.annotation.Nullable; -import org.checkerframework.checker.index.qual.NonNegative; - -final class CaffeineCachingTypeFactory extends TypeFactory { - private static final SafeLogger log = SafeLoggerFactory.get(CaffeineCachingTypeFactory.class); - - CaffeineCachingTypeFactory() { - super(new CaffeineLookupCache()); - } - - CaffeineCachingTypeFactory( - CaffeineLookupCache typeCache, - TypeParser parser, - @Nullable TypeModifier[] modifiers, - ClassLoader classLoader) { - super(typeCache, parser, modifiers, classLoader); - } - - private static CaffeineLookupCache asCaffeine(LookupCache typeCache) { - if (typeCache instanceof CaffeineLookupCache) { - return (CaffeineLookupCache) typeCache; - } else if (typeCache == null || typeCache.size() == 0) { - // Use a caffeine cache instead of the provided empty cache - return new CaffeineLookupCache(); - } else if (typeCache instanceof LRUMap) { - CaffeineLookupCache cache = new CaffeineLookupCache(); - LRUMap defaultJacksonCacheType = (LRUMap) typeCache; - defaultJacksonCacheType.contents(cache::put); - return cache; - } else { - log.error( - "Unknown LookupCache, using a new caffeine cache instead", - SafeArg.of("cacheType", typeCache.getClass())); - return new CaffeineLookupCache(); - } - } - - @Override - public CaffeineCachingTypeFactory withModifier(TypeModifier mod) { - // Updating modifiers clears the cache - return new CaffeineCachingTypeFactory( - new CaffeineLookupCache(), _parser, computeModifiers(_modifiers, mod), _classLoader); - } - - @Nullable - private static TypeModifier[] computeModifiers(TypeModifier[] existing, TypeModifier newModifier) { - // Semantics are based on the jackson-databind `withModifier` implementation. - if (newModifier == null) { - return null; - } - if (existing == null || existing.length == 0) { - return new TypeModifier[] {newModifier}; - } - return ArrayBuilders.insertInListNoDup(existing, newModifier); - } - - @Override - public CaffeineCachingTypeFactory withClassLoader(ClassLoader classLoader) { - return new CaffeineCachingTypeFactory(asCaffeine(_typeCache), _parser, _modifiers, classLoader); - } - - @Override - public CaffeineCachingTypeFactory withCache(LRUMap cache) { - return withCache((LookupCache) cache); - } - - @Override - public CaffeineCachingTypeFactory withCache(LookupCache cache) { - return new CaffeineCachingTypeFactory(asCaffeine(cache), _parser, _modifiers, _classLoader); - } - - private static final class CaffeineLookupCache implements LookupCache { - private final Cache cache; - - CaffeineLookupCache() { - this.cache = Caffeine.newBuilder() - // max-size 1000 up from 200 default as of 2.14.2 - .maximumSize(1000) - // initial-size 128 up from 16 default as of 2.14.2 - .initialCapacity(128) - .recordStats(InstrumentedStatsCounter.SUPPLIER) - .build(); - } - - @Override - public int size() { - return Ints.saturatedCast(cache.estimatedSize()); - } - - @Override - @Nullable - public JavaType get(Object key) { - return cache.getIfPresent(key); - } - - @Override - public JavaType put(Object key, JavaType value) { - return cache.asMap().put(key, value); - } - - @Override - public JavaType putIfAbsent(Object key, JavaType value) { - return cache.asMap().putIfAbsent(key, value); - } - - @Override - public void clear() { - cache.invalidateAll(); - } - - @Override - public String toString() { - return "CaffeineLookupCache{" + cache + '}'; - } - } - - private static final class InstrumentedStatsCounter implements StatsCounter { - // Collecting metrics without broadening APIs to require a TaggedMetricRegistry - @SuppressWarnings("deprecation") - private static final StatsCounter INSTANCE = - new InstrumentedStatsCounter(SharedTaggedMetricRegistries.getSingleton()); - - @SuppressWarnings("UnnecessaryLambda") - private static final Supplier SUPPLIER = () -> INSTANCE; - - private final Meter hits; - private final Meter misses; - // Eviction meters are based on RemovalCause ordinal - private final Meter[] evictions; - - private InstrumentedStatsCounter(TaggedMetricRegistry registry) { - JsonDatabindTypefactoryCacheMetrics metrics = JsonDatabindTypefactoryCacheMetrics.of(registry); - this.hits = metrics.hit(); - this.misses = metrics.miss(); - RemovalCause[] causes = RemovalCause.values(); - this.evictions = new Meter[causes.length]; - for (int i = 0; i < causes.length; i++) { - evictions[i] = metrics.eviction(causes[i].name().toLowerCase(Locale.ROOT)); - } - } - - @Override - public void recordHits(@NonNegative int count) { - hits.mark(count); - } - - @Override - public void recordMisses(@NonNegative int count) { - misses.mark(count); - } - - @Override - public void recordLoadSuccess(@NonNegative long _loadTime) { - // nop - } - - @Override - public void recordLoadFailure(@NonNegative long _loadTime) { - // nop - } - - @Override - public void recordEviction(@NonNegative int _weight, RemovalCause cause) { - evictions[cause.ordinal()].mark(); - } - - @Override - public CacheStats snapshot() { - // Item weight is always 1, evictions count and weight are always identical. - // We don't measure load success/failure/timing information. - long evictionsCount = 0; - for (Meter meter : evictions) { - evictionsCount += meter.getCount(); - } - return CacheStats.of(hits.getCount(), misses.getCount(), 0, 0, 0, evictionsCount, evictionsCount); - } - } -} diff --git a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/NonCachingTypeFactory.java b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/NonCachingTypeFactory.java new file mode 100644 index 000000000..335865c19 --- /dev/null +++ b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/NonCachingTypeFactory.java @@ -0,0 +1,110 @@ +/* + * (c) Copyright 2023 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.conjure.java.serialization; + +import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.type.TypeFactory; +import com.fasterxml.jackson.databind.type.TypeModifier; +import com.fasterxml.jackson.databind.type.TypeParser; +import com.fasterxml.jackson.databind.util.ArrayBuilders; +import com.fasterxml.jackson.databind.util.LRUMap; +import com.fasterxml.jackson.databind.util.LookupCache; +import javax.annotation.Nullable; + +/** + * A {@link TypeFactory} implementation which does not use a cache. + * @see jackson-databind#3876 + * @see jackson-benchmarks#5 + */ +final class NonCachingTypeFactory extends TypeFactory { + + NonCachingTypeFactory() { + super(NoCacheLookupCache.INSTANCE); + } + + NonCachingTypeFactory(TypeParser parser, @Nullable TypeModifier[] modifiers, ClassLoader classLoader) { + super(NoCacheLookupCache.INSTANCE, parser, modifiers, classLoader); + } + + @Override + public NonCachingTypeFactory withModifier(TypeModifier mod) { + return new NonCachingTypeFactory(_parser, computeModifiers(_modifiers, mod), _classLoader); + } + + @Nullable + private static TypeModifier[] computeModifiers(TypeModifier[] existing, TypeModifier newModifier) { + // Semantics are based on the jackson-databind `withModifier` implementation. + if (newModifier == null) { + return null; + } + if (existing == null || existing.length == 0) { + return new TypeModifier[] {newModifier}; + } + return ArrayBuilders.insertInListNoDup(existing, newModifier); + } + + @Override + public NonCachingTypeFactory withClassLoader(ClassLoader classLoader) { + return new NonCachingTypeFactory(_parser, _modifiers, classLoader); + } + + @Override + public NonCachingTypeFactory withCache(LRUMap _cache) { + // Changes to the cache are ignored + return this; + } + + @Override + public NonCachingTypeFactory withCache(LookupCache _cache) { + // Changes to the cache are ignored + return this; + } + + private enum NoCacheLookupCache implements LookupCache { + INSTANCE; + + @Override + public int size() { + return 0; + } + + @Override + @Nullable + public JavaType get(Object _key) { + return null; + } + + @Override + @Nullable + public JavaType put(Object _key, JavaType _value) { + return null; + } + + @Override + public JavaType putIfAbsent(Object _key, JavaType _value) { + return null; + } + + @Override + public void clear() {} + + @Override + public String toString() { + return "NoCacheLookupCache{}"; + } + } +} diff --git a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java index f23551247..06462bffc 100644 --- a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java +++ b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java @@ -170,7 +170,7 @@ public static ObjectMapper newSmileServerObjectMapper() { * */ public static > B withDefaultModules(B builder) { - return builder.typeFactory(new CaffeineCachingTypeFactory()) + return builder.typeFactory(new NonCachingTypeFactory()) .addModule(new GuavaModule()) .addModule(new ShimJdk7Module()) .addModule(new Jdk8Module().configureAbsentsAsNulls(true)) @@ -205,7 +205,7 @@ public static > B withDefa * */ public static ObjectMapper withDefaultModules(ObjectMapper mapper) { - return mapper.setTypeFactory(new CaffeineCachingTypeFactory()) + return mapper.setTypeFactory(new NonCachingTypeFactory()) .registerModule(new GuavaModule()) .registerModule(new ShimJdk7Module()) .registerModule(new Jdk8Module().configureAbsentsAsNulls(true)) diff --git a/conjure-java-jackson-serialization/src/main/metrics/jackson-metrics.yml b/conjure-java-jackson-serialization/src/main/metrics/jackson-metrics.yml index ec3947fce..83c56308b 100644 --- a/conjure-java-jackson-serialization/src/main/metrics/jackson-metrics.yml +++ b/conjure-java-jackson-serialization/src/main/metrics/jackson-metrics.yml @@ -10,16 +10,3 @@ namespaces: tags: - format docs: Histogram describing the length of strings parsed from input. - json.databind.typefactory.cache: - docs: Metrics produced by the Jackson Databind TypeFactory cache. - metrics: - hit: - type: meter - docs: Rate at which cache lookups are successful. - miss: - type: meter - docs: Rate at which cache lookups miss and require computation. - eviction: - type: meter - docs: Rate at which cache entries are removed, tagged by the cause for removal. - tags: [reason] diff --git a/conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java b/conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java index 880567a88..94e321363 100644 --- a/conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java +++ b/conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java @@ -20,7 +20,6 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.codahale.metrics.Histogram; -import com.codahale.metrics.Meter; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonParseException; @@ -31,7 +30,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.exc.InvalidFormatException; import com.fasterxml.jackson.databind.json.JsonMapper; -import com.fasterxml.jackson.databind.type.TypeFactory; import com.fasterxml.jackson.dataformat.smile.SmileFactory; import com.palantir.logsafe.Preconditions; import com.palantir.tritium.metrics.registry.SharedTaggedMetricRegistries; @@ -425,36 +423,7 @@ public void testTypeFactoryCache() { } private void testTypeFactory(ObjectMapper mapper) { - assertThat(mapper.getTypeFactory()).isInstanceOf(CaffeineCachingTypeFactory.class); - } - - @Test - public void testTypeFactoryCacheMetrics() { - TaggedMetricRegistry registry = SharedTaggedMetricRegistries.getSingleton(); - JsonDatabindTypefactoryCacheMetrics metrics = JsonDatabindTypefactoryCacheMetrics.of(registry); - Meter hit = metrics.hit(); - Meter miss = metrics.miss(); - TypeFactory typeFactory = ObjectMappers.newServerJsonMapper().getTypeFactory(); - - long hitBeforeFirst = hit.getCount(); - long missBeforeFirst = miss.getCount(); - typeFactory.constructType(SimpleSerializable.class); - assertThat(miss.getCount() - missBeforeFirst).isOne(); - assertThat(hit.getCount() - hitBeforeFirst).isZero(); - // After writing the same type again, we should observe hits and no additional misses. - long hitBeforeSecond = hit.getCount(); - long missBeforeSecond = miss.getCount(); - typeFactory.constructType(SimpleSerializable.class); - assertThat(miss.getCount() - missBeforeSecond).isZero(); - assertThat(hit.getCount() - hitBeforeSecond).isOne(); - } - - static final class SimpleSerializable { - @Override - @JsonProperty("str") - public String toString() { - return "stringValue"; - } + assertThat(mapper.getTypeFactory()).isInstanceOf(NonCachingTypeFactory.class); } @Test