4
\$\begingroup\$

Please let me know if you see any performance improvements, bugs, or anything you'd change and why.

    public static bool TrySpeculativeUpdate(ref int field, out int result,
        Func<int, int> update, Func<int, bool> shouldAbort)
    {
        SpinWait spinWait = new SpinWait();
        while (true)
        {
            int snapshot = field;
            if (shouldAbort(field))
            {
                result = 0;
                return false;
            }
            else
            {
                int calc = update(snapshot);
                if (Interlocked.CompareExchange(ref field, calc, snapshot) == snapshot)
                {
                    result = calc;
                    return true;
                }
            }

            spinWait.SpinOnce();
        }
    }

Can be used like this

    private bool TryIncreaseCapacity(out int newCapacity)
    {
        return TrySpeculativeUpdate(ref _currentCapacity, out newCapacity,
            (currentCapacity) => currentCapacity + 1,
            (currentCapacity) => currentCapacity == _maxCapacity);
    }

    if (this.TryIncreaseCapacity(out newCapacity))
    {
        ...
    }
    else
    {
        ...
    }

What I'm really trying to accomplish is the fastest thread safe version of Interlocked.Increment that will stop incrementing at a max value and I will have some way of detecting it stopped incrementing.

\$\endgroup\$
3
  • \$\begingroup\$ FYI, I tested this against locks and it's about 300%-350% faster on my machine. \$\endgroup\$ May 26, 2014 at 3:10
  • \$\begingroup\$ Are you really going to have several threads that spend most of their time trying to increase a counter? I assume that's what your test did, but I doubt it's how your real code will behave. If that's the case, this might be a premature optimization. \$\endgroup\$
    – svick
    Jun 24, 2014 at 13:38
  • \$\begingroup\$ Also, I think the read into snapshot should be volatile. You can use Volatile.Read() for that. \$\endgroup\$
    – svick
    Jun 24, 2014 at 13:40

1 Answer 1

2
\$\begingroup\$

I'm not sure I like the method's signature:

public static bool TrySpeculativeUpdate(ref int, out int, Func<int, int>, Func<int, bool>)

It's probably just me, but I like pushing ref and out parameters to the end of the parameters list, so I'd write it like this:

public static bool TrySpeculativeUpdate(Func<int, int>, Func<int, bool>, ref int, out int)

I'd use var to ease reading, and FWIW it might be clearer to pass snapshot instead of field to shouldAbort:

public static bool TrySpeculativeUpdate(Func<int, int> update, Func<int, bool> shouldAbort, ref int field, out int result)
{
    var spinWait = new SpinWait();
    while (true)
    {
        var snapshot = field;

        if (shouldAbort(snapshot))
        {
            result = 0;
            return false;
        }
        else
        {
            var calc = update(snapshot);
            if (Interlocked.CompareExchange(ref field, calc, snapshot) == snapshot)
            {
                result = calc;
                return true;
            }
        }

        spinWait.SpinOnce();
    }
}

That's all I could see. The naming looks about right, but I don't write enough multithreaded code to know for a fact whether this code is optimal or not - it does look good though, I don't see how nesting could be reduced, other than by extacting a tiny little specialized function responsible for this part:

if (Interlocked.CompareExchange(ref field, calc, snapshot) == snapshot)
{
    result = calc;
    return true;
}

But that clearly would be overkill IMO.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.