From 7f789b72ff756dd2e067a7c1aaea1b5d1913dc0a Mon Sep 17 00:00:00 2001 From: Thomas Heigl Date: Fri, 23 Dec 2022 11:20:12 +0100 Subject: [PATCH] Cache components, getters and constructors in `RecordSerializer` (#927) * Cache components, getters and constructors in `RecordSerializer` * Use `ClassValue` and restore default constructor for backwards compatibility --- .../kryo/serializers/RecordSerializer.java | 120 +++++++++++------- .../serializers/RecordSerializerTest.java | 2 +- 2 files changed, 76 insertions(+), 46 deletions(-) diff --git a/src/com/esotericsoftware/kryo/serializers/RecordSerializer.java b/src/com/esotericsoftware/kryo/serializers/RecordSerializer.java index dd278edd2..43b0754d8 100644 --- a/src/com/esotericsoftware/kryo/serializers/RecordSerializer.java +++ b/src/com/esotericsoftware/kryo/serializers/RecordSerializer.java @@ -19,8 +19,7 @@ package com.esotericsoftware.kryo.serializers; -import static com.esotericsoftware.minlog.Log.TRACE; -import static com.esotericsoftware.minlog.Log.trace; +import static com.esotericsoftware.minlog.Log.*; import com.esotericsoftware.kryo.Kryo; import com.esotericsoftware.kryo.KryoException; @@ -69,29 +68,43 @@ public class RecordSerializer extends ImmutableSerializer { GET_TYPE = getType; } + private static final ClassValue> CONSTRUCTOR = new ClassValue>() { + protected Constructor computeValue(Class clazz) { + final RecordComponent[] components = recordComponents(clazz, Comparator.comparing(RecordComponent::index)); + return getCanonicalConstructor(clazz, components); + } + }; + private static final ClassValue RECORD_COMPONENTS = new ClassValue() { + protected RecordComponent[] computeValue(Class type) { + return recordComponents(type, Comparator.comparing(RecordComponent::name)); + } + }; + private boolean fixedFieldTypes = false; - public RecordSerializer () { + /** @deprecated use {@link #RecordSerializer(Class) instead} */ + @Deprecated(forRemoval = true) + public RecordSerializer() { + } + + public RecordSerializer (Class clazz) { + if (!isRecord(clazz)) throw new KryoException(clazz + " is not a record"); } @Override public void write (Kryo kryo, Output output, T object) { - final Class cls = object.getClass(); - if (!isRecord(cls)) { - throw new KryoException(object + " is not a record"); - } - for (RecordComponent rc : recordComponents(cls, Comparator.comparing(RecordComponent::name))) { + for (RecordComponent rc : RECORD_COMPONENTS.get(object.getClass())) { final Class type = rc.type(); final String name = rc.name(); try { if (TRACE) trace("kryo", "Write property: " + name + " (" + type.getName() + ")"); if (type.isPrimitive()) { - kryo.writeObject(output, componentValue(object, rc)); + kryo.writeObject(output, rc.getValue(object)); } else { if (fixedFieldTypes || kryo.isFinal(type)) { - kryo.writeObjectOrNull(output, componentValue(object, rc), type); + kryo.writeObjectOrNull(output, rc.getValue(object), type); } else { - kryo.writeClassAndObject(output, componentValue(object, rc)); + kryo.writeClassAndObject(output, rc.getValue(object)); } } } catch (KryoException ex) { @@ -107,13 +120,10 @@ public void write (Kryo kryo, Output output, T object) { @Override public T read (Kryo kryo, Input input, Class type) { - if (!isRecord(type)) { - throw new KryoException("Not a record (" + type + ")"); - } - final RecordComponent[] recordComponents = recordComponents(type, Comparator.comparing(RecordComponent::name)); - final Object[] values = new Object[recordComponents.length]; - for (int i = 0; i < recordComponents.length; i++) { - final RecordComponent rc = recordComponents[i]; + final RecordComponent[] components = RECORD_COMPONENTS.get(type); + final Object[] values = new Object[components.length]; + for (int i = 0; i < components.length; i++) { + final RecordComponent rc = components[i]; final String name = rc.name(); final Class rcType = rc.type(); try { @@ -137,8 +147,7 @@ public T read (Kryo kryo, Input input, Class type) { throw ex; } } - Arrays.sort(recordComponents, Comparator.comparing(RecordComponent::index)); - return invokeCanonicalConstructor(type, recordComponents, values); + return invokeCanonicalConstructor(type, values); } /** Returns true if, and only if, the given class is a record class. */ @@ -152,15 +161,29 @@ private boolean isRecord (Class type) { /** A record component, which has a name, a type and an index. The latter is the index of the record components in the class * file's record attribute, required to invoke the record's canonical constructor . */ - final static class RecordComponent { + static final class RecordComponent { + private final Class recordType; private final String name; private final Class type; private final int index; + private final Method getter; - RecordComponent (String name, Class type, int index) { + RecordComponent (Class recordType, String name, Class type, int index) { + this.recordType = recordType; this.name = name; this.type = type; this.index = index; + + try { + getter = recordType.getDeclaredMethod(name); + if (!getter.isAccessible()) { + getter.setAccessible(true); + } + } catch (Exception t) { + KryoException ex = new KryoException(t); + ex.addTrace("Could not retrieve record component getter (" + recordType.getName() + ")"); + throw ex; + } } String name () { @@ -174,6 +197,16 @@ Class type () { int index () { return index; } + + Object getValue (Object recordObject) { + try { + return getter.invoke(recordObject); + } catch (Exception t) { + KryoException ex = new KryoException(t); + ex.addTrace("Could not retrieve record component value (" + recordType.getName() + ")"); + throw ex; + } + } } /** Returns an ordered array of the record components for the given record class. The order is imposed by the given comparator. @@ -186,6 +219,7 @@ private static RecordComponent[] recordComponents (Class type, for (int i = 0; i < rawComponents.length; i++) { final Object comp = rawComponents[i]; recordComponents[i] = new RecordComponent( + type, (String)GET_NAME.invoke(comp), (Class)GET_TYPE.invoke(comp), i); } @@ -198,46 +232,42 @@ private static RecordComponent[] recordComponents (Class type, } } - /** Retrieves the value of the record component for the given record object. */ - private static Object componentValue (Object recordObject, - RecordComponent recordComponent) { + /** Invokes the canonical constructor of a record class with the given argument values. */ + private T invokeCanonicalConstructor (Class recordType, Object[] args) { try { - Method get = recordObject.getClass().getDeclaredMethod(recordComponent.name()); - if (!get.canAccess(recordObject)) { - get.setAccessible(true); - } - return get.invoke(recordObject); + return (T) CONSTRUCTOR.get(recordType).newInstance(args); } catch (Throwable t) { KryoException ex = new KryoException(t); - ex.addTrace("Could not retrieve record components (" - + recordObject.getClass().getName() + ")"); + ex.addTrace("Could not construct type (" + recordType.getName() + ")"); throw ex; } } - /** Invokes the canonical constructor of a record class with the given argument values. */ - private static T invokeCanonicalConstructor (Class recordType, - RecordComponent[] recordComponents, - Object[] args) { + private static Constructor getCanonicalConstructor (Class recordType, RecordComponent[] recordComponents) { try { Class[] paramTypes = Arrays.stream(recordComponents) .map(RecordComponent::type) .toArray(Class[]::new); - Constructor canonicalConstructor; - try { - canonicalConstructor = recordType.getConstructor(paramTypes); - } catch (NoSuchMethodException e) { - canonicalConstructor = recordType.getDeclaredConstructor(paramTypes); - canonicalConstructor.setAccessible(true); - } - return canonicalConstructor.newInstance(args); + return getCanonicalConstructor(recordType, paramTypes); } catch (Throwable t) { KryoException ex = new KryoException(t); - ex.addTrace("Could not construct type (" + recordType.getName() + ")"); + ex.addTrace("Could not retrieve record canonical constructor (" + recordType.getName() + ")"); throw ex; } } + private static Constructor getCanonicalConstructor (Class recordType, Class[] paramTypes) + throws NoSuchMethodException { + Constructor canonicalConstructor; + try { + canonicalConstructor = recordType.getConstructor(paramTypes); + } catch (NoSuchMethodException e) { + canonicalConstructor = recordType.getDeclaredConstructor(paramTypes); + canonicalConstructor.setAccessible(true); + } + return canonicalConstructor; + } + /** Tells the RecordSerializer that all field types are effectively final. This allows the serializer to be more efficient, * since it knows field values will not be a subclass of their declared type. Default is false. */ public void setFixedFieldTypes (boolean fixedFieldTypes) { diff --git a/test-jdk14/com/esotericsoftware/kryo/serializers/RecordSerializerTest.java b/test-jdk14/com/esotericsoftware/kryo/serializers/RecordSerializerTest.java index 24a3fd74c..966a6b913 100644 --- a/test-jdk14/com/esotericsoftware/kryo/serializers/RecordSerializerTest.java +++ b/test-jdk14/com/esotericsoftware/kryo/serializers/RecordSerializerTest.java @@ -306,7 +306,7 @@ public static record RecordWithSuperType(Number n) {} @Test void testRecordWithSuperType() { - var rc = new RecordSerializer(); + var rc = new RecordSerializer<>(RecordWithSuperType.class); kryo.register(RecordWithSuperType.class, rc); final var r = new RecordWithSuperType(1L);