Support inner classes / non static member classes
Martin Grotzke
as it was mentioned in issue #15 (final fields should be handled by
FieldSerializer if setFieldsAsAccessible is set to true) kryo right
now does not support handling of inner classes (non static member
classes).
As the inner class needs a reference to the enclosing instance IMHO
this is an issue of reference management. And IMHO (from a user
perspective) it would be ok to only support inner classes if the
ReferenceFieldSerializer is used as newDefaultSerializer.
When the inner class is deserialized it would need to be able to
obtain the reference to the enclosing instance - therefore the
enclosing instance needs to be registered already.
Therefore the reference should be registered directly after
newInstance was invoked and before fields get deserialized.
What do you think about this?
Cheers,
Martin
martin.grotzke
and in readObjectData the object created via newInstance is directly
stored in the references map.
Just to see what happens I removed the check on field.isSynthetic() in
FieldSerializer.rebuildCachedFields and my testInnerClass just works.
Do you see a problem with providing a switch to support synthetic
fields?
Cheers,
Martin
On Apr 6, 1:53 pm, Martin Grotzke <martin.grot...@googlemail.com>
wrote:
Nate
The Java Object Serialization Specification states that serializing non-static inner classes has problems and should be avoided:
http://java.sun.com/javase/6/docs/platform/serialization/spec/serial-arch.html#7182
"Synthetic fields generated by javac (or other JavaTM compilers) to implement inner classes are implementation dependent and may vary between compilers". FieldSerializer sorts field names by alpha so the order of the values in the serialized bytes are known. If the synthetic field names are different from when serialization occurred, deserialization would fail.
When you removed the isSynthetic check to make your test pass, are you using reflection to create objects? If so, just removing the isSynthetic check shouldn't be enough. Eg:
static public class Container {
public class Body {}
}
static public void main (String[] args) {
Kryo kryo = new Kryo();
kryo.register(Container.class);
kryo.register(Container.Body.class);
Container container = new Container();
Body body = container.new Body();
byte[] bytes = new ObjectBuffer(kryo).writeObject(body);
Body body2 = new ObjectBuffer(kryo).readObject(bytes, body.getClass());
}
This fails because Body cannot be constructed. To fix this, we need to pass an instance of Container to the Body constructor.
I don't really see a reasonable way to support this with FieldSerializer or ReferenceFieldSerializer because there is no way to know what reference to use for the enclosing reference.
A special serializer could be written for inner classes that would first deserialize the enclosing class reference, then construct the inner class with it, and then assign the inner class instance fields. But this doesn't get rid of the problems with the synthetic field name not being standardized. Anonymous inner class names are also not standardized.
It is so much simpler just to avoid serializing non-static inner classes. Hopefully a static inner class is an option! :)
-Nate
--
You received this message because you are subscribed to the "kryo-users" group.
http://groups.google.com/group/kryo-users
To unsubscribe, reply using "remove me" as the subject.
Martin Grotzke
> The Java Object Serialization Specification states that serializing
> non-static inner classes has problems and should be avoided:
> http://java.sun.com/javase/6/docs/platform/serialization/spec/serial-arch.html#7182
> "Synthetic fields generated by javac (or other JavaTM compilers) to
> implement inner classes are implementation dependent and may vary between
> compilers". FieldSerializer sorts field names by alpha so the order of the
> values in the serialized bytes are known. If the synthetic field names are
> different from when serialization occurred, deserialization would fail.
This is a valid argument for a heterogenous environment. However, in
my case the environmentis homogenous: if I have 20 (n) app servers
they are all running the same software, the used jars are _identical_.
E.g. all web apps are running with the identical wicket jars,
therefore it's *guaranteed* that the classes are built by the same
compiler and classes are identical. So for me this is not a valid
argument against using synthetic fields.
>
> When you removed the isSynthetic check to make your test pass, are you using
> reflection to create objects? If so, just removing the isSynthetic check
> shouldn't be enough. Eg:
>
> static public class Container {
> public class Body {}
> }
> static public void main (String[] args) {
> Kryo kryo = new Kryo();
> kryo.register(Container.class);
> kryo.register(Container.Body.class);
> Container container = new Container();
> Body body = container.new Body();
> byte[] bytes = new ObjectBuffer(kryo).writeObject(body);
> Body body2 = new ObjectBuffer(kryo).readObject(bytes, body.getClass());
> }
>
> This fails because Body cannot be constructed. To fix this, we need to pass
> an instance of Container to the Body constructor.
Well, this example fails, right. But it does also not reflect my use
case, because in my case, complete object graphs are serialized where
every referenced object is also available in the graph.
I think it's important that the kryo user that wants to use synthetic
fields he should be (made) aware of this. You might call it an option
that is still not recommended, it's only there to allow users to do
their crazy shit.
> It is so much simpler just to avoid serializing non-static inner classes.
> Hopefully a static inner class is an option! :)
Nope, it's not for me, as I serialize object graphs of libraries that
are not under my control.
Especially wicket makes heavy use of non-static inner classes, and
"unfortunately" I definitely want to support wicket with
memcached-session-manager.
Cheers,
Martin
--
Martin Grotzke
http://www.javakaffee.de/blog/
Nate
Well, this example fails, right. But it does also not reflect my use
case, because in my case, complete object graphs are serialized where
every referenced object is also available in the graph.
Even if the enclosing instance appears in the graph before the inner class instance, the inner class cannot be created. Its constructor *must* be passed the enclosing class instance.
I think it's important that the kryo user that wants to use synthetic
fields he should be (made) aware of this. You might call it an option
that is still not recommended, it's only there to allow users to do
their crazy shit.
Sure. Nearly all the interesting bits of Kryo happen in the serializers, so users are almost never boxed in. I'm all for adding features to the core library if they fit well.
I messed around and implemented non-static inner class support for ReferenceFieldSerializer, SVN r99. There is a unit test, too. The implementation was a bit tricky, but at least it doesn't increase the serialized size of non-inner classes.
> It is so much simpler just to avoid serializing non-static inner classes.Nope, it's not for me, as I serialize object graphs of libraries that
> Hopefully a static inner class is an option! :)
are not under my control.
Especially wicket makes heavy use of non-static inner classes, and
"unfortunately" I definitely want to support wicket with
memcached-session-manager.
-Nate
Martin Grotzke
unfortunately this change fails for my testcase with the container
class I already posted:
public static class Container {
@SuppressWarnings( "unused" )
private final Body _body;
public Container() {
_body = new Body();
}
class Body {
}
}
I just added a simple test to your ReferenceFieldSerializerTest (to
see if it's only wrong with my testclass/kryo setup, but this test
fails, too):
public void testNonStaticInnerClass2 () {
Kryo kryo = new Kryo() {
protected Serializer newDefaultSerializer (Class type) {
return new ReferenceFieldSerializer(this, type);
}
};
kryo.register(Container.class);
kryo.register(Container.Body.class);
Container outerClass = new Container();
roundTrip(kryo, 7, outerClass);
}
It fails where RFS tries to create an instance of the inner class
using a constructor taking the enclosing instance:
[...]
Caused by: com.esotericsoftware.kryo.SerializationException: Error
constructing inner class instance:
com.esotericsoftware.kryo.serialize.ReferenceFieldSerializerTest$Container$Body
at com.esotericsoftware.kryo.serialize.ReferenceFieldSerializer.readObjectData(ReferenceFieldSerializer.java:125)
at com.esotericsoftware.kryo.serialize.FieldSerializer.readObjectData(FieldSerializer.java:188)
at com.esotericsoftware.kryo.serialize.ReferenceFieldSerializer.readObjectData(ReferenceFieldSerializer.java:145)
at com.esotericsoftware.kryo.Kryo.readClassAndObject(Kryo.java:474)
... 22 more
Caused by: java.lang.NoSuchMethodException:
com.esotericsoftware.kryo.serialize.ReferenceFieldSerializerTest$Container$Body.<init>(com.esotericsoftware.kryo.serialize.ReferenceFieldSerializerTest$Container)
at java.lang.Class.getConstructor0(Class.java:2706)
at java.lang.Class.getConstructor(Class.java:1657)
at com.esotericsoftware.kryo.serialize.ReferenceFieldSerializer.readObjectData(ReferenceFieldSerializer.java:123)
... 25 more
Btw, if you have a look at my KryoTest ([1]) you see that
Kryo.newInstance first checks if it can create a new instance from a
default constructor, and otherwise uses the ReflectionFactory to
create a new instance. With this and the FieldSerializer without the
synthetic check my Container/Body couple could be serialized as I
wrote in my previous mail.
Cheers,
Martin
Martin Grotzke
I just wanted to mention this :-)
Thanx && cheers,
Martin
On Wed, Apr 7, 2010 at 1:43 PM, Nate <nathan...@gmail.com> wrote:
Nate
Ahh, I see why it worked when you used ReflectionFactory. Now I'm thinking you were right, it would be better to keep ReferenceFieldSerializer simple and people can use ReflectionFactory or similar if they want to serialize non-static inner classes. I'll add a setting to FieldSerializer to avoid the isSynthetic call and I'll remove inner class support from ReferenceFieldSerializer. It'll be in SVN if anyone ever wants to look it up.
BTW, you might want to use this instead of ReflectionFactory:
http://objenesis.googlecode.com/svn/docs/
-Nate
Martin Grotzke
Objenesis looks interesting btw, thanx for the hint!
Cheers,
Martin