Avi-Levi / kryo

Automatically exported from code.google.com/p/kryo
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Kryo customization is good only for final classes #86

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Kryo seems ignoring that some objects have writeObject(ObjectStream) method 
that is invoked by standard java serialization. In order to handle this case, I 
intercept the default serializer and call on the object beforeSerialized() 
before serialization.

Serializer deflt = kryo.getDefaultSerializer(MyItem.class)
kryo.addDefaultSerializer(MyItem.class, new Serializer() {
    public void write(Kryo kryo, Output output, Object object) {
        ((MyItem)object).beforeSerialized();
        deflt.write(kryo, output, object);

The problem is that default serializer, when overriden, serializes only MyItem 
fields but not the fields of object.getClass(), which extends MyItem. So, I get 
abridged objects when customize the serialization. Can you please fix this and 
show how no-arg constructors can be instantiated?

Original issue reported on code.google.com by valtih1...@gmail.com on 5 Sep 2012 at 1:42

GoogleCodeExporter commented 9 years ago
These two things, writeObject invocation and no-arg constructors, make Kryo a 
prohibitive replacement for the Java serialization.

Original comment by valtih1...@gmail.com on 5 Sep 2012 at 1:55

GoogleCodeExporter commented 9 years ago
May be there is some gate function, like writeNullOrRef that precedes every 
object serialization? Can I override it?

Original comment by valtih1...@gmail.com on 5 Sep 2012 at 1:56

GoogleCodeExporter commented 9 years ago
I think that I have succeeded by intercepting writeReferenceOrNull 

    Kryo kryo = new Kryo() {
        public boolean writeReferenceOrNull (Output output, Object object, boolean mayBeNull) {
            if (object instanceof IGItem) {
                ((IGItem) object).preSerialize();
            }

            return super.writeReferenceOrNull(output, object, mayBeNull);
        }

But I am not sure how reliable this is and I had to patch the function's 
visibility: package -> public

Original comment by valtih1...@gmail.com on 5 Sep 2012 at 2:26

GoogleCodeExporter commented 9 years ago
I am having difficulty understanding what your issue is.

Kryo ignores writeObject(ObjectStream) because Kryo has nothing to do with Java 
built-in serialization.

Java built-in serialization constructs objects without invoking constructors. 
This requires classes to know about serialization to avoid inconsistent state. 
This is not recommended. It is better to use a constructor, but see "object 
creation" in the docs if you still need this type of object creation.

You seem to want to invoke a method before serialization. You can do this by 
overriding the serializer method:

kryo.register(SomeClass.class, new FieldSerializer<SomeClass>(kryo, 
SomeClass.class) {
   public void write (Kryo kryo, Output output, SomeClass object) {
      object.beforeSerialization();
      super.write(kryo, output, object);
   }
});

If you want to do this for all your objects, you can have an interface define 
beforeSerialization(), create a serialize class similar to the above that uses 
instanceof to call beforeSerialization() in Serializer#write(), and use 
Kryo#setDefaultSerializer() to use that class as the default for all objects. 
Alternatively you can subclass Kryo and return your serializer in 
newDefaultSerializer().

Original comment by nathan.s...@gmail.com on 5 Sep 2012 at 10:37

GoogleCodeExporter commented 9 years ago
> Kryo ignores writeObject(ObjectStream) because Kryo has nothing to do with 
Java built-in serialization.

writeObject(ObjectStream) is used for something different than writing into 
java stream. In my project, it just converts some fields into DB identifiers, 
before object is serialized. Externalization, for instance, provides 
writeExternal() for the same purpose. Any serialization needs an event system. 
Serializes an object? Fire event! Kryo provides nothing: neither listeners nor 
simple overridable mechanism. Not even a direct guideline how to achieve the 
same goal. 

This is a caveat (as well as no-arg constructors and inner classes) that make 
transition from java serialization -> Kryo painful. You could mention that so 
that people do not need to figure out this by experiment. 

> If you want to do this for all your objects, you can have an interface define 
beforeSerialization(), create a serialize class similar to the above that uses 
instanceof to call beforeSerialization() in Serializer#write(), and use 
Kryo#setDefaultSerializer() to use that class as the default for all objects. 

How is this different from what I tried? Where is my mistake? Do you see that I 
have to remember the default serializer and call it for actual serialization of 
the fields, after I fire necessary events? Do you see that I loose some fields, 
the ones that extend the SomeClass definition? So, it works only when SomeClass 
is "final". How can this approach work for "all objects" after that? Why do you 
mark this issue as "inappropriate"?

> Alternatively you can subclass Kryo and return your serializer in 
newDefaultSerializer()

I tried

/***********
Kryo kryo = new Kryo() {
    protected Serializer newDefaultSerializer(Class type) {
        final Serializer def = super.newDefaultSerializer(type);
        Serializer custom = new Serializer() {

            @Override
            public void write(Kryo kryo, Output output, Object object) {
                System.err.println("writing " + object + ":" + object.getClass().getSimpleName());
                def.write(kryo, output, object);
            }

            @Override
            public Object read(Kryo kryo, Input input, Class type) {
                System.err.println("reading " + type.getSimpleName());
                return def.read(kryo, input, type);
            }
        };
        System.err.println("new serializer for "+ type.getSimpleName());
        return custom;
    }
};
ByteArrayOutputStream baos = new ByteArrayOutputStream();
Output output = new Output(baos);
A a = new A("a"), b = new A("b"), c = new A("c");
a.list.add(c);
kryo.writeClassAndObject(output, a);
output.close();
System.err.println(baos.size() + " after first write");

*****/

Where A is a class with a List that may contain subelements. Here is what I get:

/**********
new serializer for A
writing a:A
writing c:A
50 after first write
writing b:A
90 after second write
*///////

So, this intercepted all my objects. Thanks. List was not captured because it 
has a more preferred serializer, I suspect. So, it seems to work but overriding 
writeReferenceOrNull was much less pain than this. You should provide a 
shortcut mechanism for intercepting the serialization/generating serialization 
events. The writeReferenceOrNull seems to serve exactly that purpose. 

Original comment by valtih1...@gmail.com on 6 Sep 2012 at 8:26

GoogleCodeExporter commented 9 years ago
> List was not captured because it has a more preferred serializer, I suspect.

Correct.

> How is this different from what I tried? Where is my mistake? Do you see that 
I have to remember the default serializer and call it for actual serialization 
of the fields, after I fire necessary events? Do you see that I loose some 
fields, the ones that extend the SomeClass definition?

I have no idea because you did not provide a simple, executable example. I 
won't waste my time trying to cobble one together just to hope I end up with 
whatever you actually tried.

> Do you see that I have to remember the default serializer and call it for 
actual serialization of the fields, after I fire necessary events?

Any class with your "beforeSerialization" method is application specific, and 
so you know what serializer it should use. There is likely no need to remember 
the default.

> Why do you mark this issue as "inappropriate"?

I didn't. It is marked as invalid.

> List was not captured because it has a more preferred serializer, I suspect.

Correct. Again, since you didn't provide the code, I can only guess you used an 
ArrayList. Intercepting this is useless, since ArrayList doesn't have a 
"beforeSerialization" method.

> The writeReferenceOrNull seems to serve exactly that purpose. 

No. That method is not called if references are disabled.

I expect you know what serializer to use for classes you want to call 
"beforeSerialization" on and can easily invoke it there. I see calling a method 
like this to be exactly as application specific as any other action needed to 
serialize an application specific class. If you don't want to do the intercept 
in the serializer, then you can override the writeObject, writeObjectOrNull, 
and writeClassAndObject methods. These are guaranteed to be called for all 
objects that are serialized.

Original comment by nathan.s...@gmail.com on 6 Sep 2012 at 9:54

GoogleCodeExporter commented 9 years ago
> I have no idea because you did not provide a simple, executable example. I 
won't waste my time trying to cobble one together just to hope I end up with 
whatever you actually tried.

How can you say such thing when your code snippets are not more executable than 
mine? I expected that you know about the issue, that not all fields are 
serialized, when you register a serializer. Here are three alternatives and 
only OverridingNewDefaultSerializer fires events and serializes everything. 

        public class A {
            String f1;
            A(String a) {
                f1 = a;
            }
            List list = new ArrayList();
            public String toString() {
                return "f1 = " + f1 + ":" + f1.getClass().getSimpleName();
            }

            boolean written = false;

            public void beforeSerialization() {
                written = true;
            }

        }

        public class B extends A {

            B(String a) {
                super(a);
                f2 = 1;
            }

            Integer f2;
            public String toString() {
                Class t = f2.getClass(); // <- AddingDefaultSerializer NullPointerException
                        // because f2 was not serialized

                return super.toString() + ", f2=" + f2 + ":" + t.getSimpleName();
            }

            boolean written2 = false;
            public void beforeSerialization() {
                written2 = true;
                super.beforeSerialization();
            }
        }

    public class OverridingNewDefaultSerializerCustomization extends Kryo {
        protected Serializer newDefaultSerializer(Class type) {
            final Serializer def = super.newDefaultSerializer(type);
            Serializer custom = new Serializer() {

                @Override
                public void write(Kryo kryo, Output output, Object object) {
                    System.err.println("writing " + object + ":" + object.getClass().getSimpleName());
                    if (object instanceof A)
                        ((A)object).beforeSerialization();
                    def.write(kryo, output, object);
                }

                @Override
                public Object read(Kryo kryo, Input input, Class type) {
                    System.err.println("reading " + type.getSimpleName());
                    return def.read(kryo, input, type);
                }
            };
            System.err.println("new serializer for "+ type.getSimpleName());
            return custom;
        }

    }

        public class RegisteringFieldSerializerCustomization extends Kryo {
            {
                register(A.class, new FieldSerializer<A>(this, A.class) {
                       public void write (Kryo kryo, Output output, A object) {
                           System.err.println("writing " + object.getClass().getSimpleName() + "(" + object + ")");
                              object.beforeSerialization();
                          super.write(kryo, output, object);
                       }
                });

            }

        }

        public class AddingDefaultSerializerCustomization extends Kryo {
            {
                final Serializer def = getDefaultSerializer(A.class);
                addDefaultSerializer(A.class, new Serializer() {
                    public void write(Kryo kryo, Output output, Object object) {
                        System.err.println("writing " + object.getClass().getSimpleName() + "(" + object + ")");
                        if (object instanceof A)
                            ((A)object).beforeSerialization();
                        def.write(kryo, output, object);
                    }

                    public Object read(Kryo kryo, Input input, Class type) {
                        return def.read(kryo, input, type);
                    }
                });
            }

        }

        public class KryoCustomization {

            public static void main(String[] args) throws InstantiationException, IllegalAccessException {
                for (Class s: new Class[] {
                        OverridingNewDefaultSerializerCustomization.class,
                        RegisteringFieldSerializerCustomization.class,
                        AddingDefaultSerializerCustomization.class})
                {
                    test(s);
                }

            }

            public static void test(Class<Kryo> cls) throws InstantiationException, IllegalAccessException {

                System.err.println("- - -\ntesting " + cls + "\n");
                Kryo kryo = cls.newInstance();
                kryo.setInstantiatorStrategy(new StdInstantiatorStrategy());
                ByteArrayOutputStream baos = new ByteArrayOutputStream();

                // Save
                Output output = new Output(baos);
                A a = new B("a"), b = new B("b"), c = new B("c");
                a.list.add(c);
                kryo.writeClassAndObject(output, a);
                output.close();
                System.err.println(baos.size() + " after first write");
                System.err.println(cls.getSimpleName() + " WRITE " +
                        ((c.written && ((B)c).written2) ? "SUCCES" : "FAILURE"));

                // Another save
                output = new Output(baos);
                kryo.writeClassAndObject(output, b);
                output.close();
                System.err.println(baos.size() + " after second write");

                // Restore
                ByteArrayInputStream in = new ByteArrayInputStream(baos.toByteArray());
                Input input = new Input(in);
                B o = null;
                for (int i = 0; i < 2; i++) {
                    o = (B) kryo.readClassAndObject(input);
                    System.err.println(" " + i + ". " + o);
                }
                System.err.println(cls.getSimpleName() + " READ " + (o.f2 == 1 ? "SUCCESS" : "FAILURE"));

            }

        }

The output is 

    - - -
    testing class kryo_test.OverridingNewDefaultSerializerCustomization

    new serializer for B
    writing f1 = a:String, f2=1:Integer:B
    writing f1 = c:String, f2=1:Integer:B
    58 after first write
    OverridingNewDefaultSerializerCustomization WRITE SUCCES
    writing f1 = b:String, f2=1:Integer:B
    102 after second write
    reading B
    reading B
     0. f1 = a:String, f2=1:Integer
    reading B
     1. f1 = b:String, f2=1:Integer
    OverridingNewDefaultSerializerCustomization READ SUCCESS
    - - -
    testing class kryo_test.RegisteringFieldSerializerCustomization

    58 after first write
    RegisteringFieldSerializerCustomization WRITE FAILURE
    102 after second write
     0. f1 = a:String, f2=1:Integer
     1. f1 = b:String, f2=1:Integer
    RegisteringFieldSerializerCustomization READ SUCCESS
    - - -
    testing class kryo_test.AddingDefaultSerializerCustomization

    writing B(f1 = a:String, f2=1:Integer)
    writing B(f1 = c:String, f2=1:Integer)
    52 after first write
    AddingDefaultSerializerCustomization WRITE SUCCES
    writing B(f1 = b:String, f2=1:Integer)
    93 after second write
    Exception in thread "main" java.lang.NullPointerException
        at kryo_test.B.toString(B.java:12)
        at java.lang.String.valueOf(String.java:2902)
        at java.lang.StringBuilder.append(StringBuilder.java:128)
        at kryo_test.KryoCustomization.test(KryoCustomization.java:57)
        at kryo_test.KryoCustomization.main(KryoCustomization.java:22)

registring field serializer is not called at all and last test that adds 
default serializer fails because it ignores the fields besides the ones 
declared in A.class. The failing solutions also suffer from the need to be 
careful and register all classes that implement KryoEvents interface. 
Registering is thus fragile/error-prone. This is not oblivious. Is it a kind of 
test you want the users to write and figure out experimentally when they want 
just a basic stuff of event triggering? 

>> Why do you mark this issue as "inappropriate"?

> I didn't. It is marked as invalid.

Ok, it's very big difference

> Any class with your "beforeSerialization" method is application specific, and 
so you know what serializer it should use.
> I see calling a method like this to be exactly as application specific as any 
other action needed to serialize an application specific class. 

Any class you serialize is application-specific. Why user must write his own 
Kryo serializer for getting events? He must write his own serialization 
framework! We should better not have the Kryo for this puropose!

Actually, it is very easy to know which client method the serializer must to 
invoke. You just define that method as a author of the framework, like java 
serializer does. As result, java serializer is easer to customize, despite it 
is not customizable at all. It is because demanding a custom wheel reinvention 
for a typical thing is stupid. 

Frameworks from different vendors may have incompatible event-firing 
mechanisms. It is fact user have to live with. Kryo, in similar way, could 
prescribe implementing KryoExternalizable.willSerializeYouInSecond() for any 
user who wants to receive seiralization events. Java writeObject/writeExternal 
methods demand invoking java-default serialization out.writeDefaultObject(). 
So, Kryo cannot call them when serializes. So, I have to rewirte my classes 
anyway when taking a new serialization engine. But, it is not enough when using 
Kryo! In addition to event-listeners, Kryo wants you to create a custom 
serializer! May be this is also not flexible enough? May be we should better 
not have Kryo at all for this reason to let the user to implement the 
application-specific serialization engine? Serialization is totally 
application-specific task, after all. 

Original comment by valtih1...@gmail.com on 6 Sep 2012 at 1:55

GoogleCodeExporter commented 9 years ago
Kryo and support for Kryo is free. You are not owed anything. It appears that 
you think I will spend my time supporting you no matter how rude and sarcastic 
you behave. This is not the case. Perhaps you are better off using a different 
library.

Original comment by nathan.s...@gmail.com on 6 Sep 2012 at 7:24

GoogleCodeExporter commented 9 years ago
Thank you for appreciating my research. I have also discovered that there is 
another extensible mechanism, KryoSerializable, which is exactly what I was 
looking for. Thank you for explaining how bad such "application-specific" thing 
is. I recommend it is lifted ahead of custom serializers section in the Quick 
Start. 

Original comment by valtih1...@gmail.com on 7 Sep 2012 at 8:35

GoogleCodeExporter commented 9 years ago
No, KryoSerializable serializable has the same issue as Externalizable: no way 
to invoke the default (de)serialization. Using new FieldSerializer.read() in 
read creates a new instance whereas only population of existing object needed. 
But, that is another issue. So overriding the new default serializer seems only 
the way. 

Original comment by valtih1...@gmail.com on 7 Sep 2012 at 10:18