CODEWITHSUNDEEP
c#.netmultithreading
Alex
Alex

Reputation: 3470

Lock and do all work, or release and grab only when necessary?

Which of these two alternatives is the better one?

Locking outside the loops?

lock (_workLock)
{
    foreach (var resultObject in getObjectTask.Result)
    {
        foreach (var key in resultObject.Keys)
        {
            string value = resultObject.GetValue(key);
            _lockedObject.DoSomething(key, value);
        }
    }
}

or locking only when the locked object is accessed?

foreach (var resultObject in getObjectTask.Result)
{
    foreach (var key in resultObject.Keys)
    {
        string value = resultObject.GetValue(key);
        lock (_workLock)
            _lockedObject.DoSomething(key, value);
    }
}

There can potentially be 5-10 simultaneous operations wanting to do this roughly at the same time. Here's the surrounding code:

var tasks =
    from provider in _objectProviders
    select Task.Factory.StartNew(() => provider.Objects)
        .ContinueWith(getObjectTask =>
        {
            // One of the operation bodies from above would go here
        });

var taskList = Task.WhenAll(tasks);
taskList.Wait();

// Use results from operations here

EDIT: This isn't really an answer, so I'm not posting it as an answer, but after the input in the comment section, I refactored my code as such, and now I no longer need a lock at all:

var tasks =
    (from provider in _objectProviders
    select Task.Factory.StartNew(() => provider.Objects)).ToList();

while (tasks.Count > 0)
{
    int completedTask = Task.WaitAny(tasks.ToArray<Task>());
    var task = tasks[completedTask];
    var objects = task.Result;

    foreach (var resultObject in objects)
    {
        foreach (var key in resultObject.Keys)
        {
            string value = resultObject.GetValue(key);
            _unlockedObject.DoSomething(key, value);
        }
    }

    tasks.RemoveAt(completedTask);
}

Upvotes: 2

Views: 108

Answers (1)

Cort Ammon
Cort Ammon

Reputation: 10883

It Depends!

Which one is better is 100% dependent on what your task is doing.

Grabbing the lock inside the loop means you waste a ton of time acquiring and releasing the lock. However, if most of your time is spent doing non-locked parallel things, it could free up the lock enough to get more parallelism.

Grabbing the lock at the top of the loop prevents parallelism, but saves on locking, which can be expensive compared to your operation.

All you can do it look at your profiler and find out which one is best in your situation. For reference, locks on modern hardware take about 100-150 operations to lock/unlock if there is no contention (that's a rough rule of thumb, not a hard set value).

Consider a third option which is a hybrid of the two: do all of your parallel operations, and batch up the serial operations. Perhaps every 10 objects, grab the lock and quickly do all of the serial work. This lets you get the best of both worlds

Upvotes: 1

Related Questions