OnDeserializedAttribute

[GoogleCodeExporter]

First of all, thank you very much for this great piece of work :) I try to
use it as an alternative to WCF's built-in serializers, and it looks
promising in terms of speed and size.

Now I have an issue with classes relying on execution of methods marked
with [OnDeserialized]. As an example, they could be used to initialize
ReadOnlyCollections that can't be serialized by protobuf because they don't
have an empty default constructor. I can work around this by adding some
ugly extra code (cannot use automatic properties anymore; need to make sure
in getters that collections are initialized etc.), however I obviously want
to avoid this.

I guess calling OnDeserialized would be much easier to implement then
OnDeserializing, OnSerializing etc., as the StreamContext's Context
property passed to the method by the DataContractSerializer is null anyway.
So it seems to be just a bit of reflection and a method invoke.

Would be great if this could be added to the code. I looked through the
code, but I'm not really sure where would be the best place to add this call...

Original issue reported on code.google.com by schmidt...@gmail.com on 15 Feb 2009 at 8:48

[GoogleCodeExporter]

Short version - yes, I've thought about that too. I can probably get something 
working committed tomorrow - that do? Re the readonly collection - are you 
thinking 
in terms of popsicle immutability? I need to put the data *somewhere* while it 
is 
deserializing the items...

Long version:
This is actually something I've put a lot of thought into already, and I think 
it can 
be done pretty trivially - and it would certainly work OK for messages 
serialized 
with protobuf-net; however, for all that google got right, there is one problem 
with 
the protocol buffers format - it doesn't guarantee when a message has ended 
(you can 
receive multiple fields for the same child entity, completely independently; 
protobuf-net never *sends* this, but it is a valid construct).

Given that protobuf-net doesn't create messages in this way, I'm actually 
inclined to 
use the simplest solution possible - the caveat is that it is possible that 
your 
OnDeserialized method might get called multiple times - but only if (for 
example) you 
are processing a byte stream specially constructed from a java/C++ client. Do 
you 
think that would be agreeable? The alternative is that I would need to do a lot 
of 
object tracking, and run a fix-up step at the end of the process, which I 
really 
don't want to do. Again - it wouldn't apply for data serialized by protobuf-net.

Original comment by marc.gravell on 15 Feb 2009 at 11:36

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Thank you very much for your quick reply! 

Regarding ReadOnlyCollections: Yes, I want to have the collections immutable; 
they
are part of a calculated result set, and modifying them afterwards could make 
the
result inconsistent. So I have private List<T> instances that are marked as
DataMember, and public ReadOnlyCollections without a setter.

My long version: You are right, this is really a decision if protobuf-net 
should be a
special-purpose, more efficient WCF formatter just "inspired" by Google's
specification; or if it should focus on allowing .NET developers to handle 
protocol
buffers in a similar and wire-compatible way as with Google's implementation for
C++/Java/Python.

In the meantime I did more comparisons with real-world data, doing
serializing/deserializing roundtrips in memory. Turns out that the size 
advantage
over DataContractSerializer (XML) is pretty much evened out when applying GZIP
compression to both and explicitly declaring "short" names in the [DataMember]
attribute constructors; it's still about 30% faster as long as only public 
members
are marked as DataMembers, however as soon as I use private fields for 
serializing,
protobuf-net becomes more than 50% slower. I guess in this case reflection is 
used to
get/set the fields, and the DataContractSerializer somehow can avoid this.

So I'm not sure if WCF-compatible serializing is really the best use case for
protobuf, so maybe you should focus more on making protobuf-net behave in a way 
a
C++/java/python user of protocol buffers would expect it to work.

What do you think?

Original comment by schmidt...@gmail.com on 16 Feb 2009 at 7:56

[GoogleCodeExporter]

My main aim is simply to provide a useful tool that lets people shift data 
around 
conveniently ;-p In particular, I'm trying to retain the cross-platform options 
(so 
it can work on Silverlight, CF, etc, where binary serialization is weak).

It turns out that the [OnBlahAttribute] classes (nor IDeserializationCallback) 
don't 
exist on CF/Silverlight/etc, so I'm tempted to go for a bespoke interface (it 
is a 
cleaner model to implement).

Re the performance of fields - I could probably optimize that quite easily; 
I'll see 
what I can do...

Marc

Original comment by marc.gravell on 16 Feb 2009 at 8:06

[GoogleCodeExporter]

For info, I've just done a commit with much improved field access; I haven't 
released 
the binaries, but it is in the source. That should fix the performance 
degredation 
with fields.

Original comment by marc.gravell on 16 Feb 2009 at 9:16

[GoogleCodeExporter]

Cool, have I accidentally ordered 24/7 platinum support? :-) Thank you very 
much!
Next time I can test is this evening, I will then let you know about the latest 
numbers.

Original comment by schmidt...@gmail.com on 16 Feb 2009 at 11:03

[GoogleCodeExporter]

Unless you say otherwise, I'll assume you're happy to compile from source; I'd 
rather 
get around to implementing the logic behind the callbacks before releasing the 
dlls, 
but can do if you need (it is all handled by a build script, so there's no cost 
to 
releasing).

Original comment by marc.gravell on 16 Feb 2009 at 11:16

• Changed state: Accepted

[GoogleCodeExporter]

Of course, that's fine, I have checked out the source from SVN anyway, it's 
just that
I'm having a different day-project at the moment ;)

Original comment by schmidt...@gmail.com on 16 Feb 2009 at 12:25

[GoogleCodeExporter]

ISerializerCallback

Original comment by marc.gravell on 16 Feb 2009 at 4:02

• Changed state: Fixed

[GoogleCodeExporter]

So, I finally ran my tests against revision 233, and it leaves 
DataContractSerializer
in the dust again :)

Regarding ISerializerCallback I have two thoughts:

   * I'm not really happy with having to implement always all four methods, when I
need just one or two of them. Probably compiler optimization will remove the 
empty
method calls, however it "smells"...

   * Methods must be public. I know, I can use explicit interface implementation to
hide it from users of my API (at least in IntelliSense), but same story here.

I would really prefer attributes over an interface for this, and I think it 
would
also fit better into the overall design. But that's just my 2 cents.

Thank you very much again!

Original comment by schmidt...@gmail.com on 16 Feb 2009 at 10:20

[GoogleCodeExporter]

Interesting input. I'm glad the performance picked up ;-p

Re the attributes... maybe I was worrying unduly... my concern was the 
confusion if 
multiple methods were attributed, but I've checked with DataContractSerializer 
and it 
doesn't let you do this anyway...

I'll take another look... the good thing is that it is fairly simple to swap 
the 
implementation details here.

Original comment by marc.gravell on 17 Feb 2009 at 5:47

• Changed state: Started

[GoogleCodeExporter]

Fixed (now using attributes; interface removed). It supports both the standard 
2.0 
attributes, and some proto-specific ones (so that it can be used on 
CF/Silverlight). 
Allows either "void Foo()" or "void Foo(StreamingContext)" signatures. No 
duplicates. 
Callbacks only supported on the contract-root of any hierarchy (virtual is 
fine).

Original comment by marc.gravell on 17 Feb 2009 at 9:44

• Changed state: Fixed

[GoogleCodeExporter]

Big thanks! No negative impact on performance, got rid of all my getter/setter 
code,
and now my data model has no dependency on protobuf anymore and no extra code 
with
still full functionality. Exactly as I wanted it. Thanks again for your great 
support!

Original comment by schmidt...@gmail.com on 18 Feb 2009 at 6:45

[GoogleCodeExporter]

No problem

Original comment by marc.gravell on 18 Feb 2009 at 8:06

• Changed state: Verified