2

We have some performance issues and are thinking about to take out some thread safe locks from some heavily used properties. More precisely only from the access modifier getter. The improvement would be that the setter access modifier is not "blocked" anymore if some other thread is making a get on the same property.

-> Of course it has to be ensured that if let's say for an integer type for example the bit value 11110011 which is 243, all bits are written once writing has started. It has to be ensured that never the write thread is unfinished and the get thread becomes some half written bits which results a wrong value. Is it like that?

If so, is that concept usable for all .net built in data types, also string?

See following code example which shows the concept:

    // for properties used just the "Built-In Types"
    // doc: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/built-in-types-table

    private int _ActualValue = 0;
    private readonly object _Lock_ActualValue = new object();
    public int ActualValue
    {
        get
        {
            //lock(_Lock_ActualValue)  <- remove lock for access modifier get ?
            //{
                return _ActualValue;
            //}
        }
        set
        {
            lock (_Lock_ActualValue)
            {
                if((value != _ActualValue) && (value < 1000))
                {
                    Log("_ActualValue", value.ToString());
                    _ActualValue = value;
                }
            }
        }
    }
  • 2
    Why are you using `lock` at all? Why not use `Interlocked` methods? https://stackoverflow.com/questions/24808291/reading-an-int-thats-updated-by-interlocked-on-other-threads – mjwills Aug 15 '18 at 10:55
  • as I know, writing an int in memory is an atomic operation, so you never can get in situation of reading not complitely writen data – vasily.sib Aug 15 '18 at 10:55
  • 4
    Aligned read/write of integers and references are atomic in .net, so yes you won't get corrupted data. That said, this lock is very unlikely to be a performance issue, unless it gets *very* contended. I suggest you run a profiler first on your application to find out what is the bottleneck – Kevin Gosse Aug 15 '18 at 10:56
  • https://learn.microsoft.com/en-us/dotnet/standard/threading/reader-writer-locks you can try – Vibeeshan Mahadeva Aug 15 '18 at 10:57
  • Possible duplicate of [What operations are atomic in C#?](https://stackoverflow.com/questions/11745440/what-operations-are-atomic-in-c) – mjwills Aug 15 '18 at 10:57
  • 4
    Also, if you do use a `lock` I'd suggest moving the logging outside of the `lock`. It may mean some of the logging is out of order, but perf will be improved due to reduced contention. – mjwills Aug 15 '18 at 11:05
  • 1
    What brought you to the conclusion that contention on these locks is your perf issue @spaghettiBox? – mjwills Aug 15 '18 at 11:07
  • 1
    Agree with Kevin Gosse; have you profiled to find out for certain where the bottlenecks are? It seems unusual to have many threads all waiting to read the same property at once, for any appreciable amount of time (enough to cause problems). – BittermanAndy Aug 15 '18 at 11:09
  • Hi back from lunch :) @Kevin Gosse answer, there are tons of such locks in our application, and we have some fast cyclic and lot of tasks running throug them. Please interprete my question as an theoretical clarification, the Log() method is just an illustration there could be made something. When it is really atomic, so this concept could work in my understanding, no corrupt data. What about bigger datatypes, string is it also atomic ? -> all of the build-in types : https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/built-in-types-table – spaghettiBox Aug 15 '18 at 11:32

2 Answers2

3

As a rule of thumb, never put optimization over thread safety. This is considered widely as a bad practice and a direct result of a lot of issues. That being said, no it will never half write a value into an atomic object such as your int ActualValue. Your approach is actually fine if you don't care about full accuracy of the value when you are getting.

EDIT

As a whole, anything that is a multi-step operation in machine code is not atomic. To clarify my answer above, in java the bool, char, byte, sbyte, short, ushort, int, uint, and float types all have atomic writes, thus will not be half written. However, the decimal, double, long, ulong, and DateTime types do not have an atomic write, thus could be partially written.

Impurity
  • 1,037
  • 2
  • 16
  • 31
  • I fully agree with you. I don't wan't to priorize thread safety vs. performance of course, I would like to build efficient code without unnecessary stuff. – spaghettiBox Aug 15 '18 at 12:03
  • Found here also a good discussion with some interesting informations: https://stackoverflow.com/questions/11745440/what-operations-are-atomic-in-c – spaghettiBox Aug 15 '18 at 12:05
  • Thank you for your feedback and the discussion link. This gives great insight into your specific question of write-safety. – Impurity Aug 15 '18 at 12:07
  • In summary, it can be said that it depends on the data type if my concept is safe or not. "The CLI guarantees that reads and writes of variables of value types that are the size (or smaller) of the processor's natural pointer size are atomic", so just in these case my concept works. For all other types a getter lock is required i would say. – spaghettiBox Aug 15 '18 at 12:09
  • @Impurity - I think it would be responsible to specifically indicate in your answer that `bool`, `char`, `byte`, `sbyte`, `short`, `ushort`, `int`, `uint`, `float` are atomic writes, but `decimal`, `double`, `long`, `ulong`, `DateTime` are not. – Enigmativity Aug 17 '18 at 03:27
0
private ReaderWriterLockSlim lockObj = new ReaderWriterLockSlim();

private int _ActualValue = 0;
public int ActualValue
{
    get
    {
        lockObj.EnterReadLock();

        try
        {
            return _ActualValue;
        }
        finally
        {
            lockObj.ExitReadLock();
        }
    }
    set
    {
        lockObj.EnterWriteLock();
        try
        {

            if((value != _ActualValue) && (value < 1000))
            {
                Log("_ActualValue", value.ToString());
                _ActualValue = value;
            }
        }
        finally
        {
            lockObj.ExitWriteLock();
        }
    }
}
Vibeeshan Mahadeva
  • 7,147
  • 8
  • 52
  • 102
  • 1
    Just ran a bench, a non-contended read with a `ReaderWriterLockSlim` is 2.5x slower than a non-contended read with a `lock`. This is a good example of premature optimization – Kevin Gosse Aug 15 '18 at 11:05