views:

113

answers:

2

I created a simple class that shows what I am trying to do without any noise. Feel free to bash away at my code. That's why I posted it here.

public class Throttled : IDisposable
{
    private readonly Action work;
    private readonly Func<bool> stop;
    private readonly ManualResetEvent continueProcessing;
    private readonly Timer throttleTimer;
    private readonly int throttlePeriod;
    private readonly int throttleLimit;
    private int totalProcessed;

    public Throttled(Action work, Func<bool> stop, int throttlePeriod, int throttleLimit)
    {
        this.work = work;
        this.stop = stop;
        this.throttlePeriod = throttlePeriod;
        this.throttleLimit = throttleLimit;
        continueProcessing = new ManualResetEvent(true);
        throttleTimer = new Timer(ThrottleUpdate, null, throttlePeriod, throttlePeriod);
    }

    public void Dispose()
    {
        throttleTimer.Dispose();
        ((IDisposable)continueProcessing).Dispose();
    }

    public void Execute()
    {
        while (!stop())
        {
            if (Interlocked.Increment(ref totalProcessed) > throttleLimit)
            {
                lock (continueProcessing)
                {
                    continueProcessing.Reset();
                }
                if (!continueProcessing.WaitOne(throttlePeriod))
                {
                    throw new TimeoutException();
                }
            }

            work();
        }
    }

    private void ThrottleUpdate(object state)
    {
        Interlocked.Exchange(ref totalProcessed, 0);
        lock (continueProcessing)
        {
            continueProcessing.Set();
        }
    }
}

Latest Code

public class Throttled
{
    private readonly Func<bool> work;
    private readonly ThrottleSettings settings;
    private readonly Stopwatch stopwatch;
    private int totalProcessed;

    public Throttled(Func<bool> work, ThrottleSettings settings)
    {
        this.work = work;
        this.settings = settings;
        stopwatch = new Stopwatch();
    }

    private void Execute()
    {
        stopwatch.Start();
        while (work())
        {
            if (++totalProcessed > settings.Limit)
            {
                var timeLeft = (int)(settings.Period - stopwatch.ElapsedMilliseconds);
                if (timeLeft > 0)
                {
                    Thread.Sleep(timeLeft);
                }
                totalProcessed = 0;
                stopwatch.Reset();
                stopwatch.Start();
            }
        }
    }
}
+1  A: 

First of all, I would completely get rid of the controlling thread, because its work can be easily done before calling to work().

Then, I would make the worker thread to be different from the main thread, thus unblocking the main thread for other tasks. Next, I would add a function to cancel the processing, which would perhaps set a flag checked the worker thread.

Edit:
According to the comments, our goal is to limit number of work() calls during each throttlePeriod ticks. We can do it better by noting the time in a stopwatch, comparing it after throttleLimit work operations, and sleeping the remaining time. This way we again don't need a timer thread.

Edit: (removed, was incorrect)
Edit:
We can do even some kind of balancing: being within a throttlePeriod, we calculate how much time did the work() take, so we can estimate hw much time all the remaining work()s are going to take, and wait between each two work()s an equal share of the remaining time. This will make us not execute all the work() very fast at the beginning of the allocated period, possibly blocking the DB.

Vlad
Interesting, how should I go about making the timing precise without using a timer?
ChaosPandion
@ChaosPandion: what do you need the timer for? It seems to record nothing, just clears the `totalProcessed`.
Vlad
@ChaosPandion: your worker code waits for the timer after *each* iteration -- is that what you need?
Vlad
Actually, work will be called `throttleLimit` times in `throttlePeriod`.
ChaosPandion
Well, for that you don't need an additional thread. You could just use `StopWatch` and measure a time which you used for `throttleLimit` iterations. If the time is less than `throttleLimit`, you could `Sleep` for the remaining time.
Vlad
Damn, this briefly crossed my mind but I went in another direction.
ChaosPandion
@ChaosPandion: note that the code most probably contains a mistake, see the last edit.
Vlad
I think you misread the code. The if statement `Interlocked.Increment(ref totalProcessed) > throttleLimit` blocks the wait on the event.
ChaosPandion
Oh, you are right, I'll remove the last edit.
Vlad
It works perfectly, I think I wanted this to be more complicated than it is.
ChaosPandion
A: 

Why throttle? and why Sleep() when you can put a thread onto a lower priority and have it soak up ALL unused CPU cycles getting its work done as fast as possible without interrupting any higher priority work?

In fact, why not put all non-UI threads onto a lower thread priority so your application remains responsive overall?

The only caveat here is if you are doing IO - disk access needs to be throttled to keep everything else running smoothly.

Hightechrider
I am throttling access to the DB to prevent locking. I need this job to run during peak business hours.
ChaosPandion