#774 Do not re-use serializers for CompatibleFieldSerializer and Tagg…

…edFieldSerializer (#788)
EsotericSoftware · Nov 16, 2020 · 94103dd · 94103dd
1 parent 04e0e90
commit 94103dd
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 4 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.");

diff --git a/src/com/esotericsoftware/kryo/serializers/FieldSerializer.java b/src/com/esotericsoftware/kryo/serializers/FieldSerializer.java
@@ -228,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;
@@ -309,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;
 		}
@@ -326,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.serializer = serializer;
+					if (valueClass != null && reuseSerializer) this.serializer = serializer;
 				}
 				kryo.getGenerics().pushGenericType(genericType);
 				if (canBeNull) {
@@ -127,7 +127,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.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);

diff --git a/test/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializerTest.java b/test/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializerTest.java
@@ -453,6 +453,38 @@ public void testClassWithSuperTypeFields() {
 		roundTrip(71, new ClassWithSuperTypeFields("foo", Arrays.asList("bar"), "baz"));
 	}
 
+	// https://github.com/EsotericSoftware/kryo/issues/774
+	@Test
+	public void testClassWithObjectField() {
+		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(12, new ClassWithObjectField(123));
+		roundTrip(13, new ClassWithObjectField("foo"));
+	}
+
+	public static class ClassWithObjectField {
+		Object value;
+
+		public ClassWithObjectField() { }
+
+		public ClassWithObjectField(Object value) {
+			this.value = value;
+		}
+
+		@Override
+		public boolean equals(Object o) {
+			if (this == o) return true;
+			if (o == null || getClass() != o.getClass()) return false;
+			ClassWithObjectField wrapper = (ClassWithObjectField) o;
+			return Objects.equals(value, wrapper.value);
+		}
+	}
+
 	public static class TestClass {
 		public String text = "something";
 		public int moo = 120;

diff --git a/test/com/esotericsoftware/kryo/serializers/TaggedFieldSerializerTest.java b/test/com/esotericsoftware/kryo/serializers/TaggedFieldSerializerTest.java
@@ -30,6 +30,7 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.util.Objects;
 
 import org.junit.Test;
 
@@ -156,6 +157,38 @@ public void testInvalidTagValue () {
 		assertTrue(receivedIAE);
 	}
 
+	// https://github.com/EsotericSoftware/kryo/issues/774
+	@Test
+	public void testClassWithObjectField() {
+		TaggedFieldSerializer<ClassWithObjectField> serializer = new TaggedFieldSerializer<>(kryo, ClassWithObjectField.class);
+		final TaggedFieldSerializer.TaggedFieldSerializerConfig config = serializer.getTaggedFieldSerializerConfig();
+		config.setChunkedEncoding(true);
+		config.setReadUnknownTagData(true);
+		kryo.register(ClassWithObjectField.class, serializer);
+
+		roundTrip(8, new ClassWithObjectField(123));
+		roundTrip(9, new ClassWithObjectField("foo"));
+	}
+
+	public static class ClassWithObjectField {
+		@Tag(0)
+		Object value;
+
+		public ClassWithObjectField() { }
+
+		public ClassWithObjectField(Object value) {
+			this.value = value;
+		}
+
+		@Override
+		public boolean equals(Object o) {
+			if (this == o) return true;
+			if (o == null || getClass() != o.getClass()) return false;
+			ClassWithObjectField wrapper = (ClassWithObjectField) o;
+			return Objects.equals(value, wrapper.value);
+		}
+	}
+
 	public static class TestClass {
 		@Tag(0) public String text = "something";
 		@Tag(1) public int moo = 120;