29.06.2019 11:39
gigavat38 30 5200 Источник
Каждый программист, использующий более одного потока в своей программе, сталкивался с примитивами синхронизации. В контексте .NET их очень и очень много, перечислять не буду, за меня это уже сделал MSDN.

Мне приходилось пользоваться многими из этих примитивов, и они прекрасно помогали справиться с задачами. Но в этой статье я хочу рассказать про обычный lock в десктопном приложении и о том как же появился новый (по крайней мере для меня) примитив, который можно назвать PriorityLock.

Проблема


При разработке высоконагруженного многопоточного приложения где то появляется менеджер, который обрабатывает бесчисленное множество потоков. Так было и у меня. И вот работал этот менеджер, обрабатывал тонны запросов от многих сотен потоков. И все у него было хорошо, а внутри трудился обычный lock.

И вот однажды пользователь (например я) нажимает кнопку в интерфейсе приложения, поток летит в менеджер (не UI поток конечно) и ожидает увидеть супер приветливый ресепшен, а вместо этого его встречает тетя Клава из самой дремучей регистратуры самой дремучей поликлиники со словами «Мне плевать кто тебя направил. У меня еще 950 таких как ты. Иди и втавай к ним. Мне всё равно как вы там разберетесь». Примерно так работает lock в .NET. И вроде все хорошо, все выполнится корректно, но пользователь явно не планировал ждать несколько секунд ответа на своё действие.

На этом душещипательная история заканчивается и начинается решение технической проблемы.

Решение


Изучив стандартные примитивы, я не нашел подходящего варианта. Поэтому решил написать свой lock, который бы имел стандартный и высокий приоритет входа. Кстати после написания я изучил и nuget, там тоже ничего подобного не нашел, хотя возможно плохо искал.

Для написания такого примитива (или уже не примитива) мне потребовались SemaphoreSlim, SpinWait и Interlocked операции. В спойлере я привел первый вариант моего PriorityLock (только синхронный код, но он и есть самый важный), и пояснения к нему.

Скрытый текст
В плане синхронизации нету никаких открытий, пока кто-то в локе, другие не могут зайти. Если пришел high priority, его пускают вперед всех ожидающих low priority.

Класс LockMgr, с ним предлагается работать в вашем коде. Именно он является тем самым объектом синхронизации. Создает объекты Locker и HighLocker, содержит в себе семафоры, SpinWait'ы, счетчики желающих попасть в критическую секцию, текущий поток и счетчик рекурсии.

public class LockMgr
{
    internal int HighCount;
    internal int LowCount;
    internal Thread CurThread;
    internal int RecursionCount;

    internal readonly SemaphoreSlim Low = new SemaphoreSlim(1);
    internal readonly SemaphoreSlim High = new SemaphoreSlim(1);
    internal SpinWait LowSpin = new SpinWait();
    internal SpinWait HighSpin = new SpinWait();

    public Locker HighLock()
    {
        return new HighLocker(this);
    }
    public Locker Lock(bool high = false)
    {
        return new Locker(this, high);
    }
}

Класс Locker реализует интерфейс IDisposable. Для реализации рекурсии при завладении локом запоминаем Id потока, после проверяем его. Далее в зависимости от приоритета, в случае высокого приоритета сразу говорим что мы пришли (увеличиваем счетчик HighCount), получаем семафор High, и ждём (если нужно) освобождения лока от низкого приорита, после мы готовы получить лок. В случае низкого приорита получает семафор Low, далее ждем завершения всех high приоритетных потоков, и, забирая на время семафор High увеличиваем LowCount.

Стоит оговориться, что смысл HighCount и LowCount разный, HighCount отображает количество приоритетных потоков, которые пришли к локу, когда LowCount всего лишь означает что поток (один единственный) с низким приоритетом зашел в лок.

public class Locker : IDisposable
{
    private readonly bool _isHigh;
    private LockMgr _mgr;

    public Locker(LockMgr mgr, bool isHigh = false)
    {
        _isHigh = isHigh;
        _mgr = mgr;
        if (mgr.CurThread == Thread.CurrentThread)
        {
            mgr.RecursionCount++;
            return;
        }
        if (_isHigh)
        {
            Interlocked.Increment(ref mgr.HighCount);
            mgr.High.Wait();
            while (Interlocked.CompareExchange(ref mgr.LowCount, 0, 0) != 0)
                mgr.HighSpin.SpinOnce();
        }
        else
        {
            mgr.Low.Wait();
            while (Interlocked.CompareExchange(ref mgr.HighCount, 0, 0) != 0)
                mgr.LowSpin.SpinOnce();
            try
            {
                mgr.High.Wait();
                Interlocked.Increment(ref mgr.LowCount);
            }
            finally
            {
                mgr.High.Release();
            }
        }
        mgr.CurThread = Thread.CurrentThread;
    }

    public void Dispose()
    {
        if (_mgr.RecursionCount > 0)
        {
            _mgr.RecursionCount--;
            _mgr = null;
            return;
        }
        _mgr.RecursionCount = 0;
        _mgr.CurThread = null;
        if (_isHigh)
        {
            _mgr.High.Release();
            Interlocked.Decrement(ref _mgr.HighCount);
        }
        else
        {
            _mgr.Low.Release();
            Interlocked.Decrement(ref _mgr.LowCount);
        }
        _mgr = null;
    }
}

public class HighLocker : Locker
{
    public HighLocker(LockMgr mgr) : base(mgr, true)
    { }
}


Использование объекта класса LockMgr получилось очень лаконичным. В примере явно показана возможность переиспользования _lockMgr внутри критической секции, при этом приоритет уже не важен.

private PriorityLock.LockMgr _lockMgr = new PriorityLock.LockMgr();

public void LowPriority()
{
  using (_lockMgr.Lock())
  {
    using (_lockMgr.HighLock())
    {
      // your code
    }
  }
}

public void HighPriority()
{
  using (_lockMgr.HighLock())
  {
    using (_lockMgr.Lock())
    {
      // your code
    }
  }
}

Таким образом я решил свою задачу. Обработка пользовательских действий стала выполнятся с высоким приоритетом, никто не пострадал, все выиграли.

Асинхронность


Так как объекты класса SemaphoreSlim поддерживают асинхронное ожидание, я так же добавил себе эту возможность. Код отличается минимально и в конце статьи я приведу ссылку на исходный код.

Здесь важно отметить то, что Task не привязан к потоку никаким образом, поэтому нельзя аналогичным образом реализовать асинхронное переиспользование лока. Более того, свойство Task.CurrentId по описанию MSDN не гарантирует ничего. На этом мои варианты закончились.

В поисках решения я наткнулся на проект NeoSmart.AsyncLock, в описании которого была указана поддержка переиспользования асинхронного лока. Технически переиспользование работает. Но к сожалению сам лок не является локом. Будте осторожны, если используете этот пакет, знайте, он работает НЕ правильно!

Заключение


В итоге получился класс поддерживающий синхронные операции с переиспользованием, и асинхронные операции без переиспользования. Асинхронные и синхронные операции можно использовать рядом, но нельзя использовать вместе! Все из-за отсутствия поддержки переиспользования асинхронным вариантом.

Надеюсь я не одинок в таких проблемах и моё решение кому то пригодится. Библиотеку я выложил на github и в nuget.

В репозитории есть тесты, показывающие работоспособность PriorityLock. На асинхронной части этого теста проверялся NeoSmart.AsyncLock, и тест он не прошел.

Ссылка на nuget
Ссылка на github

Комментарии (30)


  1. Nomad1
    29.06.2019 15:34
    #20339826
    +2

    Я позволю себе покритиковать.
    У вас лажа с производительностью в программе — критичные задачи упираются в блокировки. Один и тот же менеджер обрабатывает одновременно критичные и не критичные задачи, при этом обрабатывает весьма плохо (задержки по несколько секунд). У вас прямо просятся очереди разного приоритета, каждая работающая в отдельном потоке, а может и на выделенном процессорном ядре. Но нет, вы делаете цикл с SpinOnce, нагружающий на 100% все остальные ядра, ждущие ответа.
    Поставленную задачу этот код, наверное, выполняет — пропускать через блокировку в разном порядке. Но в ваших интересах сделать, чтобы этой задачи не было.


    1. gigavat38 Автор
      29.06.2019 16:25
      #20339964

      Спасибо за критику. Да, наверно пример не удачный. Задержки по несколько секунд исчезли после написания PriorityLock, но все равно вы правы, приоритетные очереди уместнее.
      Тем не менее я уверен каждый найдет для себя сценарии использования такого примитива, я например много где его применяю.

      По поводу производительности, я раньше тоже думал что SpinOnce это нечто злое, но как показала практика, SpinOnce не нагружает процессор никак, при этом дает процессорное время другим потокам в нормальном порядке. К тому же в комбинации с семафорами, SpinOnce не вызывается во всех ожидающих потоках.


      1. mayorovp
        01.07.2019 13:47
        #20344804

        И все-таки, у вас SpinOnce используются неправильно.


        Во-первых, SpinWait полагается создавать на операцию, а не держать полем в классе.


        Во-вторых, спин-локи должны ждать быстрое событие, а не пока закончится операция неопределенной длительности.


        1. gigavat38 Автор
          01.07.2019 13:59
          #20344864

          Прокомментируйте пожалуйста чем плохо держать SpinWait полем в классе? Сам не могу предложить аргументов против.

          По поводу второго, согласно статье SpinWait настолько умен, что может работать как в быстром, так и в долгом ожидании. Но понятно, что чем короче операции внутри лока, тем лучше.


          1. mayorovp
            01.07.2019 14:37
            #20345054
            +1

            SpinWait предназначен для того чтобы создавать его на операцию. Его внутренняя логика писалась исходя из того, что он будет создаваться на операцию. Когда вы держите его в классе — библиотечный код "думает" что в процессе работы программы вы выполняете одну и ту же неблокирующую операцию, которая никак не может выполниться успешно, и принимает решения исходя из этого.


            Применять его с "долгими" ожиданиями, действительно, документация разрешает — но говорит при этом, что вы должны проверить свойство NextSpinWillYield, и заблокировать поток самостоятельно вместо вызова SpinOnce. А вовсе не вызывать SpinOnce для "блокировки" потока.


            1. gigavat38 Автор
              01.07.2019 14:49
              #20345114

              По поводу NextSpinWillYield не соглашусь, написано использовать это для двухэтапной операции ожидания. У меня контекст использования SpinWait ограничен только ожиданием счетчика.

              А вот про создание SpinWait внутри метода соглашусь, изучил исходники, где применяется SpinWait, действительно стоит делать так как вы сказали.


              1. mayorovp
                01.07.2019 14:51
                #20345124

                А без двухэтапного ожидания мы возвращаемся к тому, что спин-локи нельзя применять для "долгих" ожиданий.


                1. gigavat38 Автор
                  01.07.2019 15:03
                  #20345194

                  Наверно без экспериментов не обойтись…
                  Хочу сказать, что в своей практике я не применяю PriorityLock для долгих ожиданий и не получал просадки производительности из-за этого.

                  Если у вас есть желание, пожалуйста приведите пример кода, который будет правильнее чем этот:

                  var spin = new SpinWait();
while (Interlocked.CompareExchange(ref mgr.LowCount, 0, 0) != 0)
    spin.SpinOnce();


                  1. mayorovp
                    01.07.2019 15:10
                    #20345224
                    +1

                    var spin = new SpinWait();
int lowCount;
while ((lowCount = Volatile.Read(ref mgr.LowCount)) != 0) {
    if (spin.NextSpinWillYield && (lowCount < 0 || Interlocked.CompareExchange(ref mgr.LowCount, -lowCount, lowCount) == lowCount)) {
        someSemaphore.Wait();
    } else {
        spin.SpinOnce();
    }
}

                    Как-то так. Разумеется, остальной код надо так же подправить с учетом того, что знак mgr.LowCount теперь хранит признак необходимости разблокировать семафор.


                    1. gigavat38 Автор
                      01.07.2019 15:12
                      #20345238

                      Спасибо. В ближайшее время постараюсь применить ваши советы!


  1. KamushekORIGINAL
    29.06.2019 16:26
    #20339966

    Но зачем, если есть ReadWriteLock? И меня одного смущает async lock?


    1. gigavat38 Автор
      29.06.2019 16:28
      #20339968

      В ReadWriteLock сразу несколько читателей могут зайти внутрь критической секции. PriorityLock позволяет только одному потоку заходить в критическую секцию как обычный lock, но имеет два приоритета примерно как в ReadWriteLock.

      А что вас смущает в async lock?


      1. KamushekORIGINAL
        29.06.2019 17:00
        #20340072

        Да, сразу не обратил внимания на это.
        Меня смущает то, что async await сделаны, чтобы избегать блокировок и экономить на этом потоки, а они используются для блокировки -.-
        Меня напрягает сам SemaphoreSlim.WaitAsync, который внутри себя явно вызывает lock, и лично мне наличие этого метода кажется весьма неоднозначным. Плюсом будет только то, что извне можно красивенько это принимать, но выдавать lock за Async.


        1. MonkAlex
          29.06.2019 17:36
          #20340208

          SemaphoreSlim.WaitAsync не занимает поток, но не дает зайти и выполнить код под блокировкой. Всё по плану, всё хорошо.


          1. KamushekORIGINAL
            29.06.2019 19:53
            #20340518

            Я про то, что внутри async метода содержится lock, а никто не ожидает от async того, что он залочится и не продолжит выполнение. Ну и как и сказал gigavat38


            1. MonkAlex
              29.06.2019 20:10
              #20340556
              +1

              Хм, тогда я не понимаю, о чём вы пишете. SemaphoreSlim.WaitAsync — полноценный async-await метод, дает и блокировку и работает асинхронно. Никакого lock внутри нет. Никакой блокировки треда нет.


              1. KamushekORIGINAL
                29.06.2019 20:26
                #20340586

                В реализации Microsoft есть конструкция lock, про которую я и говорил с:
                Сокращённый отрывок исходного кода из исходников с github:

                Код
                Не получилось у меня отформатировать =(
                Там Task, возвразающий bool, но хабр проглатывает скобочки .-.

                public Task WaitAsync(int millisecondsTimeout, CancellationToken cancellationToken)
                {//1
                ...
                lock (m_lockObj)
                {//2
                if (m_currentCount > 0)
                {//3
                ...
                if (...) m_waitHandle.Reset();
                return s_trueTask;
                }//3
                else
                {//4
                ...
                var asyncWaiter = CreateAndAddAsyncWaiter();
                return (...) ?
                asyncWaiter :
                WaitUntilCountOrTimeoutAsync(...);
                }//4
                }//2
                }//1


                1. MonkAlex
                  29.06.2019 20:34
                  #20340620
                  +1

                  Так он там только для внутренних целей, так и написано же:
                  // Dummy object used to in lock statements to protect the semaphore count, wait handle and cancelation
                  и на нём не висит синхронно цпу, пока ваш код ждёт на WaitAsync.

                  Не, если вы сможете какой-нибудь дедлок с ним создать, будет наверное печально, но в остальном не вижу проблем.


                  1. KamushekORIGINAL
                    29.06.2019 20:48
                    #20340646

                    В обычном использовании он не доставит проблем, но lock есть lock.
                    Асинхронный метод по определению неблокирующий, а в нём есть блокировка и это меня как-то напрягает. Я не хочу сказать, что «всё х***я, давай по новой», но это может поставить в довольно неловкое положение пользователя этого метода.

                    Самый очевидный сценарий проблемного поведения: Мы начинаем долбить этот семафор многими потоками с огромной частотой, что приведёт нас к lock concoy, хотя этого мы никак не могли ожидать от асинхронного метода.

                    Скрытый текст
                    Если не ошибаюсь, то про что-то подобное рассказывал Евгений Пешков на DotNext Piter 2019


                    1. gigavat38 Автор
                      29.06.2019 21:18
                      #20340724

                      Я немного о другом случае говорил: если внутри синхронного лока написать await — это гарантированнная проблема. Поэтому компилятор не дает внутри обычного lock писать await.
                      В исходниках SemaphoreSlim.WaitAsync использование lock находится в синхронной части кода, внутри lock там все детерминированно и максимально быстро выйдет из под него.


                      1. KamushekORIGINAL
                        29.06.2019 21:20
                        #20340734

                        Внутри лока да, но только если мы не залочимся на самом lock. Вот я про что


                        1. AgentFire
                          30.06.2019 11:47
                          #20341566

                          а зачем нам лочиться на приватном объекте чужого класса?


                          1. gigavat38 Автор
                            30.06.2019 12:29
                            #20341640

                            Как я понимаю KamushekORIGINAL говорит об ошибке платформы, получении lock convoy. Но думаю не стоит записывать это на нашу совесть. А если такое произошло, видимо стоит менять архитектуру на неблокирющие операции.


                            1. KamushekORIGINAL
                              30.06.2019 15:37
                              #20341948

                              Именно :3


        1. gigavat38 Автор
          29.06.2019 18:34
          #20340362

          MonkAlex все правильно написал.
          От себя лишь добавлю, что async/await — это механизм неблокирующих операций, но никто не отменяет необходимость критических секций в асинхронном коде. И использование синхронных методов синхронизации в асинхронном коде может плохо кончится, потому что Task != Thread, и зайдя в критическую секцию, таска может отпустить поток, а другая таска похватит поток и повторно зайдет в критическую секцию (в контексте использования PriorityLock, другие примитивы не проверял). Поэтому необходимы асинхронные методы синхронизации.


  1. Vadem
    29.06.2019 21:13
    #20340714
    +2

    В поисках решения я наткнулся на проект NeoSmart.AsyncLock

    Зачем его вообще использовать?
    Есть же прекрасный AsyncEx, который уже давно стандарт де-факто для таких вещей.


    1. gigavat38 Автор
      29.06.2019 21:20
      #20340728

      Солгасен с вами, я тоже использую AsyncEx.
      Но на NeoSmart.AsyncLock наткнулся при поиске переиспользования асинхронного лока. Ну и не мог не написать, что оно вообще не работает как lock.


      1. Vadem
        30.06.2019 00:50
        #20341054

        Понял. Да. Вы правы. NeoSmart.AsyncLock выдаётся первым по запросу async lock С#, а значит многие могут на нём и остановиться при поиске решения.


  1. megasuperlexa
    30.06.2019 10:28
    #20341416

    Можно поинтересоваться, в какой области вы работаете? Как получилось, что у вас столько шереного стейта, к которому необходимо синхронизировать доступ и даже делать собственные механизмы синхронизации? Не пытались ли решить проблему архитектурными методами.


    1. gigavat38 Автор
      30.06.2019 12:15
      #20341612

      Приложение из примера автоматизирует пользовательские шаблоны, оно манипулирует большим количеством потоков и большим количеством тяжелых процессов (ограничено лишь ресурсами компьютера). Менеджер из примера обрабатывает задания, при этом должен держать несколько коллекций в синхронизированном состоянии.

      На самом деле даже используя обычный lock это не было узким местом. PriorityLock появился для решения проблем с UI. Архитектурно задачу я решил позже, вынес UI в отдельный процесс, потому что понял что основные фризы интерфейса создавал GC.