CODEWITHSUNDEEP
javamultithreading
user2479100
user2479100

Reputation: 105

LinkedBlockingQueue .take() performance

I am using LinkedBlockingQueue that is getting filled by multiple threads, the number of items is big (tens of millions objects).

LinkedBlockingQueue.take() takes a lot of time (checked via profiler) - 56% of time.
The queue is never empty!!

What can affect the performance of the take() method?

UPDATE: I did some changes in code that handles the result of the take(), I also placed the take() into another thread, the performance was improved almost by 50%.

I don't understand how this is possible because I didn't change any logic of putters...

UPDATE:

I've counted the number of times queues is full before calling take():
With original code 90% of calls queue was full.
With improved code 13% of calls queue was full.

Upvotes: 0

Views: 2217

Answers (2)

selig
selig

Reputation: 4844

If we look at the code here we can see that a lock must be taken before taking an element. If lots of threads are taking then there will be contention on this lock - the threads aren't waiting for something to appear but for other threads to take things.

 public E take() throws InterruptedException {
        E x;
        int c = -1;
        final AtomicInteger count = this.count;
        final ReentrantLock takeLock = this.takeLock;
        takeLock.lockInterruptibly();
        try {
            try {
                while (count.get() == 0)
                    notEmpty.await();
            } catch (InterruptedException ie) {
                notEmpty.signal(); // propagate to a non-interrupted thread
                throw ie;
            }

            x = extract();
            c = count.getAndDecrement();
            if (c > 1)
                notEmpty.signal();
        } finally {
            takeLock.unlock();
        }
        if (c == capacity)
            signalNotFull();
        return x;
    }

EDIT

In the case where you have one taker and lots of putters and the queue keeps getting full the bottleneck will become signalNotFull() given by this code

private void signalNotFull() {
    final ReentrantLock putLock = this.putLock;
    putLock.lock();
    try {
        notFull.signal();
    } finally {
        putLock.unlock();
    }
}

This needs to take the putLock to signal the fact that there is now space in the queue.

Upvotes: 5

Peter Lawrey
Peter Lawrey

Reputation: 533442

take() is fairly light weight but if you call it enough it will consume lots of CPU. It sounds like you are passing large numbers of objects which require very little work for the consumer(s). I suggest trying to restructure you problem so this is done more efficiently.

You can for example,

  • use Disruptor which is designed for this sort of problem.
  • send blocks/batches of data e.g. a List of objects instead of many individual object.
  • use multiple Exchangers (fine for a small number of writers)
  • use something like Chronicle if your aim is to persist the data.

It is also possible your profile is not completely accurate. When you measure small periods of time it can give mixed results.

BTW: the take() is single threaded. If you have many threads trying to call take() at the same time, they will be blocking each other.

Upvotes: 5

Related Questions