2

I have class Foo that stores a string. The Bar property is expected to be accessed/assigned to by multiple threads and reads must access the latest value so I put a lock in place:

public class Foo
{
    private readonly object _lock = new object();
    private string _bar;

    public string Bar
    {
        get
        {
            lock (_lock)
            {
                return _bar;
            }
        }

        set
        {
            lock (_lock)
            {
                _bar = value;
            }
        }
    }
}

I am wondering will using Interlocked.Exchange will achieve the same result but potentially better performance?

public class Foo
{
    private string _bar;

    public string Bar
    {
        get => _bar;
        set => Interlocked.Exchange(ref _bar, value);
    }
}
rexcfnghk
  • 14,435
  • 1
  • 30
  • 57
  • 1
    This should be established by measurement. If this isn't a hot path, it doesn't even really matter. My suspicion is that `CompareExchange` will win out. – spender Oct 11 '17 at 10:40
  • Yes, it will be faster. However it might have a different result, as it will ignore the values which "loose" the run: https://social.msdn.microsoft.com/Forums/vstudio/en-US/e68f94c1-5677-45ee-9511-7a49a892323f/interlockedcompareexchangeint32-int32-int32?forum=clr – Marco Oct 11 '17 at 10:41
  • That really depends on how you're using that property. In vacuum there's nothing wrong with a single property being accessed by multiple threads, it's only if you need to start worrying about e.g. having the most recent value, value not changing between read and write, etc. you need to implement some kind of locking/synchronization. Not knowing your needs it's hard to answer that question. – decPL Oct 11 '17 at 10:41
  • 1
    `Interlocked.CompareExchange` requires three arguments, but your example only has two. https://msdn.microsoft.com/en-us/library/bb297966(v=vs.110).aspx – Thariq Nugrohotomo Oct 11 '17 at 10:43
  • None of this will protect you against concurrent mutations of an existing object accessed via the `Bar` property. It's at about this time that I'd start considering the benefits of immutability. – spender Oct 11 '17 at 10:43
  • @decPL I have updated my question to reflect that I need the most updated value from reads – rexcfnghk Oct 11 '17 at 10:45
  • @Thariq I have fixed that typo – rexcfnghk Oct 11 '17 at 10:46
  • Ah. Didn't read that `Bar` is a string. Mutation, as per your question, would not be possible. – spender Oct 11 '17 at 10:47
  • 1
    @rexcfnghk I'm not entirely sure the `Interlocked.CompareExchange` version helps you here - it synchronizes write access, not read. On the other hand, IIRC the `volatile` keyword should be enough to satisfy your need: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/volatile – decPL Oct 11 '17 at 10:48
  • Not sure why you'd choose `CompareExchange` over `Exchange`. All you've introduced here is a race condition and a (potentially silently failing) missed update if two threads happen to be competing to `set`. – Damien_The_Unbeliever Oct 11 '17 at 10:50
  • 2
    ...trying to convince myself that this isn't a dupe of https://stackoverflow.com/questions/154551/volatile-vs-interlocked-vs-lock , https://stackoverflow.com/questions/20800954/is-interlocked-compareexchange-really-faster-than-a-simple-lock etc... – spender Oct 11 '17 at 10:50
  • @Damien oh I didn't realise there is a simpler API. Will update – rexcfnghk Oct 11 '17 at 11:28
  • 1
    No, no amount of throwing in `Interlocked` will get you "the latest value" with unsynchronized reads. The main problem is that people usually don't realize what they're asking for when they write "the latest value" without thinking about what that really means, on the processor level. The moral is not to attempt to optimize `lock` until you've shown that you have an actual bottleneck to optimize. (Also, putting the `lock` in the property precludes safe use of setting/getting multiple properties at once; consider letting clients do the locking on the whole object if that could become an issue.) – Jeroen Mostert Oct 11 '17 at 11:42
  • 2
    Both are fundamentally wrong. The only side-effect they have is the memory barrier they provide. But it is the expensive one, a full one, this needs VolatileRead() in the getter and VolatileWrite() in the setter to have any odds of improvement. Then you get to fret over correctness, already very weak in the first version since fine-grained locking almost never works. In practice you always care about a consistent view of the entire object, not just one property. It can only be proved correct by testing on a processor with a weak memory model. Today only ARM is left. – Hans Passant Oct 11 '17 at 11:58
  • Possible duplicate of [C# thread safety with get/set](https://stackoverflow.com/questions/505515/c-sharp-thread-safety-with-get-set) – CSharpie Oct 11 '17 at 15:22
  • @HansPassant do you mind explaining a bit further? I don’t quite get what you mean by ‘both are fundamentally wrong’. – rexcfnghk Oct 12 '17 at 01:21
  • Possible duplicate of [Volatile vs. Interlocked vs. lock](https://stackoverflow.com/questions/154551/volatile-vs-interlocked-vs-lock) – qxg Oct 16 '17 at 03:22

0 Answers0