Nested lock to the same object performance

Question

Hi

Is it a performace cost to use a nested locks to the same object.

Say we have:

    public void AddRange(IEnumeratable<Item> items)
    {
        lock (_syncObject)
        {
            foreach (Item item in items)
            {
                InsertItem(item);
            }
        }
    }

    public void InsertItem(Item item)
    {
        lock (_syncObject)
        {
            //..
        }
    }

Is it ok to do so "the performance side"?

Thanks in advance.

Answer 1

lock is not free. it has to check for certain things before returning. How many things and what it has to do, depends on implementation. I would guess that this kind of use is common and MS did some optimization for this usecase.

I'd still recommend that you'd have a separate implementation of AddRange with all the things done in one shot. This of course depends on the rest of the class' interface (are there listeners and can they receive messages that several objects were added etc.).

This is fairly easy testcase, do some millions of nested locking (what you propose) and the same with an other lock.

Notice also the different possible order if you use non-nested lock, you may get an object in middle of a range you're adding:

AddRange _sync1
  AddItem _sync2
  AddItem _sync2
  --- interruption, other thread calls:
  AddItem _sync2
  --- AddRange again:
  AddItem _sync2

When syncing with a single _syncObject, nobody can interrupt because the lock is already held by another thread.

Answer 2

I don't know how the performance is affected, but when we expect it to decrease performance i'd suggest you implement your code the other way around:

public void InsertItem(Item item)
{
    AddRange(new IEnumeratable({item}))
}

public void AddRange(IEnumeratable<Item> items)
{
    lock (_syncObject)
    {
        foreach (Item item in items)
        {
            // Insert code ..
        }
    }
}

@AddRange(new IEnumeratable({item})) : I'm not a syntax wizkid so please correct me if this is not right!

Answer 3

Lock has costs, I suggest you to implement your code like this:

public void AddRange(IEnumeratable<Item> items)
{
    lock (_syncObject) // Locking only once.
    {
        foreach (Item item in items)
        {
            InsertItemImpl(item);
        }
    }
}

private void InsertItemImpl(Item item)
{
     // inserting the item
}

public void InsertItem(Item item)
{
    lock (_syncObject)
    {
        InsertItemImpl(item);
    }
}