HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Object pool pattern for asynchronous socket operations

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
operationsasynchronousforpoolobjectsocketpattern

Problem

I have written my own object pool class called ObjectPool which uses SpinLock and Stack:

```
public class ObjectPool where T : class
{
private int _size;
private Func _factory;
private SpinLock _spinLock;

// storage for the pool objects.
// the first item is expected to be most often case.
private T _firstItem;
private Stack _cache; //Use Queue for first-in-first-out (FIFO)

public ObjectPool(Func factory) : this(factory, Environment.ProcessorCount * 2) { }

public ObjectPool(Func factory, int size)
{
_factory = factory;
_size = size;
_cache = new Stack();
_spinLock = new SpinLock(false);
}

///
/// Produces an instance.
///
public T Rent()
{
bool lockTaken = false;
_spinLock.Enter(ref lockTaken);

try
{
T instance = _firstItem;
if (instance == null || instance != Interlocked.CompareExchange(ref _firstItem, null, instance))
{
instance = RentFromCache();
}

return instance;
}
finally
{
if (lockTaken)
{
_spinLock.Exit(false);
}
}
}

private T RentFromCache()
{
var cache = _cache;

if (cache.Count > 0)
{
T instance = _cache.Pop();

if (instance != null)
{
return instance;
}
}

return CreateInstance();
}

private T CreateInstance()
{
var instance = _factory();
return instance;
}

///
/// Returns objects to the pool.
///
public void Return(T obj)
{
bool lockTaken = false;
_spinLock.Enter(ref lockTaken);

try
{
if (_firstItem == null)
{
// worst case scenario: two objects may be stored into same slot.
_firstItem = obj;

Solution

Because you don't change

private int _size;
private Func _factory;
private Stack _cache;


private SpinLock _spinLock; //Use Queue for first-in-first-out (FIFO)

you should make them readonly.

Correction after the comment of t3chb0t:

Don't make the SpinLock readonly. Further clarification: https://stackoverflow.com/a/9235028/2655508


The underlying problem is that the C# compiler creates a copy of a readonly value type field when you call a non-static method on it and executes that method on the copy - because the method could have side effects that change the value of the struct - which is not allowed for readonly fields.

private T RentFromCache()
{
    var cache = _cache;

    if (cache.Count > 0)
    {
        T instance = _cache.Pop();

        if (instance != null)
        {
            return instance;
        }
    }

    return CreateInstance();
}


Here you are using var cache only for checking its Count property. I don't see a reason to not use the class level _cache directly. Using the class level variable directly won't confuse the reader of the code. The same applies to ReturnToCache() as well.

Code Snippets

private int _size;
private Func<T> _factory;
private Stack<T> _cache;
private T RentFromCache()
{
    var cache = _cache;

    if (cache.Count > 0)
    {
        T instance = _cache.Pop();

        if (instance != null)
        {
            return instance;
        }
    }

    return CreateInstance();
}

Context

StackExchange Code Review Q#156379, answer score: 4

Revisions (0)

No revisions yet.