Throttling class

The idea of this class is that several threads are sending data over a network and each thread are sharing the same instance of this class and before sending N bytes over the network each thread is calling ThrottledWait(n).

My worry is that each thread might run on different core and get different value for DateTime.UtcNow.Ticks. I am not 100% sure it's thread-safe.

Also calling Thread.Sleep(ts) might sleep for longer than asked for and might cause traffic to not be smooth because of aliasing so we might want to do a thread.sleep() for less than the calculated amount and waste the remaining time checking DateTime.UtcNow.Ticks in a busy loop.

```
public class Throttler
{
// Use this constant as average rate to disable throttling
public const long NoLimit = -1;
// Number of consumed tokens
private long _consumedTokens;
// timestamp of last refill time
private long _lastRefillTime;
// ticks per period
private long _periodTicks;

private double _averageRate;

public long BurstSize
{
get;
set;
}

public long AverageRate
{
get { return (long)_averageRate; }
set { _averageRate = value; }
}

public TimeSpan Period
{
get
{
return new TimeSpan(_periodTicks);
}
set
{
_periodTicks = value.Ticks;
}
}
public Throttler()
{
BurstSize = 1;
AverageRate = NoLimit;
Period = TimeSpan.FromSeconds(1);
}

///
/// Create a Throttler
/// ex: To throttle to 1024 byte per seconds with burst of 200 byte use
/// new Throttler(1024,TimeSpan.FromSeconds(1), 200);
///
/// The number of tokens to add to the bucket every interval.
/// Timespan of on interval.
///
public Throttler(long averageRate, TimeSpan period, long burstSize = 1)
{
BurstSize = burstSize;
AverageRate = averageRate;
Period = period;
}

Take this answer as general advice.

I see that the nesting seems to be a bit deeper than what people normally see. Most of the time it is not a problem. However, sometimes placing a lot of code in a deeply indented block might force the reader to scroll to the right to view the rest of the code. It's not significant but it can save a few seconds of scrolling for the person who reads your code (maybe even yourself).

There are a few ways to tackle this problem. I'll demonstrate with your code.
Consider the following snippet:

if (newTokens > 0)
    {
        long newRefillTime = refillTime == 0
            ? currentTimeTicks
            : refillTime + (long)(newTokens * _periodTicks / _averageRate);

        if (Interlocked.CompareExchange(ref _lastRefillTime, newRefillTime, refillTime) == refillTime)
        {
            // Loop until we succeed in refilling "newTokens" tokens
            while (true)
            {
                long currentLevel = System.Threading.Volatile.Read(ref _consumedTokens);
                long adjustedLevel = (long)Math.Min(currentLevel, BurstSize); // In case burstSize decreased
                long newLevel = (long) Math.Max(0, adjustedLevel - newTokens);
                if (Interlocked.CompareExchange(ref _consumedTokens, newLevel, currentLevel) == currentLevel)
                {
                    return;
                }
            }
        }
    }

The outermost if block doesn't have an else. One thing we can do is invert the condition and return.

if (newTokens <= 0)
        {
            return;
        }
        // rest of the code

There is an immediate improvement. We can extend this further.

if (Interlocked.CompareExchange(ref _lastRefillTime, newRefillTime, refillTime) != refillTime)
        {
            return;
        }
        // Loop until we succeed in refilling "newTokens" tokens
        while (true)
        {
            long currentLevel = System.Threading.Volatile.Read(ref _consumedTokens);
            long adjustedLevel = (long) Math.Min(currentLevel, BurstSize); // In case burstSize decreased
            long newLevel = (long) Math.Max(0, adjustedLevel - newTokens);
            if (Interlocked.CompareExchange(ref _consumedTokens, newLevel, currentLevel) == currentLevel)
            {
                return;
            }
        }

It is easiest for if statements that are near the end of a method and lack an else. For other cases, it might not be possible. I generally try to detect the cases that I don't need to handle and exit quickly, rather than check if I need to execute a piece of code. The idea is to keep on performing eliminations until we have the ideal conditions necessary for the main code to work.

For the while loop, you could use a do {...} while (...); loop. Lesser code and exactly what you intend to do anyway:

// Loop until we succeed in refilling "newTokens" tokens
        do
        {
            long currentLevel = System.Threading.Volatile.Read(ref _consumedTokens);
            long adjustedLevel = (long) Math.Min(currentLevel, BurstSize); // In case burstSize decreased
            long newLevel = (long) Math.Max(0, adjustedLevel - newTokens);
        }
        while (Interlocked.CompareExchange(ref _consumedTokens, newLevel, currentLevel) != currentLevel);

Performance remains fairly the same. It might seem unnecessary but it helps prevent monstrosities like:

A little goes a long way.

StackExchange Code Review Q#157478, answer score: 6