mbraceproject / FsPickler

A fast multi-format message serializer for .NET
http://mbraceproject.github.io/FsPickler/
MIT License
324 stars 52 forks source link

Problem with limiting API for defining custom picklers #67

Closed Horusiath closed 7 years ago

Horusiath commented 8 years ago

I've got following problem: I need to integrate with 3rd party library, which produces objects, that are not serializable, unless I will define custom way of pickling/unpickling for them. Those objects can be composed in fields of other objects, collections etc. I cannot annotate them with [<CustomPickler>], but I can recognize them by interface, they implement. Therefore the only possible option here seems to be defining custom pickler factory for that interface and use FsPickler.RegisterPicklerFactory on it. This however introduces some other problems:

Any clues how to solve these problems? Thanks in advance.

eiriktsarpalis commented 8 years ago

Hi Bartosz.

Regarding your points:

In any case, could you give a short code sample of what you are trying to do?

0x53A commented 7 years ago

Hi Eirik,

I am in a similar situation to Horusiath. (In my case I want to serialize Akka.Net objects, e.g. IActorRef).

If you have some time, could you please take a look at https://github.com/mbraceproject/FsPickler/pull/82?

This should enable the scenario Bartosz and I require, where a factory can be registered to a custom non-global context, while also keeping the default global behaviour.

eiriktsarpalis commented 7 years ago

Hi Lukas,

Apologies for the delayed response. The singleton pickler repository was deliberately introduced in later versions of the library. This happens because generating picklers is expensive and involves a great deal of code generation. In a typical application (like MBrace) a single pickler cache would take up to 4MB of memory. We want to avoid people misusing the library by generating a new set of picklers every time they serialize or deserialize something.

eiriktsarpalis commented 7 years ago

That said, I can think of few ways the library could make pickler caches cheaper, but it doesn't completely eradicate the problem and requires significant refactorings and breaking changes to the API.

0x53A commented 7 years ago

But how would it hurt to keep the current behavior by default (the singleton instance) while at the same time enabling independent parallel instances?

This would be backwards-compatible, both from a (binary-)api point and from a performance point.

It would only (possibly) degrade performance, when I actually spin up multiple containers, but when that happens, it is my explicit choice.

The current workarounds are even worse, because the only way to get isolation at the moment is by spinning up a new AppDomain.

eiriktsarpalis commented 7 years ago

We had very real space leak issues because of this. People are inclined to create a new serializer instance on each serializer/deserialization cycle even though the API in FsPickler is thread safe.

0x53A commented 7 years ago

@eiriktsarpalis So this is just a case of not giving the user enough rope to hang themselves with?

You could either mark the new API as obsolete, or add them to an Internal namespace to warn the users, but I think not having it is a big issue because it just hard-blocks certain scenarios.

In my case I want a context per Akka.net ActorSystem. There is (mostly) only one or two systems over the whole lifetime of the application.

dsyme commented 7 years ago

Closing this old discussion/question