Выполняющий блокирующий шаблон

Question

Я работаю над приведенным ниже кодом и стараюсь сделать это так быстро, как может быть.

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

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

У кого-нибудь есть отзывы о том, как это можно улучшить?

public class TriggerReduce 
{ 
    private readonly object _lock = new object(); 
    private readonly int _autoReduceInterval = 5; 
    private DateTime _lastTriggered; 

    public void Execute(object sender, EventArgs e) 
    { 
     var currentTime = DateTime.Now; 
     if (currentTime.Subtract(_lastTriggered).Duration().TotalMinutes > _autoReduceInterval) 
     { 
      var shouldRun = false; 
      lock (_lock) 
      { 
       if (currentTime.Subtract(_lastTriggered).Duration().TotalMinutes > _autoReduceInterval) 
       { 
        _lastTriggered = currentTime; 
        shouldRun = true; 
       } 
      } 

      if (shouldRun) 
      { 
       Task.Factory.StartNew(() => 
       { 
        //Trigger reduce which is a long running task 
       }, TaskCreationOptions.LongRunning); 
      } 
     } 
    } 
} 

Answer 1

Use Monitor.TryEnter.

if (Monitor.TryEnter(_lock)) 
{ 
    try 
    { 
     if (currentTime.Subtract(_lastTriggered).Duration().TotalMinutes > 
      _autoReduceInterval) 
     { 
      _lastTriggered = currentTime; 
      shouldRun = true; 
     } 
    } 
    finally 
    { 
     Monitor.Exit(_lock); 
    } 
} 

Answer 2

О, я бы этого не делал! Помещенный «если (CURRENTTIME» и «shouldRun» вещи обратно в замок

Не изменять/проверять состояние за пределами замка. -. Это обязательно завинтить

В этом случае поток, который только что установил 'shouldRun' в true, может иметь свое решение, отмененное другим потоком, который входит и снова устанавливает false, чтобы снова зацепиться за блокировку. Первый поток затем не попадает в «StartNew», а более поздний поток выиграл «т либо потому, что первый поток установить _lastTriggered к текущему времени.

OTOH :) так как„shouldRun“ является автоматически varaible, а не полем, это не состояние. Только один поток может попасть внутрь замка, дважды проверьте интервал и обновите время _lastTriggered.

Мне не нравится эта двойная проверка, но на данный момент не может понять, почему это не сработает.

Answer 3

Было бы полезно избежать блокировки и вместо этого использовать Interlocked.Exchange?

E.g.

private long _lastTriggeredTicks; 

private DateTime lastTriggered 
{ 
    get 
    { 
     var l = Interlocked.Read(ref _lastTriggeredTicks); 
     return new DateTime(l); 
    } 
    set 
    { 
     Interlocked.Exchange(ref _lastTriggeredTicks, value); 
    } 
} 

Из того, что я понимаю Interlocked is faster than a lock заявление.

Answer 4

public class TriggerReduce //StartNew is fast and returns fast 
{ 

    private readonly object _lock = new object(); 
    private readonly int _triggerIntervalMins = 5; 

    private DateTime _nextTriggerAt = DateTime.MinValue; 
    private bool inTrigger = false; 

    public void Execute(object sender, EventArgs e) 
    { 
     lock (_lock) 
     { 
      var currentTime = DateTime.Now; 
      if (_nextTriggerAt > currentTime) 
       return; 

      _nextTriggerAt = currentTime.AddMinutes(_triggerIntervalMins);//runs X mins after last task started running (or longer if task took longer than X mins) 
     } 

     Task.Factory.StartNew(() => 
     { 
      //Trigger reduce which is a long running task 
     }, TaskCreationOptions.LongRunning); 
    } 
} 


public class TriggerReduce//startNew is a long running function that you want to wait before you recalculate next execution time 
{ 

    private readonly object _lock = new object(); 
    private readonly int _triggerIntervalMins = 5; 

    private DateTime _nextTriggerAt = DateTime.MinValue; 
    private bool inTrigger = false; 

    public void Execute(object sender, EventArgs e) 
    { 
     var currentTime; 
     lock (_lock) 
     { 
      currentTime = DateTime.Now; 
      if (inTrigger || (_nextTriggerAt > currentTime)) 
       return; 

      inTrigger = true; 
     } 
     Task.Factory.StartNew(() => 
     { 
      //Trigger reduce which is a long running task 
     }, TaskCreationOptions.LongRunning); 

     lock (_lock) 
     { 
      inTrigger = false; 
      _nextTriggerAt = DateTime.Now.AddMinutes(_triggerIntervalMins);//runs X mins after task finishes 
      //_nextTriggerAt = currentTime.AddMinutes(_triggerIntervalMins);//runs X mins after last task started running (or longer if task took longer than X mins) 
     } 
    } 
} 

Answer 5

Я думаю, что у вас уже есть довольно разумный подход. Большая проблема в том, что вы получаете доступ к _lastTriggered вне замка. Двойная блокировка идиомы здесь не будет работать. Просто ваш код, чтобы он выглядел так.

public void Execute(object sender, EventArgs e) 
{ 
    var currentTime = DateTime.Now; 
    var shouldRun = false; 
    lock (_lock) 
    { 
     TimeSpan span = currentTime - _lastTriggeed; 
     if (span.TotalMinutes > _autoReduceInterval) 
     { 
      _lastTriggered = currentTime; 
      shouldRun = true; 
     } 
    } 

    if (shouldRun) 
    { 
     Task.Factory.StartNew(() => 
     { 
      //Trigger reduce which is a long running task 
     }, TaskCreationOptions.LongRunning); 
    } 
}