From 0c0a0b461fa3b1b3ccbb9ea5a180d7a04fdf208b Mon Sep 17 00:00:00 2001 From: Bertrand Renuart Date: Wed, 8 Sep 2021 21:41:12 +0200 Subject: [PATCH] Cache JsonSerializer "per" ObjectMapper - Maintain a separate cache of JsonProvider per ObjectMapper - do not create a new instance of SerializerProvider ourselves but instead use ObjectMapper#getSerializerProviderInstance introduced in Jackson 2.7 - do not create a JsonSerializer for the object class ourselves but instead delegate to Jackson SerializerProvider and therefore leverage its internal cache Addresses issue #642 --- .../marker/ObjectFieldsAppendingMarker.java | 127 +++++++++--------- 1 file changed, 65 insertions(+), 62 deletions(-) diff --git a/src/main/java/net/logstash/logback/marker/ObjectFieldsAppendingMarker.java b/src/main/java/net/logstash/logback/marker/ObjectFieldsAppendingMarker.java index e97d89c2..6f38d074 100755 --- a/src/main/java/net/logstash/logback/marker/ObjectFieldsAppendingMarker.java +++ b/src/main/java/net/logstash/logback/marker/ObjectFieldsAppendingMarker.java @@ -16,6 +16,7 @@ package net.logstash.logback.marker; import java.io.IOException; +import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; @@ -28,10 +29,7 @@ import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.JsonSerializer; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.SerializationConfig; import com.fasterxml.jackson.databind.SerializerProvider; -import com.fasterxml.jackson.databind.ser.DefaultSerializerProvider; -import com.fasterxml.jackson.databind.ser.ResolvableSerializer; import com.fasterxml.jackson.databind.util.NameTransformer; import org.slf4j.Marker; @@ -91,8 +89,7 @@ public class ObjectFieldsAppendingMarker extends LogstashMarker implements Struc * * Since apps will typically serialize the same types of objects repeatedly, they shouldn't grow too much. */ - private static final ConcurrentHashMap, JsonSerializer> beanSerializers = new ConcurrentHashMap, JsonSerializer>(); - private static final ConcurrentHashMap serializerProviders = new ConcurrentHashMap(); + private static final Map serializerHelper = new ConcurrentHashMap<>(); public ObjectFieldsAppendingMarker(Object object) { super(MARKER_NAME); @@ -101,70 +98,18 @@ public ObjectFieldsAppendingMarker(Object object) { @Override public void writeTo(JsonGenerator generator) throws IOException { - if (object != null) { - ObjectMapper mapper = (ObjectMapper) generator.getCodec(); - JsonSerializer serializer = getBeanSerializer(mapper); - if (serializer.isUnwrappingSerializer()) { - serializer.serialize(object, generator, getSerializerProvider(mapper)); - } + if (this.object != null) { + SerializerHelper helper = getSerializerHelper(generator); + helper.serialize(generator, this.object); } } - + + @Override public String toStringSelf() { return StructuredArguments.toString(object); } - /** - * Gets a serializer that will write the {@link #object} unwrapped. - */ - private JsonSerializer getBeanSerializer(ObjectMapper mapper) throws JsonMappingException { - - JsonSerializer jsonSerializer = beanSerializers.get(object.getClass()); - - if (jsonSerializer == null) { - SerializerProvider serializerProvider = getSerializerProvider(mapper); - JsonSerializer newSerializer = mapper.getSerializerFactory().createSerializer( - serializerProvider, - mapper.getSerializationConfig().constructType(object.getClass())) - .unwrappingSerializer(NameTransformer.NOP); - - if (newSerializer instanceof ResolvableSerializer) { - ((ResolvableSerializer) newSerializer).resolve(serializerProvider); - } - - JsonSerializer existingSerializer = beanSerializers.putIfAbsent( - object.getClass(), - newSerializer); - - jsonSerializer = (existingSerializer == null) ? newSerializer : existingSerializer; - } - return jsonSerializer; - - } - - /** - * Gets a {@link SerializerProvider} configured with the {@link ObjectMapper}'s {@link SerializationConfig} - * ({@link ObjectMapper#getSerializationConfig()}) to be used for serialization. - *

- * Note that the {@link ObjectMapper}'s {@link SerializerProvider} ({@link ObjectMapper#getSerializerProvider()}) - * cannot be used directly, because the {@link SerializerProvider}'s {@link SerializationConfig} ({@link SerializerProvider#getConfig()}) is null, - * which causes NullPointerExceptions when it is used. - */ - private SerializerProvider getSerializerProvider(ObjectMapper mapper) { - - SerializerProvider provider = serializerProviders.get(mapper); - if (provider == null) { - - SerializerProvider newProvider = ((DefaultSerializerProvider) mapper.getSerializerProvider()) - .createInstance(mapper.getSerializationConfig(), mapper.getSerializerFactory()); - - SerializerProvider existingProvider = serializerProviders.putIfAbsent(mapper, newProvider); - - provider = (existingProvider == null) ? newProvider : existingProvider; - } - return provider; - } @Override public boolean equals(Object obj) { @@ -182,6 +127,7 @@ public boolean equals(Object obj) { return Objects.equals(this.object, other.object); } + @Override public int hashCode() { final int prime = 31; @@ -190,4 +136,61 @@ public int hashCode() { result = prime * result + (this.object == null ? 0 : this.object.hashCode()); return result; } + + + /** + * Get a {@link SerializerHelper} suitable for use with the given {@link JsonGenerator}. + * + * @param gen the {@link JsonGenerator} for which an helper should be returned + * @return a {@link SerializerHelper} + */ + private static SerializerHelper getSerializerHelper(JsonGenerator gen) { + ObjectMapper mapper = (ObjectMapper) gen.getCodec(); + return serializerHelper.computeIfAbsent(mapper, SerializerHelper::new); + } + + private static class SerializerHelper { + private final SerializerProvider serializers; + private final ConcurrentHashMap, JsonSerializer> cache = new ConcurrentHashMap<>(); + + SerializerHelper(ObjectMapper mapper) { + this.serializers = mapper.getSerializerProviderInstance(); + } + + /** + * Serialize the given value using the supplied generator + * + * @param gen the {@link JsonGenerator} to use to serialize the value + * @param value the value to serialize + * @throws IOException thrown when the underlying {@link JsonGenerator} could not be created + * or when it has problems to serialize the given value + */ + public void serialize(JsonGenerator gen, Object value) throws IOException { + if (value != null) { + JsonSerializer unwrappingSerializer = getUnwrappingSerializer(value.getClass()); + + /* + * Make sure the serializer accepts to serialize in an "unwrapped" fashion. + * This may not be the case for serializer for Number types for instance. + */ + if (unwrappingSerializer.isUnwrappingSerializer()) { + unwrappingSerializer.serialize(value, gen, serializers); + } + } + } + + private JsonSerializer getUnwrappingSerializer(Class type) throws JsonMappingException { + JsonSerializer serializer = cache.get(type); + if (serializer == null) { + serializer = createUnwrappingSerializer(type); + cache.put(type, serializer); + } + return serializer; + } + + private JsonSerializer createUnwrappingSerializer(Class type) throws JsonMappingException { + JsonSerializer serializer = serializers.findValueSerializer(type); + return serializer.unwrappingSerializer(NameTransformer.NOP); + } + } }