From 704476323e9fbc0bba4766e4a5b21dda0bcace04 Mon Sep 17 00:00:00 2001 From: Ray Greenwell Date: Wed, 24 Mar 2010 19:51:56 +0000 Subject: [PATCH] Faster Key construction via the generated getKey() factory. A little bit back Mike added a fast-path for Key construction for keys with one Comparable. Keys with multiple values would still validate the order every time. This was necessary even when using the generated factory because Class' getFields() does not guarantee any order. In practice, the Sun JVM seems to return them in declared order, but apparently other JVMs may not. The arguments to getKey() are determined when 'genrecord' is run, but the field order used during runtime is computed at runtime. This change generates code that stashes the key fields in an order deteremined at the time 'genrecord' is run and that is guaranteed to match the argument order to each PersistentRecord subclass' getKey(). Thus, getKey() can now directly construct a key with minimal overhead. The field order is still computed at runtime as a fallback for PersistentRecord classes that haven't yet been updated. The ordering of getFields() is still being relied upon at 'genrecord' time, but Mike may introduce a patch that will guarantee that ordering matches declaration order. In the meantime, it's no worse than before and if somehow someone does run 'genrecord' with an alternate JVM they should immediately notice the discrepency in their diffs. The Key constructor utilized is still public because it is used by DepotUtil. A nice change would be to make it package-private. --- src/java/com/samskivert/depot/Key.java | 1 + .../samskivert/depot/PersistentRecord.java | 21 ++++++++++++++++++- .../impl/DepotMigrationHistoryRecord.java | 8 +++---- .../com/samskivert/depot/impl/DepotUtil.java | 18 +++++++++++++++- .../samskivert/depot/tests/MonkeyRecord.java | 8 +++---- .../samskivert/depot/tests/TestRecord.java | 8 +++---- .../samskivert/depot/tools/record_key.tmpl | 8 +++---- 7 files changed, 54 insertions(+), 18 deletions(-) diff --git a/src/java/com/samskivert/depot/Key.java b/src/java/com/samskivert/depot/Key.java index 96b7f83..6ec3a78 100644 --- a/src/java/com/samskivert/depot/Key.java +++ b/src/java/com/samskivert/depot/Key.java @@ -135,6 +135,7 @@ public class Key extends WhereClause * Used to create a key when you know you have the canonical values array. Don't call this * unless you know what you're doing! */ + // TODO: This should perhaps be made package-private, but DepotMarshaller uses it public Key (Class pClass, Comparable[] values) { _pClass = pClass; diff --git a/src/java/com/samskivert/depot/PersistentRecord.java b/src/java/com/samskivert/depot/PersistentRecord.java index e9741b5..98ca713 100644 --- a/src/java/com/samskivert/depot/PersistentRecord.java +++ b/src/java/com/samskivert/depot/PersistentRecord.java @@ -23,6 +23,7 @@ package com.samskivert.depot; import java.io.Serializable; import com.samskivert.depot.expression.ColumnExp; +import com.samskivert.depot.impl.DepotUtil; /** * The base class for all persistent records used in Depot. Persistent records must be cloneable @@ -41,7 +42,7 @@ public class PersistentRecord try { return (PersistentRecord) super.clone(); } catch (CloneNotSupportedException cnse) { - throw new RuntimeException(cnse); // this should never happen + throw new AssertionError(cnse); // this should never happen since we are Cloneable } } @@ -53,4 +54,22 @@ public class PersistentRecord { return new ColumnExp(clazz, fieldName); } + + /** + * Creates a new Key for this class. Called by autogenerated code, and placed here because + * we hope to make this form of the Key constructor protected. + */ + protected static Key newKey ( + Class pClass, Comparable... values) + { + return new Key(pClass, values); + } + + /** + * Register the key fields for a subclass. Called by autogenerated code. + */ + protected static void registerKeyFields (ColumnExp... fields) + { + DepotUtil.registerKeyFields(fields); + } } diff --git a/src/java/com/samskivert/depot/impl/DepotMigrationHistoryRecord.java b/src/java/com/samskivert/depot/impl/DepotMigrationHistoryRecord.java index 262adc4..6307f4f 100644 --- a/src/java/com/samskivert/depot/impl/DepotMigrationHistoryRecord.java +++ b/src/java/com/samskivert/depot/impl/DepotMigrationHistoryRecord.java @@ -56,10 +56,10 @@ public class DepotMigrationHistoryRecord extends PersistentRecord */ public static Key getKey (String ident) { - return new Key( - DepotMigrationHistoryRecord.class, - new ColumnExp[] { IDENT }, - new Comparable[] { ident }); + return newKey(_R, ident); } + + /** Register the key fields in an order matching the getKey() factory. */ + static { registerKeyFields(IDENT); } // AUTO-GENERATED: METHODS END } diff --git a/src/java/com/samskivert/depot/impl/DepotUtil.java b/src/java/com/samskivert/depot/impl/DepotUtil.java index 66a87ef..b8b7b4c 100644 --- a/src/java/com/samskivert/depot/impl/DepotUtil.java +++ b/src/java/com/samskivert/depot/impl/DepotUtil.java @@ -46,18 +46,34 @@ public class DepotUtil return _keyFields.get(pClass); } + /** + * Register the key fields to their PersistentRecord class. It should never be necessary + * to do this manually, as it is done via a static initializer in generated PersistentRecord + * subclasses. Calling this method after the key fields have already been registered will + * have no effect. + */ + public static void registerKeyFields (ColumnExp... fields) + { + // TODO: Checks? For example: Validate all exps from same class? + // Make a defensive copy of the array? Hide this method from public consumption? + _keyFields.putIfAbsent(fields[0].getPersistentClass(), fields); + } + /** * Returns the name of the supplied class minus its package. */ public static String justClassName (Class clazz) { - return clazz.getName().substring(clazz.getName().lastIndexOf(".")+1); + return clazz.getName().substring(clazz.getName().lastIndexOf(".") + 1); } /** A (never expiring) cache of primary key field names for all persistent classes (of which * there are merely dozens, so we don't need to worry about expiring). */ protected static ConcurrentMap, ColumnExp[]> _keyFields = new MapMaker() + // newly generated PersistentRecord classes will register their key fields via + // registerKeyFields, which will return an ordering determined at genrecord time. + // We fall back to computing the fields at runtime for older PersistentRecord classes. .makeComputingMap(new Function, ColumnExp[]>() { public ColumnExp[] apply (Class pClass) { List kflist = Lists.newArrayList(); diff --git a/src/java/com/samskivert/depot/tests/MonkeyRecord.java b/src/java/com/samskivert/depot/tests/MonkeyRecord.java index 3c4e9fe..ed3673d 100644 --- a/src/java/com/samskivert/depot/tests/MonkeyRecord.java +++ b/src/java/com/samskivert/depot/tests/MonkeyRecord.java @@ -52,10 +52,10 @@ public class MonkeyRecord extends PersistentRecord */ public static Key getKey (int species, int monkeyId) { - return new Key( - MonkeyRecord.class, - new ColumnExp[] { SPECIES, MONKEY_ID }, - new Comparable[] { species, monkeyId }); + return newKey(_R, species, monkeyId); } + + /** Register the key fields in an order matching the getKey() factory. */ + static { registerKeyFields(SPECIES, MONKEY_ID); } // AUTO-GENERATED: METHODS END } diff --git a/src/java/com/samskivert/depot/tests/TestRecord.java b/src/java/com/samskivert/depot/tests/TestRecord.java index bf8374b..efd9d25 100644 --- a/src/java/com/samskivert/depot/tests/TestRecord.java +++ b/src/java/com/samskivert/depot/tests/TestRecord.java @@ -80,10 +80,10 @@ public class TestRecord extends PersistentRecord */ public static Key getKey (int recordId) { - return new Key( - TestRecord.class, - new ColumnExp[] { RECORD_ID }, - new Comparable[] { recordId }); + return newKey(_R, recordId); } + + /** Register the key fields in an order matching the getKey() factory. */ + static { registerKeyFields(RECORD_ID); } // AUTO-GENERATED: METHODS END } diff --git a/src/java/com/samskivert/depot/tools/record_key.tmpl b/src/java/com/samskivert/depot/tools/record_key.tmpl index 84a604a..fbaffb8 100644 --- a/src/java/com/samskivert/depot/tools/record_key.tmpl +++ b/src/java/com/samskivert/depot/tools/record_key.tmpl @@ -4,8 +4,8 @@ */ public static Key<@record@> getKey (@argList@) { - return new Key<@record@>( - @record@.class, - new ColumnExp[] { @fieldNameList@ }, - new Comparable[] { @argNameList@ }); + return newKey(_R, @argNameList@); } + + /** Register the key fields in an order matching the getKey() factory. */ + static { registerKeyFields(@fieldNameList@); }