From ae34697c8ddf019aa9d0a5f8908d4d8e1f7007d4 Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Mon, 24 Nov 2008 22:05:04 +0000 Subject: [PATCH] KeySet is going to get stuffed into the cache, so we're making it more memory efficient. Also switched from Collection to Iterable in some internal spots where we don't need anything other than an iterator. I wanted to push this all the way out to DepotRepository but ran into some snags. I'm leaving in this change in case I feel like desnaggling the external bits later. --- .../com/samskivert/depot/BuildVisitor.java | 4 +- .../com/samskivert/depot/DepotRepository.java | 2 +- .../com/samskivert/depot/FindAllQuery.java | 12 +- src/java/com/samskivert/depot/Key.java | 41 ++- src/java/com/samskivert/depot/KeySet.java | 295 ++++++++++++++---- .../depot/tests/TestRepository.java | 29 +- 6 files changed, 286 insertions(+), 97 deletions(-) diff --git a/src/java/com/samskivert/depot/BuildVisitor.java b/src/java/com/samskivert/depot/BuildVisitor.java index 3576297..ad10f5a 100644 --- a/src/java/com/samskivert/depot/BuildVisitor.java +++ b/src/java/com/samskivert/depot/BuildVisitor.java @@ -100,7 +100,7 @@ public abstract class BuildVisitor implements ExpressionVisitor { Class pClass = key.getPersistentClass(); String[] keyFields = DepotUtil.getKeyFields(pClass); - List> values = key.getValues(); + Comparable[] values = key.getValues(); for (int ii = 0; ii < keyFields.length; ii ++) { if (ii > 0) { _builder.append(" and "); @@ -111,7 +111,7 @@ public abstract class BuildVisitor implements ExpressionVisitor _enableOverrides = true; appendRhsColumn(pClass, keyFields[ii]); _enableOverrides = saved; - _builder.append(values.get(ii) == null ? " is null " : " = ? "); + _builder.append(values[ii] == null ? " is null " : " = ? "); } } diff --git a/src/java/com/samskivert/depot/DepotRepository.java b/src/java/com/samskivert/depot/DepotRepository.java index 484e7ea..d73e3a4 100644 --- a/src/java/com/samskivert/depot/DepotRepository.java +++ b/src/java/com/samskivert/depot/DepotRepository.java @@ -828,7 +828,7 @@ public abstract class DepotRepository { if (_ctx.getMarshaller(type).hasPrimaryKey()) { // look up the primary keys for all rows matching our where clause and delete using those - KeySet pwhere = new KeySet(type, findAllKeys(type, true, where)); + KeySet pwhere = KeySet.newKeySet(type, findAllKeys(type, true, where)); return deleteAll(type, pwhere, pwhere); } else { // otherwise just do the delete directly as we can't have cached a record that has no diff --git a/src/java/com/samskivert/depot/FindAllQuery.java b/src/java/com/samskivert/depot/FindAllQuery.java index 67d5e3a..5e60761 100644 --- a/src/java/com/samskivert/depot/FindAllQuery.java +++ b/src/java/com/samskivert/depot/FindAllQuery.java @@ -133,7 +133,7 @@ public abstract class FindAllQuery extends Query extends FindAllQuery { - public WithKeys (PersistenceContext ctx, Collection> keys) + public WithKeys (PersistenceContext ctx, Iterable> keys) throws DatabaseException { super(ctx, keys.iterator().next().getPersistentClass()); @@ -160,7 +160,7 @@ public abstract class FindAllQuery extends Query> _keys; + protected Iterable> _keys; protected Map, T> _entities = Maps.newHashMap(); protected Set> _fetchKeys = Sets.newHashSet(); } @@ -226,7 +226,7 @@ public abstract class FindAllQuery extends Query> allKeys, + protected void loadFromCache (PersistenceContext ctx, Iterable> allKeys, Map, T> entities, Set> fetchKeys) { for (Key key : allKeys) { @@ -240,7 +240,7 @@ public abstract class FindAllQuery extends Query resolve (Collection> allKeys, Map, T> entities) + protected List resolve (Iterable> allKeys, Map, T> entities) { List result = Lists.newArrayList(); for (Key key : allKeys) { @@ -253,7 +253,7 @@ public abstract class FindAllQuery extends Query loadAndResolve (PersistenceContext ctx, Connection conn, - Collection> allKeys, Set> fetchKeys, + Iterable> allKeys, Set> fetchKeys, Map, T> entities, String origStmt) throws SQLException { @@ -288,7 +288,7 @@ public abstract class FindAllQuery extends Query(_type, _marsh.getFieldNames(), - new KeySet(_type, keys))); + KeySet.newKeySet(_type, keys))); PreparedStatement stmt = _builder.prepare(conn); try { Set> got = Sets.newHashSet(); diff --git a/src/java/com/samskivert/depot/Key.java b/src/java/com/samskivert/depot/Key.java index 1c788de..10f55ea 100644 --- a/src/java/com/samskivert/depot/Key.java +++ b/src/java/com/samskivert/depot/Key.java @@ -22,6 +22,7 @@ package com.samskivert.depot; import java.io.Serializable; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.Map; @@ -47,14 +48,14 @@ public class Key extends WhereClause * combine a buncy of keys into a {@link KeySet}. */ public static class Expression implements SQLExpression { - public Expression (Class pClass, List> values) { + public Expression (Class pClass, Comparable[] values) { _pClass = pClass; _values = values; } public Class getPersistentClass () { return _pClass; } - public List> getValues () { + public Comparable[] getValues () { return _values; } public void accept (ExpressionVisitor builder) { @@ -64,7 +65,7 @@ public class Key extends WhereClause classSet.add(getPersistentClass()); } protected Class _pClass; - protected List> _values; + protected Comparable[] _values; } /** @@ -115,19 +116,18 @@ public class Key extends WhereClause String[] keyFields = DepotUtil.getKeyFields(pClass); // now extract the values in field order and ensure none are extra or missing - _values = Lists.newArrayList(); + _values = new Comparable[values.length]; for (int ii = 0; ii < keyFields.length; ii++) { - Comparable nugget = map.remove(keyFields[ii]); - if (nugget == null) { + Comparable value = map.remove(keyFields[ii]); + if (value == null) { // make sure we were provided with a value for this primary key field throw new IllegalArgumentException("Missing value for key field: " + keyFields[ii]); } - if (nugget instanceof Serializable) { - _values.add(nugget); - continue; + if (!(value instanceof Serializable)) { + throw new IllegalArgumentException( + "Non-serializable argument [key=" + keyFields[ii] + ", value=" + value + "]"); } - throw new IllegalArgumentException( - "Non-serializable argument [key=" + keyFields[ii] + ", arg=" + nugget + "]"); + _values[ii] = value; } // finally make sure we were not given any fields that are not in fact primary key fields @@ -137,6 +137,15 @@ public class Key extends WhereClause } } + /** + * Used to create a key when you know you have the canonical values array. + */ + protected Key (Class pClass, Comparable[] values) + { + _pClass = pClass; + _values = values; + } + /** * Returns the persistent class for which we represent a key. */ @@ -148,7 +157,7 @@ public class Key extends WhereClause /** * Returns the values bound to this key. */ - public List> getValues () + public Comparable[] getValues () { return _values; } @@ -209,7 +218,7 @@ public class Key extends WhereClause if (ii > 0) { builder.append(":"); } - builder.append(keyFields[ii]).append("=").append(_values.get(ii)); + builder.append(keyFields[ii]).append("=").append(_values[ii]); } } @@ -229,13 +238,13 @@ public class Key extends WhereClause if (obj == null || getClass() != obj.getClass()) { return false; } - return _values.equals(((Key) obj)._values); + return Arrays.equals(_values, ((Key) obj)._values); } @Override public int hashCode () { - return _values.hashCode(); + return Arrays.hashCode(_values); } @Override @@ -252,5 +261,5 @@ public class Key extends WhereClause protected final Class _pClass; /** The expression that identifies our row. */ - protected final ArrayList> _values; // must declare as ArrayList for Serializable + protected final Comparable[] _values; } diff --git a/src/java/com/samskivert/depot/KeySet.java b/src/java/com/samskivert/depot/KeySet.java index a0bcaf7..bde2d32 100644 --- a/src/java/com/samskivert/depot/KeySet.java +++ b/src/java/com/samskivert/depot/KeySet.java @@ -20,7 +20,16 @@ package com.samskivert.depot; +import java.util.AbstractCollection; +import java.util.Arrays; import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; + +import com.google.common.collect.Iterators; +import com.google.common.base.Function; + +import com.samskivert.util.StringUtil; import com.samskivert.depot.expression.ExpressionVisitor; import com.samskivert.depot.expression.LiteralExp; @@ -28,6 +37,8 @@ import com.samskivert.depot.expression.SQLExpression; import com.samskivert.depot.operator.Conditionals; import com.samskivert.depot.operator.Logic; +import static com.samskivert.depot.Log.log; + /** * Contains a set of primary keys that match a set of persistent records. This is used internally * in Depot when decomposing queries into two parts: first a query for the primary keys that @@ -36,63 +47,243 @@ import com.samskivert.depot.operator.Logic; * modifications and deletons using this set will automatically flush the appropriate records from * the cache. */ -public class KeySet extends WhereClause - implements SQLExpression, ValidatingCacheInvalidator +public abstract class KeySet extends WhereClause + implements SQLExpression, ValidatingCacheInvalidator, Iterable> { /** - * Creates a set from the supplied primary keys. + * Creates a key set for the supplied persistent record and keys. */ - public KeySet (Class pClass, Collection> keys) + public static KeySet newKeySet ( + Class pClass, Collection> keys) { - _pClass = pClass; - _keys = keys; + if (keys.size() == 0) { + return new EmptyKeySet(pClass); + } String[] keyFields = DepotUtil.getKeyFields(pClass); - if (keys.size() == 0) { - _condition = new LiteralExp("false"); - - } else if (keyFields.length == 1) { - // TODO: remove when we update to 1.6 and change Postgres In handling - if (keys.size() > Conditionals.In.MAX_KEYS) { - throw new IllegalArgumentException("Cannot create where clause for more than " + - Conditionals.In.MAX_KEYS + " at a time."); - } - - // Single-column keys result in the compact IN(keyVal1, keyVal2, ...) - Comparable first = keys.iterator().next().getValues().get(0); - Comparable[] keyFieldValues; + if (keyFields.length == 1) { + Comparable first = keys.iterator().next().getValues()[0]; + Comparable[] keyArray; if (first instanceof Integer) { - keyFieldValues = new Integer[keys.size()]; + keyArray = new Integer[keys.size()]; } else if (first instanceof Integer) { - keyFieldValues = new String[keys.size()]; + keyArray = new String[keys.size()]; } else { - keyFieldValues = new Comparable[keys.size()]; + keyArray = new Comparable[keys.size()]; } int ii = 0; for (Key key : keys) { - keyFieldValues[ii++] = key.getValues().get(0); + keyArray[ii++] = key.getValues()[0]; } - _condition = new Conditionals.In(pClass, keyFields[0], keyFieldValues); + return new SingleKeySet(pClass, keyArray); } else { // TODO: is there a maximum size of an or query? 32768? - - // Multi-column keys result in OR'd AND's, of unknown efficiency (TODO check). - SQLExpression[] keyArray = new SQLExpression[keys.size()]; + Comparable[][] keysValues = new Comparable[keys.size()][]; int ii = 0; for (Key key : keys) { - keyArray[ii++] = key.getWhereExpression(); + keysValues[ii++] = key.getValues(); } - _condition = new Logic.Or(keyArray); + return new MultiKeySet(pClass, keysValues); } } - // from WhereClause - public SQLExpression getWhereExpression () + /** + * Creates a key set for the supplied persistent record and collection of simple keys. + * + * @exception IllegalArgumentException thrown if the supplied record does not use a simple + * (single-column) primay key. + */ + public static KeySet newSimpleKeySet ( + Class pClass, Collection> keys) { - return _condition; + String[] keyFields = DepotUtil.getKeyFields(pClass); + if (keyFields.length != 1) { + throw new IllegalArgumentException( + "Cannot create KeySet using simple keys for record with non-simple primary key " + + "[record=" + pClass + "]"); + } + if (keys.size() == 0) { + return new EmptyKeySet(pClass); + } else { + Comparable first = keys.iterator().next(); + Comparable[] keyArray; + if (first instanceof Integer) { + keyArray = new Integer[keys.size()]; + } else if (first instanceof Integer) { + keyArray = new String[keys.size()]; + } else { + keyArray = new Comparable[keys.size()]; + } + return new SingleKeySet(pClass, keys.toArray(keyArray)); + } } + protected static class EmptyKeySet extends KeySet + { + public EmptyKeySet (Class pClass) { + super(pClass); + } + + // from WhereClause + public SQLExpression getWhereExpression () { + return new LiteralExp("false"); + } + + // from Iterable> + public Iterator> iterator () { + return Collections.>emptyList().iterator(); + } + + @Override public int size () { + return 0; + } + + @Override public boolean equals (Object obj) { + if (this == obj) { + return true; + } + return (obj instanceof EmptyKeySet) && _pClass.equals(((EmptyKeySet)obj)._pClass); + } + + @Override public int hashCode () { + return _pClass.hashCode(); + } + + @Override public String toString () { + return DepotUtil.justClassName(_pClass) + "(empty)"; + } + } + + protected static class SingleKeySet extends KeySet + { + public SingleKeySet (Class pClass, Comparable[] keys) { + super(pClass); + // TODO: remove when we update to 1.6 and change Postgres In handling + if (keys.length > Conditionals.In.MAX_KEYS) { + throw new IllegalArgumentException("Cannot create where clause for more than " + + Conditionals.In.MAX_KEYS + " at a time."); + } + _keys = keys; + } + + // from WhereClause + public SQLExpression getWhereExpression () { + // Single-column keys result in the compact IN(keyVal1, keyVal2, ...) + return new Conditionals.In(_pClass, DepotUtil.getKeyFields(_pClass)[0], _keys); + } + + // from Iterable> + public Iterator> iterator () { + return Iterators.transform( + Iterators.forArray(_keys, 0, _keys.length), new Function, Key>() { + public Key apply (Comparable key) { + return new Key(_pClass, new Comparable[] { key }); + } + }); + } + + @Override public int size () { + return _keys.length; + } + + @Override public boolean equals (Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof SingleKeySet)) { + return false; + } + SingleKeySet oset = (SingleKeySet)obj; + return _pClass.equals(oset._pClass) && Arrays.equals(_keys, oset._keys); + } + + @Override public int hashCode () { + return 31 * _pClass.hashCode() + Arrays.hashCode(_keys); + } + + @Override public String toString () { + return DepotUtil.justClassName(_pClass) + StringUtil.toString(_keys); + } + + protected Comparable[] _keys; + } + + protected static class MultiKeySet extends KeySet + { + public MultiKeySet (Class pClass, Comparable[][] keys) + { + super(pClass); + _keys = keys; + } + + // from WhereClause + public SQLExpression getWhereExpression () { + // Multi-column keys result in OR'd AND's, of unknown efficiency (TODO check). + SQLExpression[] keyexps = new SQLExpression[_keys.length]; + int ii = 0; + for (Comparable[] kvals : _keys) { + keyexps[ii++] = new Key.Expression(_pClass, kvals); + } + return new Logic.Or(keyexps); + } + + // from Iterable> + public Iterator> iterator () { + return Iterators.transform( + Iterators.forArray(_keys, 0, _keys.length), new Function[], Key>() { + public Key apply (Comparable[] key) { + return new Key(_pClass, key); + } + }); + } + + @Override public int size () { + return _keys.length; + } + + @Override public boolean equals (Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof MultiKeySet)) { + return false; + } + MultiKeySet oset = (MultiKeySet)obj; + return _pClass.equals(oset._pClass) && Arrays.equals(_keys, oset._keys); + } + + @Override public int hashCode () { + return 31 * _pClass.hashCode() + Arrays.hashCode(_keys); + } + + @Override public String toString () { + return DepotUtil.justClassName(_pClass) + StringUtil.toString(_keys); + } + + protected Comparable[][] _keys; + } + + /** + * Returns an unmodifiable {@link Collection} view on this KeySet. + */ + public Collection> toCollection () + { + return new AbstractCollection>() { + public Iterator> iterator () { + return KeySet.this.iterator(); + } + public int size () { + return KeySet.this.size(); + } + }; + } + + /** + * Returns the number of keys in this set. + */ + public abstract int size (); + // from SQLExpression public void addClasses (Collection> classSet) { @@ -105,6 +296,13 @@ public class KeySet extends WhereClause builder.visit(this); } + // from ValidatingCacheInvalidator + public void invalidate (PersistenceContext ctx) { + for (Key key : this) { + ctx.cacheInvalidate(key); + } + } + // from ValidatingCacheInvalidator public void validateFlushType (Class pClass) { @@ -115,14 +313,6 @@ public class KeySet extends WhereClause } } - // from CacheInvalidator - public void invalidate (PersistenceContext ctx) - { - for (Key key : _keys) { - ctx.cacheInvalidate(key); - } - } - @Override // from WhereClause public void validateQueryType (Class pClass) { @@ -130,31 +320,10 @@ public class KeySet extends WhereClause validateTypesMatch(pClass, _pClass); } - @Override - public boolean equals (Object obj) + protected KeySet (Class pClass) { - if (this == obj) { - return true; - } - if (obj == null || getClass() != obj.getClass()) { - return false; - } - return _condition.equals(((KeySet) obj)._condition); - } - - @Override - public int hashCode () - { - return _condition.hashCode(); - } - - @Override - public String toString () - { - return _keys.toString(); + _pClass = pClass; } protected Class _pClass; - protected Collection> _keys; - protected SQLExpression _condition; } diff --git a/src/java/com/samskivert/depot/tests/TestRepository.java b/src/java/com/samskivert/depot/tests/TestRepository.java index 01846df..5e047da 100644 --- a/src/java/com/samskivert/depot/tests/TestRepository.java +++ b/src/java/com/samskivert/depot/tests/TestRepository.java @@ -22,17 +22,17 @@ package com.samskivert.depot.tests; import java.sql.Date; import java.sql.Timestamp; -// import java.util.HashSet; +import java.util.Collections; import java.util.Set; -// import com.google.common.collect.Sets; +import com.google.common.collect.Sets; import com.samskivert.util.RandomUtil; import com.samskivert.jdbc.StaticConnectionProvider; import com.samskivert.depot.DepotRepository; -// import com.samskivert.depot.Key; -// import com.samskivert.depot.KeySet; +import com.samskivert.depot.Key; +import com.samskivert.depot.KeySet; import com.samskivert.depot.PersistenceContext; import com.samskivert.depot.PersistentRecord; import com.samskivert.depot.SchemaMigration; @@ -55,7 +55,7 @@ public class TestRepository extends DepotRepository return recordId + ":" + name; } } - + public static void main (String[] args) throws Exception { @@ -109,7 +109,18 @@ public class TestRepository extends DepotRepository record.lastModified = tnow; repo.insert(record); } - + + // test the empty ky set + KeySet none = KeySet.newKeySet( + TestRecord.class, Collections.>emptySet()); + System.out.println("Load none " + repo.loadAll(none.toCollection()) + "."); + System.out.println("Delete none " + repo.deleteAll(TestRecord.class, none) + "."); + + // test a partial key set + KeySet some = KeySet.newSimpleKeySet( + TestRecord.class, Sets.newHashSet(1, 3, 5, 7, 9)); + System.out.println("Load some " + repo.loadAll(some.toCollection()) + "."); + System.out.println("Names " + repo.findAll(TestNameRecord.class) + "."); System.out.println("Have " + repo.findAll(TestRecord.class).size() + " records."); repo.deleteAll(TestRecord.class, new Where(new Conditionals.LessThan( @@ -117,11 +128,11 @@ public class TestRepository extends DepotRepository System.out.println("Now have " + repo.findAll(TestRecord.class).size() + " records."); repo.deleteAll(TestRecord.class, new Where(new LiteralExp("true"))); // // TODO: try to break our In() clause -// Set> ids = Sets.newHashSet(); +// Set ids = Sets.newHashSet(); // for (int ii = 1; ii <= Conditionals.In.MAX_KEYS*2+3; ii++) { -// ids.add(TestRecord.getKey(ii)); +// ids.add(ii); // } -// repo.deleteAll(TestRecord.class, new KeySet(TestRecord.class, ids)); +// repo.deleteAll(TestRecord.class, KeySet.newSimpleKeySet(TestRecord.class, ids)); System.out.println("Now have " + repo.findAll(TestRecord.class).size() + " records."); }