#774 Do not re-use serializer instances for Compatible and Tagged…

… serializers in `ReflectField`
EsotericSoftware · Nov 16, 2020 · fcbf217 · fcbf217
1 parent 5e29ca3
commit fcbf217
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 31 deletions.
diff --git a/src/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializer.java b/src/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializer.java
@@ -47,7 +47,7 @@
 public class CompatibleFieldSerializer<T> extends FieldSerializer<T> {
 	private static final int binarySearchThreshold = 32;
 
-	private CompatibleFieldSerializerConfig config;
+	private final CompatibleFieldSerializerConfig config;
 
 	public CompatibleFieldSerializer (Kryo kryo, Class type) {
 		this(kryo, type, new CompatibleFieldSerializerConfig());
@@ -102,6 +102,7 @@ public void write (Kryo kryo, Output output, T object) {
 				}
 				cachedField.setCanBeNull(false);
 				cachedField.setValueClass(valueClass);
+				cachedField.setReuseSerializer(false);
 			}
 
 			cachedField.write(fieldOutput, object);
@@ -174,6 +175,7 @@ public T read (Kryo kryo, Input input, Class<? extends T> type) {
 
 				cachedField.setCanBeNull(false);
 				cachedField.setValueClass(valueClass);
+				cachedField.setReuseSerializer(false);
 			} else if (cachedField == null) {
 				if (!chunked) throw new KryoException("Unknown field. (" + getType().getName() + ")");
 				if (TRACE) trace("kryo", "Skip unknown field.");
@@ -242,11 +244,6 @@ else if (compare > 0)
 		return fields;
 	}
 
-	@Override
-	boolean supportsReuse() {
-		return !config.readUnknownFieldData;
-	}
-
 	public CompatibleFieldSerializerConfig getCompatibleFieldSerializerConfig () {
 		return config;
 	}

diff --git a/src/com/esotericsoftware/kryo/serializers/FieldSerializer.java b/src/com/esotericsoftware/kryo/serializers/FieldSerializer.java
@@ -211,11 +211,6 @@ protected T createCopy (Kryo kryo, T original) {
 		return (T)kryo.newInstance(original.getClass());
 	}
 
-	/** If true, the serializer supports re-use of serializers for all instances of a field */
-	boolean supportsReuse() {
-		return true;
-	}
-
 	@Override
 	public T copy (Kryo kryo, T original) {
 		T copy = createCopy(kryo, original);
@@ -233,7 +228,7 @@ public abstract static class CachedField {
 		String name;
 		Class valueClass;
 		Serializer serializer;
-		boolean canBeNull, varEncoding = true, optimizePositive;
+		boolean canBeNull, varEncoding = true, optimizePositive, reuseSerializer = true;
 
 		// For AsmField.
 		FieldAccess access;
@@ -314,6 +309,18 @@ public boolean getOptimizePositive () {
 			return optimizePositive;
 		}
 
+		/** When true, serializers are re-used for all instances of the field if the {@link #valueClass} is known. Re-using
+		 * serializers is significantly faster than looking them up for every read/write. However, this only works reliably
+		 * when the {@link #valueClass} of the field never changes. Serializers that do not guarantee this must set the flag to
+		 * false. */
+		void setReuseSerializer(boolean reuseSerializer) {
+			this.reuseSerializer = reuseSerializer;
+		}
+
+		boolean getReuseSerializer() {
+			return reuseSerializer;
+		}
+
 		public String getName () {
 			return name;
 		}
@@ -331,6 +338,7 @@ public String toString () {
 		public abstract void read (Input input, Object object);
 
 		public abstract void copy (Object original, Object copy);
+
 	}
 
 	/** Indicates a field should be ignored when its declaring class is registered unless the {@link Kryo#getContext() context} has

diff --git a/src/com/esotericsoftware/kryo/serializers/ReflectField.java b/src/com/esotericsoftware/kryo/serializers/ReflectField.java
@@ -73,7 +73,7 @@ public void write (Output output, Object object) {
 				if (serializer == null) {
 					serializer = kryo.getSerializer(concreteType);
 					// The concrete type of the field is known, always use the same serializer.
-					if (valueClass != null && this.fieldSerializer.supportsReuse()) this.serializer = serializer;
+					if (valueClass != null && reuseSerializer) this.serializer = serializer;
 				}
 				kryo.getGenerics().pushGenericType(genericType);
 				if (canBeNull) {
@@ -121,7 +121,7 @@ public void read (Input input, Object object) {
 				if (serializer == null) {
 					serializer = kryo.getSerializer(concreteType);
 					// The concrete type of the field is known, always use the same serializer.
-					if (valueClass != null && this.fieldSerializer.supportsReuse()) this.serializer = serializer;
+					if (valueClass != null && reuseSerializer) this.serializer = serializer;
 				}
 				kryo.getGenerics().pushGenericType(genericType);
 				if (canBeNull)

diff --git a/src/com/esotericsoftware/kryo/serializers/TaggedFieldSerializer.java b/src/com/esotericsoftware/kryo/serializers/TaggedFieldSerializer.java
@@ -157,6 +157,7 @@ public void write (Kryo kryo, Output output, T object) {
 				}
 				cachedField.setCanBeNull(false);
 				cachedField.setValueClass(valueClass);
+				cachedField.setReuseSerializer(false);
 			}
 
 			cachedField.write(fieldOutput, object);
@@ -226,6 +227,7 @@ public T read (Kryo kryo, Input input, Class<? extends T> type) {
 				}
 				cachedField.setCanBeNull(false);
 				cachedField.setValueClass(valueClass);
+				cachedField.setReuseSerializer(false);
 			} else if (cachedField == null) {
 				if (!chunked) throw new KryoException("Unknown field tag: " + tag + " (" + getType().getName() + ")");
 				if (TRACE) trace("kryo", "Skip unknown field tag: " + tag);
@@ -242,11 +244,6 @@ public T read (Kryo kryo, Input input, Class<? extends T> type) {
 		return object;
 	}
 
-	@Override
-	boolean supportsReuse() {
-		return !config.readUnknownTagData;
-	}
-
 	public TaggedFieldSerializerConfig getTaggedFieldSerializerConfig () {
 		return config;
 	}

diff --git a/test/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializerTest.java b/test/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializerTest.java
@@ -24,7 +24,6 @@
 import com.esotericsoftware.kryo.Kryo;
 import com.esotericsoftware.kryo.KryoException;
 import com.esotericsoftware.kryo.KryoTestCase;
-import com.esotericsoftware.kryo.SerializerFactory;
 import com.esotericsoftware.kryo.SerializerFactory.CompatibleFieldSerializerFactory;
 
 import java.io.Serializable;
@@ -457,12 +456,15 @@ public void testClassWithSuperTypeFields() {
 	// https://github.com/EsotericSoftware/kryo/issues/774
 	@Test
 	public void testClassWithObjectField() {
-		kryo.setRegistrationRequired(false);
-		kryo.setDefaultSerializer(new SerializerFactory.CompatibleFieldSerializerFactory(
-				new CompatibleFieldSerializer.CompatibleFieldSerializerConfig()));
+		CompatibleFieldSerializer<ClassWithSuperTypeFields> serializer = new CompatibleFieldSerializer<>(kryo,
+				ClassWithObjectField.class);
+		CompatibleFieldSerializer.CompatibleFieldSerializerConfig config = serializer.getCompatibleFieldSerializerConfig();
+		config.setChunkedEncoding(true);
+		config.setReadUnknownFieldData(true);
+		kryo.register(ClassWithObjectField.class, serializer);
 
-		roundTrip(99, new ClassWithObjectField(123));
-		roundTrip(100, new ClassWithObjectField("foo"));
+		roundTrip(12, new ClassWithObjectField(123));
+		roundTrip(13, new ClassWithObjectField("foo"));
 	}
 
 	public static class ClassWithObjectField {

diff --git a/test/com/esotericsoftware/kryo/serializers/TaggedFieldSerializerTest.java b/test/com/esotericsoftware/kryo/serializers/TaggedFieldSerializerTest.java
@@ -23,7 +23,6 @@
 
 import com.esotericsoftware.kryo.Kryo;
 import com.esotericsoftware.kryo.KryoTestCase;
-import com.esotericsoftware.kryo.SerializerFactory;
 import com.esotericsoftware.kryo.SerializerFactory.TaggedFieldSerializerFactory;
 import com.esotericsoftware.kryo.io.Input;
 import com.esotericsoftware.kryo.io.Output;
@@ -161,13 +160,14 @@ public void testInvalidTagValue () {
 	// https://github.com/EsotericSoftware/kryo/issues/774
 	@Test
 	public void testClassWithObjectField() {
-		kryo.setRegistrationRequired(false);
-		final TaggedFieldSerializer.TaggedFieldSerializerConfig config = new TaggedFieldSerializer.TaggedFieldSerializerConfig();
+		TaggedFieldSerializer<ClassWithObjectField> serializer = new TaggedFieldSerializer<>(kryo, ClassWithObjectField.class);
+		final TaggedFieldSerializer.TaggedFieldSerializerConfig config = serializer.getTaggedFieldSerializerConfig();
+		config.setChunkedEncoding(true);
 		config.setReadUnknownTagData(true);
-		kryo.setDefaultSerializer(new SerializerFactory.TaggedFieldSerializerFactory(config));
+		kryo.register(ClassWithObjectField.class, serializer);
 
-		roundTrip(91, new ClassWithObjectField(123));
-		roundTrip(92, new ClassWithObjectField("foo"));
+		roundTrip(8, new ClassWithObjectField(123));
+		roundTrip(9, new ClassWithObjectField("foo"));
 	}
 
 	public static class ClassWithObjectField {