Throttle actions by number per period

I want to throttle asynchronous actions at a specific maximum rate of n actions per t period, and have pending actions wait until there is a free slot (rather than drop them). I want to receive the outcome When an action completes or fails. I also want the option of cancelling an action. The order of actions is unimportant, though I have been considering if a priority queue would be worth the trouble.

For example, if your maxActions was 2 and your maxPeriod 1 second, if 3 actions each taking 500 ms were queued simultaneously then the first two would run immediately and complete in 500ms but the 3rd would have to wait a whole second before starting, rather than starting at 500ms.

What I've come up with so far is based around two semaphores, one to control the number of actions active at any one time, and one to control the number of actions throughout the max period. I release separately, because it is possible that a request may take longer than the given period.

Are there any logical errors to this approach (e.g., could a semaphore potentially never be released)? Is there a completely different approach that would be better?

```
public class Throttle
{
private readonly TimeSpan _maxPeriod;
private readonly System.Threading.SemaphoreSlim _throttleActions, _throttlePeriods;

public Throttle(int maxActions, TimeSpan maxPeriod)
{
_throttleActions = new System.Threading.SemaphoreSlim(maxActions, maxActions);
_throttlePeriods = new System.Threading.SemaphoreSlim(maxActions, maxActions);
_maxPeriod = maxPeriod;
}

public Task Queue(Func action, System.Threading.CancellationToken cancel)
{
return _throttleActions.WaitAsync(cancel).ContinueWith(t =>
{
try
{
_throttlePeriods.Wait(cancel);

// Release after period
// - Allow bursts up to maxActions requests at once
// - Do not allow more than maxActions requests p

private readonly TimeSpan _maxPeriod;
private readonly System.Threading.SemaphoreSlim _throttleActions, _throttlePeriods;

I would add using System.Threading; at the top of the code file, so as to avoid having to fully qualify everything you're referencing in that namespace.

Also, at a glance (especially with the fully qualified SemaphoreSlim) it looks like you're declaring two readonly fields here, but there's really 3. I find this requires less brain juice to glance at:

private readonly TimeSpan _maxPeriod;
private readonly SemaphoreSlim _throttleActions;
private readonly SemaphoreSlim _throttlePeriods;

That said I like that the fields are readonly and that you're using an underscore prefix, which avoids having to qualify the fields with this in the constructor:

public Throttle(int maxActions, TimeSpan maxPeriod)
{
    this.throttleActions = new SemaphoreSlim(maxActions, maxActions);
    this.throttlePeriods = new SemaphoreSlim(maxActions, maxActions);
    this.maxPeriod = maxPeriod;
}

...and the reason you would have to qualify the fields in the constructor without that underscore prefix, is because you have a consistent naming scheme for your constructor parameters vs. the fields they're assigning, and that's excellent.

I'm not very familiar with asynchronous programming, multithreading or even the TPL - but Throttle tickles my Name-O-Meter™, as it's a verb more than a noun.. and class names should be nouns. Would Throttler be weird for a class like this? The only time a verb seems fine to me is when the class is static, such as Convert.

public Task Queue(Func action, System.Threading.CancellationToken cancel)

Here we have the opposite situation: you're using a noun where a verb would be more appropriate - Enqueue sounds more like it.

I'm on the fence about these comments:

// Release after period
    // - Allow bursts up to maxActions requests at once
    // - Do not allow more than maxActions requests per period

It seems to me that they wouldn't be needed if the constructor had XML comments for its parameters:

/// 
/// Instantiates a new Throttle object
/// 
/// The maximum number of actions allowed per period.
/// The time span to use for delaying tasks.
public Throttle(int maxActions, TimeSpan maxPeriod)
{
    this.throttleActions = new SemaphoreSlim(maxActions, maxActions);
    this.throttlePeriods = new SemaphoreSlim(maxActions, maxActions);
    this.maxPeriod = maxPeriod;
}

I've never used Task.Delay, so I'm not 100% on the actual meaning of maxPeriod - but you get the idea: not only XML documentation cleans up comments in the middle of your implementation, it's also picked up by IntelliSense and shown when you're writing the client code.

StackExchange Code Review Q#87132, answer score: 6