views:

447

answers:

2

Referencing this answer to a question.

Can this be rewritten as:

    private static BinaryFormatter formatter = new BinaryFormatter();

    public static T DeepClone<T>(this T a)
 {
  using(MemoryStream stream = new MemoryStream())
  {
   formatter.Serialize(stream, a);
   stream.Position = 0;
   return (T)formatter.Deserialize(stream);
  }
 }

So avoiding constructing (and GC'ing) a new BinaryFormatter for each call?

This code path is getting hit very frequently as it involves our caching layer and I would like to make it as lightweight as possible.

Thanks.

+3  A: 

According to MSDN:

Any public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe.

So you need to synchronize access to Serialize/Deserialize methods.

Have you identified particular performance issues by creating a local serializer instance every time?


UPDATE:

I would trust MSDN because even if in some cases we can verify that instance members might be thread safe this doesn't mean that with the next service pack/update/framework version this will continue to be the case.

Looking with Reflector at BinaryFormatter constructor:

public BinaryFormatter()
{
    this.m_typeFormat = FormatterTypeStyle.TypesAlways;
    this.m_securityLevel = TypeFilterLevel.Full;
    this.m_surrogates = null;
    this.m_context = new StreamingContext(StreamingContextStates.All);
}

And StreamingContext constructor:

public StreamingContext(StreamingContextStates state, object additional)
{
    this.m_state = state;
    this.m_additionalContext = additional;
}

Quite frankly assigning 6 properties (most of which are enums) should be blindingly fast. IMHO most of the time would be spent in Serialize/Deserialize methods.

Darin Dimitrov
Yes that is currently the hot path (we've measured). Its not the end of the world to instanciate a formatted for each request, but I wondered if anyone knew if it did internal caching etc. I was aware of the notice on MSDN, but as you probably know, it says that for a lot of classes that actually are instance thread safe in reality :)
Kieran Benton
+4  A: 

You could use the [ThreadStatic] attribute and initialize if value is null. This will work assuming your reusing threads.

[ThreadStatic]
private static BinaryFormatter formatter = null;

public static T DeepClone<T>(this T a)
    {
            if( formatter == null ) formatter = new BinaryFormatter();
            using(MemoryStream stream = new MemoryStream())
            {
                    formatter.Serialize(stream, a);
                    stream.Position = 0;
                    return (T)formatter.Deserialize(stream);
            }
    }

Of course, the other option is to use Relfector.Net from Red Gate and review the implementation of the binary formatter. After reading the code you should be able to decide if it is safe for cross threaded use; however, Darin is correct in that it could break in a future release.

csharptest.net