Does this basic Java object pool work?

Question

Does the following basic object pool work? I have a more sophisticated one based on the same idea (i.e. maintaining both a Semaphore and a BlockingQueue). My question is - do I need both Semaphore and BlockingQueue? Am I right that I don't need to do any synchronisation?

import java.util.Collection;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.Semaphore;

public final class Pool<T> {

    private final BlockingQueue<T> objects;
    private final Semaphore permits;

    public Pool(Collection<? extends T> objects) {
        // we have as many permits as objects in our pool:
        this.permits = new Semaphore(objects.size());
        this.objects = new ArrayBlockingQueue<T>(objects.size(), false, objects);
    }

    public T borrow() {
        this.permits.acquireUninterruptibly();
        // we have a permit, so there must be one in there:
        return this.objects.poll();
    }

    public void giveBack(T object) {
        this.objects.add(object);
        this.permits.release();
    }
}

Answer 1

Maybe you should check that objects exists, that's the only thing I have.

Edit: I didn't read the code that carefully. So I edtied the post a bit. :(

Answer 2

A somewhat modified sjlee's example; allowing creation of expensive objects on demand. My case did not require any blocking facility hence I have replaced this with non-blocking queue type. As a benefit, there's no need to deal with InterruptedExceptions.

import java.util.Collection;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;

public abstract class ObjectPool<T> {

    private final Queue<T> objects;

    public ObjectPool() {
        this.objects = new ConcurrentLinkedQueue<T>();
    }

    public ObjectPool(Collection<? extends T> objects) {
        this.objects = new ConcurrentLinkedQueue<T>(objects);
    }

    public abstract T createExpensiveObject();

    public T borrow() {
        T t;
        if ((t = objects.poll()) == null) {
            t = createExpensiveObject();
        }
        return t;
    }

    public void giveBack(T object) {
        this.objects.offer(object);   // no point to wait for free space, just return
    }
}

Answer 3

Maybe use a stack instead of a queue? This gives a chance of getting an object that is still sitting in the processor cache.

Answer 4

Here is one more simple and complete pool for latter one. It's better than the simplest, and it's simple.

From here

/**
 * 
 * @see <a href=http://www.javacodegeeks.com/2013/08/simple-and-lightweight-pool-implementation.html>simple pool</>
 */
abstract static class ObjectPool<T>
{
    private ConcurrentLinkedQueue<T> pool;

    private ScheduledExecutorService executorService;

    /**
     * Creates the pool.
     *
     * @param minIdle minimum number of objects residing in the pool
     */
    public ObjectPool(final int minIdle)
    {
        // initialize pool
        initialize(minIdle);
    }

    /**
     * Creates the pool.
     *
     * @param minIdle            minimum number of objects residing in the pool
     * @param maxIdle            maximum number of objects residing in the pool
     * @param validationInterval time in seconds for periodical checking of minIdle / maxIdle conditions in a separate thread.
     *                           When the number of objects is less than minIdle, missing instances will be created.
     *                           When the number of objects is greater than maxIdle, too many instances will be removed.
     */
    public ObjectPool(final int minIdle, final int maxIdle, final long validationInterval)
    {
        // initialize pool
        initialize(minIdle);

        // check pool conditions in a separate thread
        executorService = Executors.newSingleThreadScheduledExecutor();
        executorService.scheduleWithFixedDelay(new Runnable()
        {
            @Override
            public void run()
            {
                int size = pool.size();
                if (size < minIdle)
                {
                    int sizeToBeAdded = minIdle - size;
                    for (int i = 0; i < sizeToBeAdded; i++)
                    {
                        pool.add(createObject());
                    }
                } else if (size > maxIdle)
                {
                    int sizeToBeRemoved = size - maxIdle;
                    for (int i = 0; i < sizeToBeRemoved; i++)
                    {
                        pool.poll();
                    }
                }
            }
        }, validationInterval, validationInterval, TimeUnit.SECONDS);
    }

    /**
     * Gets the next free object from the pool. If the pool doesn't contain any objects,
     * a new object will be created and given to the caller of this method back.
     *
     * @return T borrowed object
     */
    public T borrowObject()
    {
        T object;
        if ((object = pool.poll()) == null)
        {
            object = createObject();
        }

        return object;
    }

    /**
     * Returns object back to the pool.
     *
     * @param object object to be returned
     */
    public void returnObject(T object)
    {
        if (object == null)
        {
            return;
        }

        this.pool.offer(object);
    }

    /**
     * Shutdown this pool.
     */
    public void shutdown()
    {
        if (executorService != null)
        {
            executorService.shutdown();
        }
    }

    /**
     * Creates a new object.
     *
     * @return T new object
     */
    protected abstract T createObject();

    private void initialize(final int minIdle)
    {
        pool = new ConcurrentLinkedQueue<T>();

        for (int i = 0; i < minIdle; i++)
        {
            pool.add(createObject());
        }
    }
}

Answer 5

Its worth nothing that an ArrayBlockingQueue creates an object when you take an entry from it. So your pool won't actually save objects. It could only help if your objects are expensive to create.

Answer 6

Use take() instead of poll(), and put() instead of add(). The semaphore is then completely redundant so you can just get rid of it. But yes, that looks good.