From 0b77c89fa7ce69251f9146a948016af080635bcd Mon Sep 17 00:00:00 2001 From: Ray Greenwell Date: Thu, 9 Dec 2010 02:39:01 +0000 Subject: [PATCH] Improvements to Collection streaming support. Any Collection can now be streamed. You can send Collections.emptyMap() back to a client, for example. (It will unstream as a HashMap on the other side.) This actually streams everything to the exact same bytes as before. Using specific classes is discouraged: ArrayList, StreamableArrayList... If a Streamer is created for a class that has a field of that type, a warning will be generated, urging you to change the field definition to the interface. Also: that setup is now deferred until needed, because in clyde/projectx there are a number of Streamable classes that have fields of specific classes, like ArrayIntSet for example, but don't use standard streaming to transmit them. In that case it's totally cool for them to have a more specific type. Since that setup is potentially deferred, one change may be observed for a class that uses custom readObject() and writeObject() methods but still calls defaultReadObject(). The field marshallers will now be configured a little later in the game than before. But, we're actually saving a bit of memory by not setting up those marshallers for streamable classes that never use them. Woo. Please let me know if this causes any space shuttle explosions or spikes the punch at your quinceanera. git-svn-id: svn+ssh://src.earth.threerings.net/narya/trunk@6348 542714f4-19e9-0310-aa3c-eee0fc999fb1 --- .../com/threerings/io/BasicStreamers.java | 3 - .../com/threerings/io/FieldMarshaller.java | 19 +++- .../com/threerings/io/ObjectInputStream.java | 10 +- .../com/threerings/io/ObjectOutputStream.java | 8 ++ src/main/java/com/threerings/io/Streamer.java | 106 ++++++++++++++---- .../presents/dobj/CompoundEvent.java | 8 +- 6 files changed, 114 insertions(+), 40 deletions(-) diff --git a/src/main/java/com/threerings/io/BasicStreamers.java b/src/main/java/com/threerings/io/BasicStreamers.java index b8ed48824..e9e02dd63 100644 --- a/src/main/java/com/threerings/io/BasicStreamers.java +++ b/src/main/java/com/threerings/io/BasicStreamers.java @@ -67,12 +67,9 @@ public class BasicStreamers .put(double[].class, new DoubleArrayStreamer()) .put(Object[].class, new ObjectArrayStreamer()) .put(List.class, ListStreamer.INSTANCE) - .put(ArrayList.class, ListStreamer.INSTANCE) .put(Collection.class, ListStreamer.INSTANCE) .put(Set.class, SetStreamer.INSTANCE) - .put(HashSet.class, SetStreamer.INSTANCE) .put(Map.class, MapStreamer.INSTANCE) - .put(HashMap.class, MapStreamer.INSTANCE) .build(); /** Streams {@link Boolean} instances. */ diff --git a/src/main/java/com/threerings/io/FieldMarshaller.java b/src/main/java/com/threerings/io/FieldMarshaller.java index 360cc7ab8..c71cf1ab7 100644 --- a/src/main/java/com/threerings/io/FieldMarshaller.java +++ b/src/main/java/com/threerings/io/FieldMarshaller.java @@ -26,7 +26,6 @@ import java.lang.reflect.Method; import java.lang.reflect.ReflectPermission; import java.util.Date; -import java.util.HashMap; import java.util.Map; import com.google.common.collect.Maps; @@ -117,10 +116,20 @@ public abstract class FieldMarshaller // if we have an exact match, use that FieldMarshaller fm = _marshallers.get(ftype); + if (fm == null) { + Class collClass = Streamer.getCollectionClass(ftype); + if (collClass != null && !collClass.equals(ftype)) { + log.warning("Specific field types are discouraged for Collections and Maps.", + "class", field.getDeclaringClass(), "field", field.getName(), + "type", ftype, "shouldBe", collClass); + fm = _marshallers.get(collClass); + } - // otherwise if the class is a pure interface or streamable, use the streamable marshaller - if (fm == null && (ftype.isInterface() || Streamer.isStreamable(ftype))) { - fm = _marshallers.get(Streamable.class); + // otherwise if the class is a pure interface or streamable, + // use the streamable marshaller + if (fm == null && (ftype.isInterface() || Streamer.isStreamable(ftype))) { + fm = _marshallers.get(Streamable.class); + } } return fm; @@ -395,7 +404,7 @@ public abstract class FieldMarshaller } /** Contains a mapping from field type to field marshaller instance for that type. */ - protected static HashMap, FieldMarshaller> _marshallers; + protected static Map, FieldMarshaller> _marshallers; /** The field marshaller for pooled strings. */ protected static FieldMarshaller _internMarshaller; diff --git a/src/main/java/com/threerings/io/ObjectInputStream.java b/src/main/java/com/threerings/io/ObjectInputStream.java index 3692df7c1..661ee81a7 100644 --- a/src/main/java/com/threerings/io/ObjectInputStream.java +++ b/src/main/java/com/threerings/io/ObjectInputStream.java @@ -21,8 +21,8 @@ package com.threerings.io; -import java.util.ArrayList; -import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.io.ByteArrayInputStream; import java.io.DataInputStream; @@ -337,10 +337,10 @@ public class ObjectInputStream extends DataInputStream /** Used to map classes to numeric codes and the {@link Streamer} instance used to write * them. */ - protected ArrayList _classmap; + protected List _classmap; /** Maps numeric codes to pooled strings. */ - protected ArrayList _internmap; + protected List _internmap; /** The object currently being read from the stream. */ protected Object _current; @@ -352,7 +352,7 @@ public class ObjectInputStream extends DataInputStream protected ClassLoader _loader; /** An optional set of class name translations to use when unserializing objects. */ - protected HashMap _translations; + protected Map _translations; /** Used to activate verbose debug logging. */ protected static final boolean STREAM_DEBUG = false; diff --git a/src/main/java/com/threerings/io/ObjectOutputStream.java b/src/main/java/com/threerings/io/ObjectOutputStream.java index 5b32642c3..c81c02d1a 100644 --- a/src/main/java/com/threerings/io/ObjectOutputStream.java +++ b/src/main/java/com/threerings/io/ObjectOutputStream.java @@ -174,6 +174,14 @@ public class ObjectOutputStream extends DataOutputStream // create a class mapping for this class if we've not got one if (cmap == null) { + // see if we just want to use an existing class mapping + Class collClass = Streamer.getCollectionClass(sclass); + if (collClass != null && !collClass.equals(sclass)) { + cmap = writeClassMapping(collClass); + _classmap.put(sclass, cmap); + return cmap; + } + // create a streamer instance and assign a code to this class Streamer streamer = Streamer.getStreamer(sclass); // we specifically do not inline the getStreamer() call into the ClassMapping diff --git a/src/main/java/com/threerings/io/Streamer.java b/src/main/java/com/threerings/io/Streamer.java index f6ef1706e..9a4d4f490 100644 --- a/src/main/java/com/threerings/io/Streamer.java +++ b/src/main/java/com/threerings/io/Streamer.java @@ -30,8 +30,10 @@ import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; +import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Set; import java.io.IOException; @@ -74,9 +76,11 @@ public class Streamer return true; } - // if it's not an array, it must be streamable + // if it's not an array, it must be streamable, or a Collection or Map if (!target.isArray()) { - return Streamable.class.isAssignableFrom(target); + return Streamable.class.isAssignableFrom(target) || + Collection.class.isAssignableFrom(target) || + Map.class.isAssignableFrom(target); } // otherwise extract the component type and loop back around for another check... @@ -95,6 +99,31 @@ public class Streamer ((Enum)object).getDeclaringClass() : object.getClass(); } + /** + * If the specified class is not Streamable and is a Collection type, return the + * most specific supported Collection interface type; otherwise return null. + */ + public static Class getCollectionClass (Class clazz) + { + if (Streamable.class.isAssignableFrom(clazz)) { + // the class is natively streamable, let's ignore it + return null; + } + if (Map.class.isAssignableFrom(clazz)) { + return Map.class; + } + if (Set.class.isAssignableFrom(clazz)) { + return Set.class; + } + if (List.class.isAssignableFrom(clazz)) { + return List.class; + } + if (Collection.class.isAssignableFrom(clazz)) { + return Collection.class; + } + return null; + } + /** * Obtains a {@link Streamer} that can be used to read and write objects of the specified * target class. {@link Streamer} instances are shared among all {@link ObjectInputStream}s and @@ -115,28 +144,37 @@ public class Streamer Streamer stream = _streamers.get(target); if (stream == null) { - // make sure this is a streamable class - if (!isStreamable(target)) { - throw new IOException( - "Requested to stream invalid class '" + target.getName() + "'"); + // see if it's a collection type + Class collClass = getCollectionClass(target); + if (collClass != null) { + stream = getStreamer(collClass); + + } else { + // otherwise make sure this is a streamable class + if (!isStreamable(target)) { + throw new IOException( + "Requested to stream invalid class '" + target.getName() + "'"); + } + // create a streamer for this class and cache it + if (ObjectInputStream.STREAM_DEBUG) { + log.info("Creating a streamer for '" + target.getName() + "'."); + } + + // create our streamer in a privileged block so that it can introspect on the to be + // streamed class + try { + stream = AccessController.doPrivileged( + new PrivilegedExceptionAction() { + public Streamer run () throws IOException { + return new Streamer(target); + } + }); + } catch (PrivilegedActionException pae) { + throw (IOException) pae.getCause(); + } } - // create a streamer for this class and cache it - if (ObjectInputStream.STREAM_DEBUG) { - log.info("Creating a streamer for '" + target.getName() + "'."); - } - - // create our streamer in a privileged block so that it can introspect on the to be - // streamed class - try { - stream = AccessController.doPrivileged(new PrivilegedExceptionAction() { - public Streamer run () throws IOException { - return new Streamer(target); - } - }); - } catch (PrivilegedActionException pae) { - throw (IOException) pae.getCause(); - } + // cache the streamer by the class type _streamers.put(target, stream); } return stream; @@ -224,6 +262,9 @@ public class Streamer } // otherwise simply write out the fields via our field marshallers + if (_marshallers == null) { + initMarshallers(); + } int fcount = _fields.length; for (int ii = 0; ii < fcount; ii++) { Field field = _fields[ii]; @@ -373,6 +414,9 @@ public class Streamer } // otherwise simply read the fields via our field marshallers + if (_marshallers == null) { + initMarshallers(); + } int fcount = _fields.length; for (int ii = 0; ii < fcount; ii++) { Field field = _fields[ii]; @@ -457,6 +501,10 @@ public class Streamer } // and that's all we'll need return; + + } else if (_target.isEnum()) { + // nothing extra for enums + return; } // look up the reader and writer methods @@ -471,9 +519,21 @@ public class Streamer // nothing to worry about, we just don't have one } + // let's try to fail fast and initialize the marshallers now if we know + // we're going to need them. + if ((_reader == null) || (_writer == null)) { + initMarshallers(); + } + } + + /** + * Initialize the marshallers. + */ + protected void initMarshallers () + { // reflect on all the object's fields and remove all marked with NotStreamable List fields = Lists.newArrayList(); - ClassUtil.getFields(target, fields); + ClassUtil.getFields(_target, fields); _fields = Iterables.toArray(Iterables.filter(fields, _isStreamableFieldPred), Field.class); int fcount = _fields.length; diff --git a/src/main/java/com/threerings/presents/dobj/CompoundEvent.java b/src/main/java/com/threerings/presents/dobj/CompoundEvent.java index 28b309abf..a26418c4d 100644 --- a/src/main/java/com/threerings/presents/dobj/CompoundEvent.java +++ b/src/main/java/com/threerings/presents/dobj/CompoundEvent.java @@ -23,7 +23,7 @@ package com.threerings.presents.dobj; import java.util.List; -import com.threerings.util.StreamableArrayList; +import com.google.common.collect.Lists; import com.threerings.presents.net.Transport; @@ -57,7 +57,7 @@ public class CompoundEvent extends DEvent _omgr = omgr; _target = target; - _events = StreamableArrayList.newList(); + _events = Lists.newArrayList(); } /** @@ -176,7 +176,7 @@ public class CompoundEvent extends DEvent { buf.append("COMPOUND:"); super.toString(buf); - for (int ii = 0; ii < _events.size(); ii++) { + for (int ii = 0, nn = _events.size(); ii < nn; ii++) { buf.append(", ").append(_events.get(ii)); } } @@ -188,5 +188,5 @@ public class CompoundEvent extends DEvent protected transient DObject _target; /** A list of the events associated with this compound event. */ - protected StreamableArrayList _events; + protected List _events; }