Comments 34
Я позволю себе покритиковать.
У вас лажа с производительностью в программе — критичные задачи упираются в блокировки. Один и тот же менеджер обрабатывает одновременно критичные и не критичные задачи, при этом обрабатывает весьма плохо (задержки по несколько секунд). У вас прямо просятся очереди разного приоритета, каждая работающая в отдельном потоке, а может и на выделенном процессорном ядре. Но нет, вы делаете цикл с SpinOnce, нагружающий на 100% все остальные ядра, ждущие ответа.
Поставленную задачу этот код, наверное, выполняет — пропускать через блокировку в разном порядке. Но в ваших интересах сделать, чтобы этой задачи не было.
У вас лажа с производительностью в программе — критичные задачи упираются в блокировки. Один и тот же менеджер обрабатывает одновременно критичные и не критичные задачи, при этом обрабатывает весьма плохо (задержки по несколько секунд). У вас прямо просятся очереди разного приоритета, каждая работающая в отдельном потоке, а может и на выделенном процессорном ядре. Но нет, вы делаете цикл с SpinOnce, нагружающий на 100% все остальные ядра, ждущие ответа.
Поставленную задачу этот код, наверное, выполняет — пропускать через блокировку в разном порядке. Но в ваших интересах сделать, чтобы этой задачи не было.
Спасибо за критику. Да, наверно пример не удачный. Задержки по несколько секунд исчезли после написания PriorityLock, но все равно вы правы, приоритетные очереди уместнее.
Тем не менее я уверен каждый найдет для себя сценарии использования такого примитива, я например много где его применяю.
По поводу производительности, я раньше тоже думал что SpinOnce это нечто злое, но как показала практика, SpinOnce не нагружает процессор никак, при этом дает процессорное время другим потокам в нормальном порядке. К тому же в комбинации с семафорами, SpinOnce не вызывается во всех ожидающих потоках.
Тем не менее я уверен каждый найдет для себя сценарии использования такого примитива, я например много где его применяю.
По поводу производительности, я раньше тоже думал что SpinOnce это нечто злое, но как показала практика, SpinOnce не нагружает процессор никак, при этом дает процессорное время другим потокам в нормальном порядке. К тому же в комбинации с семафорами, SpinOnce не вызывается во всех ожидающих потоках.
И все-таки, у вас SpinOnce используются неправильно.
Во-первых, SpinWait полагается создавать на операцию, а не держать полем в классе.
Во-вторых, спин-локи должны ждать быстрое событие, а не пока закончится операция неопределенной длительности.
Прокомментируйте пожалуйста чем плохо держать SpinWait полем в классе? Сам не могу предложить аргументов против.
По поводу второго, согласно статье SpinWait настолько умен, что может работать как в быстром, так и в долгом ожидании. Но понятно, что чем короче операции внутри лока, тем лучше.
По поводу второго, согласно статье SpinWait настолько умен, что может работать как в быстром, так и в долгом ожидании. Но понятно, что чем короче операции внутри лока, тем лучше.
SpinWait предназначен для того чтобы создавать его на операцию. Его внутренняя логика писалась исходя из того, что он будет создаваться на операцию. Когда вы держите его в классе — библиотечный код "думает" что в процессе работы программы вы выполняете одну и ту же неблокирующую операцию, которая никак не может выполниться успешно, и принимает решения исходя из этого.
Применять его с "долгими" ожиданиями, действительно, документация разрешает — но говорит при этом, что вы должны проверить свойство NextSpinWillYield, и заблокировать поток самостоятельно вместо вызова SpinOnce. А вовсе не вызывать SpinOnce для "блокировки" потока.
По поводу NextSpinWillYield не соглашусь, написано использовать это для двухэтапной операции ожидания. У меня контекст использования SpinWait ограничен только ожиданием счетчика.
А вот про создание SpinWait внутри метода соглашусь, изучил исходники, где применяется SpinWait, действительно стоит делать так как вы сказали.
А вот про создание SpinWait внутри метода соглашусь, изучил исходники, где применяется SpinWait, действительно стоит делать так как вы сказали.
А без двухэтапного ожидания мы возвращаемся к тому, что спин-локи нельзя применять для "долгих" ожиданий.
Наверно без экспериментов не обойтись…
Хочу сказать, что в своей практике я не применяю PriorityLock для долгих ожиданий и не получал просадки производительности из-за этого.
Если у вас есть желание, пожалуйста приведите пример кода, который будет правильнее чем этот:
Хочу сказать, что в своей практике я не применяю 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?В ReadWriteLock сразу несколько читателей могут зайти внутрь критической секции. PriorityLock позволяет только одному потоку заходить в критическую секцию как обычный lock, но имеет два приоритета примерно как в ReadWriteLock.
А что вас смущает в async lock?
А что вас смущает в async lock?
Да, сразу не обратил внимания на это.
Меня смущает то, что async await сделаны, чтобы избегать блокировок и экономить на этом потоки, а они используются для блокировки -.-
Меня напрягает сам
Меня смущает то, что async await сделаны, чтобы избегать блокировок и экономить на этом потоки, а они используются для блокировки -.-
Меня напрягает сам
SemaphoreSlim.WaitAsync, который внутри себя явно вызывает lock, и лично мне наличие этого метода кажется весьма неоднозначным. Плюсом будет только то, что извне можно красивенько это принимать, но выдавать lock за Async.
SemaphoreSlim.WaitAsync не занимает поток, но не дает зайти и выполнить код под блокировкой. Всё по плану, всё хорошо.
Я про то, что внутри async метода содержится lock, а никто не ожидает от async того, что он залочится и не продолжит выполнение. Ну и как и сказал gigavat38
Хм, тогда я не понимаю, о чём вы пишете. SemaphoreSlim.WaitAsync — полноценный async-await метод, дает и блокировку и работает асинхронно. Никакого lock внутри нет. Никакой блокировки треда нет.
В реализации Microsoft есть конструкция lock, про которую я и говорил с:
Сокращённый отрывок исходного кода из исходников с github:
Сокращённый отрывок исходного кода из исходников с github:
Код
Не получилось у меня отформатировать =(
Там Task, возвразающий bool, но хабр проглатывает скобочки .-.
Там 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.
Не, если вы сможете какой-нибудь дедлок с ним создать, будет наверное печально, но в остальном не вижу проблем.
// Dummy object used to in lock statements to protect the semaphore count, wait handle and cancelation
и на нём не висит синхронно цпу, пока ваш код ждёт на WaitAsync.
Не, если вы сможете какой-нибудь дедлок с ним создать, будет наверное печально, но в остальном не вижу проблем.
В обычном использовании он не доставит проблем, но lock есть lock.
Асинхронный метод по определению неблокирующий, а в нём есть блокировка и это меня как-то напрягает. Я не хочу сказать, что «всёх***я, давай по новой», но это может поставить в довольно неловкое положение пользователя этого метода.
Самый очевидный сценарий проблемного поведения: Мы начинаем долбить этот семафор многими потоками с огромной частотой, что приведёт нас к lock concoy, хотя этого мы никак не могли ожидать от асинхронного метода.
Асинхронный метод по определению неблокирующий, а в нём есть блокировка и это меня как-то напрягает. Я не хочу сказать, что «всё
Самый очевидный сценарий проблемного поведения: Мы начинаем долбить этот семафор многими потоками с огромной частотой, что приведёт нас к lock concoy, хотя этого мы никак не могли ожидать от асинхронного метода.
Скрытый текст
Если не ошибаюсь, то про что-то подобное рассказывал Евгений Пешков на DotNext Piter 2019
Я немного о другом случае говорил: если внутри синхронного лока написать await — это гарантированнная проблема. Поэтому компилятор не дает внутри обычного lock писать await.
В исходниках SemaphoreSlim.WaitAsync использование lock находится в синхронной части кода, внутри lock там все детерминированно и максимально быстро выйдет из под него.
В исходниках SemaphoreSlim.WaitAsync использование lock находится в синхронной части кода, внутри lock там все детерминированно и максимально быстро выйдет из под него.
MonkAlex все правильно написал.
От себя лишь добавлю, что async/await — это механизм неблокирующих операций, но никто не отменяет необходимость критических секций в асинхронном коде. И использование синхронных методов синхронизации в асинхронном коде может плохо кончится, потому что Task != Thread, и зайдя в критическую секцию, таска может отпустить поток, а другая таска похватит поток и повторно зайдет в критическую секцию (в контексте использования PriorityLock, другие примитивы не проверял). Поэтому необходимы асинхронные методы синхронизации.
От себя лишь добавлю, что async/await — это механизм неблокирующих операций, но никто не отменяет необходимость критических секций в асинхронном коде. И использование синхронных методов синхронизации в асинхронном коде может плохо кончится, потому что Task != Thread, и зайдя в критическую секцию, таска может отпустить поток, а другая таска похватит поток и повторно зайдет в критическую секцию (в контексте использования PriorityLock, другие примитивы не проверял). Поэтому необходимы асинхронные методы синхронизации.
В поисках решения я наткнулся на проект NeoSmart.AsyncLock
Зачем его вообще использовать?
Есть же прекрасный AsyncEx, который уже давно стандарт де-факто для таких вещей.
Можно поинтересоваться, в какой области вы работаете? Как получилось, что у вас столько шереного стейта, к которому необходимо синхронизировать доступ и даже делать собственные механизмы синхронизации? Не пытались ли решить проблему архитектурными методами.
Приложение из примера автоматизирует пользовательские шаблоны, оно манипулирует большим количеством потоков и большим количеством тяжелых процессов (ограничено лишь ресурсами компьютера). Менеджер из примера обрабатывает задания, при этом должен держать несколько коллекций в синхронизированном состоянии.
На самом деле даже используя обычный lock это не было узким местом. PriorityLock появился для решения проблем с UI. Архитектурно задачу я решил позже, вынес UI в отдельный процесс, потому что понял что основные фризы интерфейса создавал GC.
На самом деле даже используя обычный lock это не было узким местом. PriorityLock появился для решения проблем с UI. Архитектурно задачу я решил позже, вынес UI в отдельный процесс, потому что понял что основные фризы интерфейса создавал GC.
Sign up to leave a comment.
Lock с приоритетами в .NET