views:

142

answers:

7

I have a class which doesn't currently need to be thread-safe, but in future we might want to make a thread-safe version. The way I see it, I can either make it thread-safe now by putting locks around the relevant functions, or I can make them virtual now and put locks around them in overrides in a descendent class later on. That is, today I can either do this:

public void DoStuff()
{
    lock (this.SyncRoot)
    {
        // Do stuff...
    }
}

Or I can do this:

public virtual void DoStuff()
{
    // Do stuff...
}

Which option will get the stuff done faster, today?

+2  A: 

The second, since the virtual call is pretty cheap (the lock is also an additional call, which is going to be more expensive than the virtual call itself).

Also, the second leaves it to you to implement locking exactly when you need it.

Lucero
+1  A: 

The virtual call will almost certainly be faster. A virtual call involves an extra level of indirection. A lock normally involves a switch to kernel mode -- figure at least 100 times slower as a conservative estimate.

Jerry Coffin
Assuming that the common case is uncontended, then no, a lock at least on Linux and Windows do not imply entering the kernel. Now, however, acquiring and releasing a lock requires the processor to execute memory barrier instructions, which typically are quite expensive (hundreds of cycles, depending on hardware)
janneb
...and memory barriers kill your multi-CPU and multi-core efficiency.
Lucero
+2  A: 

If you're intending to make DoStuff synchronized (and guarantee it to be for any given subclass), then you're better off not making it virtual and using a protected virtual member to do the actual work.

public void DoStuff()
{
    lock(this.SyncRoot)
    {
        InternalDoStuff();
    }
}

protected virtual void InternalDoStuff()
{
    // do stuff
}

This also gives you the option of not locking in the current code (meaning that DoStuff simply calls InternalDoStuff with no other code), but still being able to slip it in at a later date without having to touch your inherited code.

As for speed, the placement of the lock statement won't have any effect.

Adam Robinson
+1  A: 

The second. If you don't need thread safety today don't do it. Expose the SyncRoot property so that in future the method could be turned thread-safe. But make sure to clearly state in the documentation that the method is not thread-safe.

Darin Dimitrov
the SyncRoot property is something I'd only add for the thread-safe version - I should have made that clearer in the question.
Simon
+3  A: 

A virtual function call is basically an array lookup plus an indirect function call. If the virtual call is happening in a loop, i.e. if the virtual function call is being called several times from the same place on the same instance, then on most iterations it will be no slower than a non-inlined normal function call. Modern CPU branch predictors predict the target of a virtual function call and speculatively execute this target in parallel with fetching the address of the function.

A lock, on the other hand, always involves at least an atomic operation or two under the hood. Such operations are pretty much guaranteed to wreak havok on a CPU pipeline because they require memory barriers.

dsimcha
+1  A: 

Egads, I can't find the reference, perhaps it's in Joe Duffy's book, but the Lock might be a no-op until another thread starts hitting it (i.e. lazy creation).

Also, this Lock does not enter kernel mode anyways, is based on the Interlocked### API.

Finally, musing about these topics is fun, but the best thing to do is to always time your own code.

Chris O
+1  A: 

I tested the VB SyncLock and it was almost 300 times slower.

dbasnett