Комментарии 34
У вас лажа с производительностью в программе — критичные задачи упираются в блокировки. Один и тот же менеджер обрабатывает одновременно критичные и не критичные задачи, при этом обрабатывает весьма плохо (задержки по несколько секунд). У вас прямо просятся очереди разного приоритета, каждая работающая в отдельном потоке, а может и на выделенном процессорном ядре. Но нет, вы делаете цикл с SpinOnce, нагружающий на 100% все остальные ядра, ждущие ответа.
Поставленную задачу этот код, наверное, выполняет — пропускать через блокировку в разном порядке. Но в ваших интересах сделать, чтобы этой задачи не было.
Тем не менее я уверен каждый найдет для себя сценарии использования такого примитива, я например много где его применяю.
По поводу производительности, я раньше тоже думал что SpinOnce это нечто злое, но как показала практика, SpinOnce не нагружает процессор никак, при этом дает процессорное время другим потокам в нормальном порядке. К тому же в комбинации с семафорами, SpinOnce не вызывается во всех ожидающих потоках.
И все-таки, у вас SpinOnce используются неправильно.
Во-первых, SpinWait полагается создавать на операцию, а не держать полем в классе.
Во-вторых, спин-локи должны ждать быстрое событие, а не пока закончится операция неопределенной длительности.
По поводу второго, согласно статье SpinWait настолько умен, что может работать как в быстром, так и в долгом ожидании. Но понятно, что чем короче операции внутри лока, тем лучше.
SpinWait предназначен для того чтобы создавать его на операцию. Его внутренняя логика писалась исходя из того, что он будет создаваться на операцию. Когда вы держите его в классе — библиотечный код "думает" что в процессе работы программы вы выполняете одну и ту же неблокирующую операцию, которая никак не может выполниться успешно, и принимает решения исходя из этого.
Применять его с "долгими" ожиданиями, действительно, документация разрешает — но говорит при этом, что вы должны проверить свойство NextSpinWillYield, и заблокировать поток самостоятельно вместо вызова SpinOnce. А вовсе не вызывать SpinOnce для "блокировки" потока.
А вот про создание SpinWait внутри метода соглашусь, изучил исходники, где применяется SpinWait, действительно стоит делать так как вы сказали.
А без двухэтапного ожидания мы возвращаемся к тому, что спин-локи нельзя применять для "долгих" ожиданий.
Хочу сказать, что в своей практике я не применяю PriorityLock для долгих ожиданий и не получал просадки производительности из-за этого.
Если у вас есть желание, пожалуйста приведите пример кода, который будет правильнее чем этот:
var spin = new SpinWait();
while (Interlocked.CompareExchange(ref mgr.LowCount, 0, 0) != 0)
spin.SpinOnce();
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
теперь хранит признак необходимости разблокировать семафор.
PS свежим взглядом обнаружил ошибку: условие lowCount < 0
тут лишнее, оно ни от чего не защищает; если в этот цикл попадет сразу два потока — ошибка будет неизбежна. Для вашей программы это не страшно, у вас там сверху ожидание на семафоре, но условие все равно лишнее.
Ну нет, так делать нельзя, у вас теперь код ошибочный (а был просто неоптимальным).
Я не случайно использовал одну и ту же переменную для хранения двух разных признаков...
ReadWriteLock
? И меня одного смущает async lock?А что вас смущает в async lock?
Меня смущает то, что async await сделаны, чтобы избегать блокировок и экономить на этом потоки, а они используются для блокировки -.-
Меня напрягает сам
SemaphoreSlim.WaitAsync
, который внутри себя явно вызывает lock, и лично мне наличие этого метода кажется весьма неоднозначным. Плюсом будет только то, что извне можно красивенько это принимать, но выдавать lock за Async.Сокращённый отрывок исходного кода из исходников с 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
// Dummy object used to in lock statements to protect the semaphore count, wait handle and cancelation
и на нём не висит синхронно цпу, пока ваш код ждёт на WaitAsync.
Не, если вы сможете какой-нибудь дедлок с ним создать, будет наверное печально, но в остальном не вижу проблем.
Асинхронный метод по определению неблокирующий, а в нём есть блокировка и это меня как-то напрягает. Я не хочу сказать, что «всё
Самый очевидный сценарий проблемного поведения: Мы начинаем долбить этот семафор многими потоками с огромной частотой, что приведёт нас к lock concoy, хотя этого мы никак не могли ожидать от асинхронного метода.
В исходниках SemaphoreSlim.WaitAsync использование lock находится в синхронной части кода, внутри lock там все детерминированно и максимально быстро выйдет из под него.
От себя лишь добавлю, что async/await — это механизм неблокирующих операций, но никто не отменяет необходимость критических секций в асинхронном коде. И использование синхронных методов синхронизации в асинхронном коде может плохо кончится, потому что Task != Thread, и зайдя в критическую секцию, таска может отпустить поток, а другая таска похватит поток и повторно зайдет в критическую секцию (в контексте использования PriorityLock, другие примитивы не проверял). Поэтому необходимы асинхронные методы синхронизации.
Но на NeoSmart.AsyncLock наткнулся при поиске переиспользования асинхронного лока. Ну и не мог не написать, что оно вообще не работает как lock.
На самом деле даже используя обычный lock это не было узким местом. PriorityLock появился для решения проблем с UI. Архитектурно задачу я решил позже, вынес UI в отдельный процесс, потому что понял что основные фризы интерфейса создавал GC.
Lock с приоритетами в .NET