Это правильный дизайн синхронизации Interlocked?

У меня есть система, которая берет образцы. У меня есть несколько клиентских потоков в приложении, которые заинтересованы в этих образцах, но фактический процесс взятия образца может происходить только в одном контексте. Это достаточно быстро, чтобы блокировать вызывающий процесс до тех пор, пока не будет выполнена выборка, но достаточно медленно, чтобы я не хотел, чтобы несколько потоков накапливали запросы. Я придумал этот дизайн (урезанный до минимума деталей):

public class Sample
{
    private static Sample _lastSample;
    private static int _isSampling;

    public static Sample TakeSample(AutomationManager automation)
    {
        //Only start sampling if not already sampling in some other context
        if (Interlocked.CompareExchange(ref _isSampling, 0, 1) == 0)
        {
            try
            {
                Sample sample = new Sample();
                sample.PerformSampling(automation);
                _lastSample = sample;
            }
            finally
            {
                //We're done sampling
                _isSampling = 0;
            }
        }

        return _lastSample;
    }

    private void PerformSampling(AutomationManager automation)
    {
        //Lots of stuff going on that shouldn't be run in more than one context at the same time
    }
}

Это безопасно для использования в сценарии, который я описал?


multithreading c# synchronization locking interlocked
person Dan Bryant    schedule 07.04.2010    source источник

Ответы (2)


arrow_upward
5
arrow_downward

Да, это выглядит безопасно, потому что int здесь атомарный тип. Но я бы все же посоветовал заменить

private static int _isSampling;

с

private static object _samplingLock = new object();

и используйте:

lock(_samplingLock)
{
    Sample sample = new Sample();
    sample.PerformSampling(automation);
   _lastSample = sample;
}

Просто потому, что это рекомендуемый шаблон, а также гарантирует, что все обращения к _lastSample обрабатываются правильно.

NB: я ожидаю сопоставимой скорости, блокировка использует управляемый класс Monitor, который использует Interlocked внутри.

Изменить:

Я пропустил аспект отката, вот еще одна версия:

   if (System.Threading.Monitor.TryEnter(_samplingLock))
   {
     try
     {
         .... // sample stuff
     }
     finally
     {
          System.Threading.Monitor.Exit(_samplingLock);
     }
   }
person Henk Holterman    schedule 07.04.2010
comment
TryEnter, вероятно, лучше с точки зрения обслуживания, так как, скорее всего, больше людей будут знакомы с Monitor.TryEnter, чем с Interlocked.CompareExchange. В этом случае я не слишком беспокоюсь о накладных расходах, так как я беру образцы всего несколько раз в секунду. Я уверен, что когда он действительно выполняет выборку, накладные расходы на сам процесс выборки будут немного больше, чем на блокирующий примитив. - person Dan Bryant; 08.04.2010
comment
Мне нравится Monitor.TryEnter, но разве мы не должны использовать ReaderWriterLockSlim в наши дни?? msdn.microsoft.com/en-us/library/ - person Mike_G; 08.04.2010
comment
@Mike_g: Нет, только когда этого требует ситуация. Я не вижу этого здесь. - person Henk Holterman; 08.04.2010
comment
Хорошо, теперь он использует TryEnter; так много для «умного» решения. :) Кстати, я не могу редактировать сообщение, но правильный метод — Monitor.Exit(_samplingLock). - person Dan Bryant; 08.04.2010
comment
Interlocked.CompareExchange() очень эффективен, чем данное решение. Ты только что изменил то, что он просил. - person Simple Fellow; 19.10.2015
comment
@SimpleFellow - не понятно, что вы имеете в виду. Эффективный был рассмотрен здесь, речь идет об общей практике блокировки ресурса, такого как _lastSample . - person Henk Holterman; 19.10.2015
comment
не Interlocked.CompareExchange() в разы быстрее, чем Monitor.TryEnter()/Exit()? (Ссылка: blog.teamleadnet.com /2012/02/) - person Simple Fellow; 19.10.2015
comment
Это быстрее, но заметите ли вы это здесь? Не выполняйте микрооптимизацию за счет удобочитаемости. - person Henk Holterman; 21.10.2015

arrow_upward
-1
arrow_downward

Обычно я объявляю volatile bool и делаю что-то вроде:

private volatile bool _isBusy;
private static Sample _lastSample;

private Sample DoSomething()
{
     lock(_lastSample)
     {
       if(_isBusy)
          return _lastSample;
       _isBusy = true;
     }

     try
     {
       _lastSample = new sameple//do something
     }
     finally
     {
        lock(_lastSample)
        {
           _isBusy = false;
        }
     }
     return _lastSample;
}
person Mike_G    schedule 07.04.2010
comment
Нет необходимости объявлять bool volatile, если вы все равно используете его только внутри блокировки. Кроме того, очень плохая практика блокировать изменяемое поле. В вашем образце возможно одновременное введение двух блоков блокировки. Когда вы блокируете изменяемое поле, я ожидаю подробного комментария о том, почему это не повредит (так сильно), если два (или, может быть, даже больше) потока одновременно войдут в блокирующий блок. - person Patrick Huizinga; 18.04.2011