patterncsharpMinor
Throttling duplicate requests in an HttpModule
Viewed 0 times
duplicatethrottlinghttpmodulerequests
Problem
I'm writing a method to throttle duplicate requests taking place within multiple
Currently, I have the following setup:
This will work but I have no way of disposing of the semaphore instances since I need to preserve the
By my calculation it would take up ~156 megabytes of memory for 1 million items stored in the dictionary. It's not a massive amount but I'd really like it to be scalable and somehow find a way to clean up after myself.
Is this a sensible pattern and is there a way I can improve on it?
HttpModule instances within a web application.Currently, I have the following setup:
// Container for semaphores
private static readonly ConcurrentDictionary
SemaphoreSlims = new ConcurrentDictionary();
// Wrapper for getting semaphore
private static SemaphoreSlim GetSemaphoreSlim(string id)
{
return SemaphoreSlims.GetOrAdd(id, new SemaphoreSlim(1, 1));
}
private async Task ProcessImageAsync(HttpContext context)
{
// `hash` is the request path hashed.
SemaphoreSlim semaphore = GetSemaphoreSlim(hash);
await semaphore.WaitAsync();
try
{
// Do awaitable task
}
finally
{
semaphore.Release();
}
}This will work but I have no way of disposing of the semaphore instances since I need to preserve the
ConcurrentDictionary and cannot dispose of a semaphore within the Dispose() method for that HttpModule instance as it could be currently waiting in another instance.By my calculation it would take up ~156 megabytes of memory for 1 million items stored in the dictionary. It's not a massive amount but I'd really like it to be scalable and somehow find a way to clean up after myself.
Is this a sensible pattern and is there a way I can improve on it?
Solution
Semaphore has a bigger overhead than a traditional lock pattern, aside being too complex for what you need. You could use a class such as this:
Then your class can simply be written as:
You will have a class that only allows a single hash to be run at any one time, with the added side effect of not having to release the Semaphore ;)
Not sure if this would work, i'd need to benchmark but, you could reduce the amount of memory further by reducing the commonality before you hash, eg:
Update
I thought that you might be able to use
Actual locking code
Wrapped to use a string as the key
Testing code
public class DeDuper : IDisposable
{
private static readonly ConcurrentDictionary currentRequests = new ConcurrentDictionary();
private string _hash;
public DeDuper(string hash)
{
_hash = hash;
var lockable = currentRequests.GetOrAdd(hash, () => new Object());
Monitor.Enter(lockable);
}
public void Dispose()
{
object lockable;
if(currentRequests.TryGetValue(_hash, out lockable) && Monitor.IsEntered(lockable))
{
Monitor.Exit(lockable);
}
}
~DeDuper()
{
Dispose();
}
}Then your class can simply be written as:
private async Task ProcessImageAsync(HttpContext context)
{
using(new DeDuper(hash))
{
// do awaitable task
}
}You will have a class that only allows a single hash to be run at any one time, with the added side effect of not having to release the Semaphore ;)
Not sure if this would work, i'd need to benchmark but, you could reduce the amount of memory further by reducing the commonality before you hash, eg:
http://mydomain.com/path1/part1 becomes path1/part1
http://mydomain.com/path1/part2 becomes path1/part2Update
I thought that you might be able to use
Interlocked.CompareExchange to do the job that you want, but it turns out AutoResetEvent works better. Here is a working sample:Actual locking code
public class LockAcquisition : IDisposable
{
private static readonly ConcurrentDictionary currentRequests = new ConcurrentDictionary();
private readonly LockWrapper _currentLock;
public LockAcquisition(T key)
{
_currentLock = currentRequests.GetOrAdd(key, new LockWrapper());
_currentLock.AcquireLock();
}
public void Dispose()
{
if (_currentLock != null)
{
_currentLock.Dispose();
}
}
~LockAcquisition()
{
Dispose();
}
private class LockWrapper : IDisposable
{
private readonly AutoResetEvent _locker = new AutoResetEvent(true);
public void AcquireLock()
{
_locker.WaitOne();
}
public void Dispose()
{
_locker.Set();
}
~LockWrapper()
{
Dispose();
}
}
}Wrapped to use a string as the key
public class LockAcquisition : LockAcquisition
{
public LockAcquisition(string key)
: base(key)
{
}
}Testing code
internal class Program
{
private static void Main(string[] args)
{
Run();
Console.ReadKey();
}
private static async void Run()
{
const string hash = "123456";
const string hash2 = "1234567";
using (new LockAcquisition(hash))
{
using (new LockAcquisition(hash2))
{
using (new LockAcquisition(hash)) // This will deadlocked
{
}
await Task.Delay(TimeSpan.FromSeconds(2));
Console.WriteLine("Done 2");
}
await Task.Delay(TimeSpan.FromSeconds(2));
Console.WriteLine("Done 1");
}
}
}Code Snippets
public class DeDuper : IDisposable
{
private static readonly ConcurrentDictionary<String, Object> currentRequests = new ConcurrentDictionary();
private string _hash;
public DeDuper(string hash)
{
_hash = hash;
var lockable = currentRequests.GetOrAdd(hash, () => new Object());
Monitor.Enter(lockable);
}
public void Dispose()
{
object lockable;
if(currentRequests.TryGetValue(_hash, out lockable) && Monitor.IsEntered(lockable))
{
Monitor.Exit(lockable);
}
}
~DeDuper()
{
Dispose();
}
}private async Task ProcessImageAsync(HttpContext context)
{
using(new DeDuper(hash))
{
// do awaitable task
}
}http://mydomain.com/path1/part1 becomes path1/part1
http://mydomain.com/path1/part2 becomes path1/part2public class LockAcquisition<T> : IDisposable
{
private static readonly ConcurrentDictionary<T, LockWrapper> currentRequests = new ConcurrentDictionary<T, LockWrapper>();
private readonly LockWrapper _currentLock;
public LockAcquisition(T key)
{
_currentLock = currentRequests.GetOrAdd(key, new LockWrapper());
_currentLock.AcquireLock();
}
public void Dispose()
{
if (_currentLock != null)
{
_currentLock.Dispose();
}
}
~LockAcquisition()
{
Dispose();
}
private class LockWrapper : IDisposable
{
private readonly AutoResetEvent _locker = new AutoResetEvent(true);
public void AcquireLock()
{
_locker.WaitOne();
}
public void Dispose()
{
_locker.Set();
}
~LockWrapper()
{
Dispose();
}
}
}public class LockAcquisition : LockAcquisition<String>
{
public LockAcquisition(string key)
: base(key)
{
}
}Context
StackExchange Code Review Q#58964, answer score: 2
Revisions (0)
No revisions yet.