The bad thing around basic "lock" is that it locks that part of code for writing as well as reading...
You can't just lock in "Writing" method, as you could have incomplete "reading" in the meantime, so this has all the chance to prevent concurrent read even on thread-safe collections...
The best use-case for lock is when you want to be sure that a method won't be executed by multiple thread at the same time... (typically a method like Die() shouldn't allow concurrent calls at all, an object can only die() once at a given time)
Using too much synchronizing method (locks...) can reduce drastically the speed of a code part, methods could be waiting for a read lock to wear out from an other thread when they don't even need write access... (typically statistics thread will gather data periodically and may call methods to retrieve collection that will locks somewhere)
I don't know if there is anyway to handle read/write lock, I saw some code in DOL using this (around player login request I think), I have no idea if it could be more efficient or if it can turn into a "debug hell"...
Anyway locking collection is most of time a bad habit, we should handle access to the collection using method that can clearly choose between locking, getting a snapshot, updating asynchronously and not rely on objects that directly gives access to their Collection Property without checks and where a lot of parts of the game browse or update this collection without even "locking" it's syncroot everytime... (locking a syncroot is only usefull if every method does this before trying to access...)
For my part there are some area of DOL where I replace Generic Collections, with Concurrent Collections, but these aren't the best way to share collection, updates are asynchronous and can lead to racing condition, and "Getters" always get copies of the collections they use heavy memory and can have worse performance than a humanly "locked" collections, by default these objects are created with some "update queues" that allow to update them without even blocking (and the queue is then emptied asynchronously) to provide concurrency by default they create N queues where N is processor count (clearly overkill for most of our need)
There are some "blocking" collection in the "Concurrent" name space but it's meant for "producer->consumer" threaded scénarii, it block on read waiting for input (it's more like a FIFO)
They are also some good habit that are "Thread Safe", replacing a collection is always thread safe !!
- Code: Select all
public void MethodUpdateCollection(int withValue)
{
var copy = new Collection(m_privateCollection);
copy.Add(withValue);
m_privateCollection = copy;
}
At worst some "racing" conditions can retrieve the reference of the previous object... and be careful, the copy isn't thread safe
there should be some locking when copying the collection !
Project
DOL Server
Forum
