skbkontur / GroBuf

Fast binary serializer
MIT License
64 stars 10 forks source link

Memory leak when more instatnces created #29

Closed Bicik closed 2 years ago

Bicik commented 2 years ago

Hello, I'd like to use GroBuf serializer as one of the "sinks" in our remoting client-server communication because of its high performance compare to standard BinaryFormatter. We have a single call type of communication (new instance of server-side class for each client call) so there are a lot of instances of serializer created. The Serializer class doesn't implement IDisposable interface and cause memory leak.

Is there any way, how to destroy the serializer? Is it possible to implement IDisposable interface? Use static instance of the serializer is not possible for us because of the perfomance degrade.

Thank you a lot V.

Anton92nd commented 2 years ago

Hi, Bicik. Can you be more specific about perfomance issues with single instance of the serializer? What is their nature?

Also, have you measured performance of Serialize/Deserialize methods when creating new serializer instance per request? I think there will be serious performance drop in these methods in such a case. Each GroBuf instance lazily generates serialization/deserialization code for each type you want to serialize/deserialize, compiles it and then stores compiled code in a cache associated with this instance. It's crucial to performance that you don't generate and compile code multiple times for a single type, so you can execute code from cache next time you see this type. If you dispose of the instance, you will also dispose of the cache, and you will need to generate & compile code again and again for each type you serialize. So GroBuf is intended to be used via singleton pattern.

Bicik commented 2 years ago

Hello, on the server side I used it this way:

`public override byte[] GetListBin(Guid threadId) { var tmpResult = GetList(threadId);

        if (tmpResult == null || tmpResult.Count == 0)
            return null;

        var serializer = new Serializer(new PropertiesExtractor(), options: GroBufOptions.WriteEmptyObjects);

        return serializer.Serialize(tmpResult);
    }`

Type of input parameter "tmpResult" is List<object[]>, where object[] is the result of DbDataReader.GetValues(). So types of these objects are only basic types returned from database. I think, that is the reason, why I've not found any performance issue when creating new instance of the serializer.

The problem with using GroBuf as singleton on the server side is simply creating a bottleneck. We have to serve a lot of clients at the time (1000+) and they download MBs of data (from 10s to 100s MBs). I didn´t find info, that the serializer is thread safe, so I suppose use it like this:

`private static Serializer serializer = new Serializer(new PropertiesExtractor(), options: GroBufOptions.WriteEmptyObjects); private static object serializerLock = new object(); ... public override byte[] GetListBin(Guid threadId) { var tmpResult = GetList(threadId);

        if (tmpResult == null || tmpResult.Count == 0)
            return null;
       lock(serializerLock)
       {
             return serializer.Serialize(tmpResult);
       }
    }`
Anton92nd commented 2 years ago

Yes, GroBuf is thread-safe, you don't have to put it inside a lock section. It doesn't have any state, that can be corrupted by multithreaded access. As for the generated code cache I mentioned earlier, it is always accessed in a thread-safe manner (via lock sections), and methods stored in the cache are also thread-safe, so you can serialize multiple instances of the same type concurrently.

Anton92nd commented 2 years ago

We use GroBuf like this in heavily-loaded applications for severals years and we haven't encountered any issues concerning concurrent serialization/deserialization. I think I should add a note about thread-safety somewhere in the README file.

Bicik commented 2 years ago

OK, thank you, I'll try to use it as a singleton. Just one question: Is it possible to complete Strongname to both assemblies downloaded via nuget packages?

Anton92nd commented 2 years ago

Check this out: https://www.nuget.org/packages/GroBuf/1.8.1

Feel free to close the issue if you don't have any more questions.