Skip to content

Commit

Permalink
Opt out of the TypeFactory cache until we can resolve cache contention
Browse files Browse the repository at this point in the history
FasterXML/jackson-databind#3876
FasterXML/jackson-benchmarks#5

The linked benchmarks show a small advantage of caching over
not caching in most cases, in the best case for a cache: when
a single operation is repetedly executed. In practice, the cache
is used for all operations and sees heavy evictions, where contention
is more likely and has greater impact on users.

Given the hash collision issues that we're aware of, and the impacts
we've observed, we will remove the cache until a version is available
which isn't impacted by the linked issue for performance that's
more predictable.
  • Loading branch information
carterkozak committed Apr 12, 2023
1 parent 0852a78 commit c976bd1
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 265 deletions.
1 change: 0 additions & 1 deletion conjure-java-jackson-serialization/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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 <a href="https://github.com/FasterXML/jackson-databind/issues/3876">jackson-databind#3876</a>
* @see <a href="https://github.com/FasterXML/jackson-benchmarks/pull/5">jackson-benchmarks#5</a>
*/
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<Object, JavaType> _cache) {
// Changes to the cache are ignored
return this;
}

@Override
public NonCachingTypeFactory withCache(LookupCache<Object, JavaType> _cache) {
// Changes to the cache are ignored
return this;
}

private enum NoCacheLookupCache implements LookupCache<Object, JavaType> {
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{}";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public static ObjectMapper newSmileServerObjectMapper() {
* </ul>
*/
public static <M extends ObjectMapper, B extends MapperBuilder<M, B>> 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))
Expand Down Expand Up @@ -205,7 +205,7 @@ public static <M extends ObjectMapper, B extends MapperBuilder<M, B>> B withDefa
* </ul>
*/
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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Loading

0 comments on commit c976bd1

Please sign in to comment.