8

Currently in a multithreaded environment, we are using a LinkedList to hold data. Sometimes in the logs we get NoSuchElementException while it is polling the linkedlist. Please help in understanding the performance impact if we move from the linkedlist to ConcurrentLinkedQueue implementation.

Thanks, Sachin

Sachin
  • 139
  • 2
  • 2
  • 6

2 Answers2

9

When you get a NoSuchElementException then this maybe because of not synchronizing properly. For example: You're checking with it.hasNext() if an element is in the list and afterwards trying to fetch it with it.next(). This may fail when the element has been removed in between and that can also happen when you use synchronized versions of Collection API.

So your problem cannot really be solved with moving to ConcurrentLinkedQueue. You may not getting an exception but you've to be prepared that null is returned even when you checked before that it is not empty. (This is still the same error but implementation differs.) This is true as long as there is no proper synchronization in YOUR code having checks for emptiness and element retrieving in the SAME synchronized scope.

There is a good chance that you trade NoSuchElementException for having new NullPointerException afterwards.

This may not be an answer directly addressing your question about performance, but having NoSuchElementException in LinkedList as a reason to move to ConcurrentLinkedQueue sounds a bit strange.

Edit

Some pseudo-code for broken implementations:

//list is a LinkedList
if(!list.isEmpty()) {
    ... list.getFirst()
}

Some pseudo-code for proper sync:

//list is a LinkedList
synchronized(list) {
    if(!list.isEmpty()) {
        ... list.getFirst()
    }
}

Some code for "broken" sync (does not work as intended). This maybe the result of directly switching from LinkedList to CLQ in the hope of getting rid of synchronization on your own.

//queue is instance of CLQ
if(!queue.isEmpty()) { // Does not really make sense, because ...
    ... queue.poll() //May return null! Good chance for NPE here!
}

Some proper code:

//queue is instance of CLQ
element = queue.poll();

if(element != null) {
   ...
}

or

//queue is instance of CLQ
synchronized(queue) {
    if(!queue.isEmpty()) {
        ... queue.poll() //is not null
    }
}
Fabian Barney
  • 14,219
  • 5
  • 40
  • 60
  • There is a proper check before the list is polled. Since we are still getting NoSuchElementException, it means there was data in the list when its size was checked but not when it was polled. It could be because some other thread has already polled it out in the mean time. Moving to ConcurrentLinkedQueue would avoid such concurrent access cases. – Sachin Sep 06 '11 at 08:54
  • @Sachin This is exactly what I said. And that is exactly what moving to ConcurrentLinkedQueue would NOT solve! – Fabian Barney Sep 06 '11 at 08:56
  • Except if he is using the linked list as a queue, thus not iterating through it, only adding/removing first/last elements. – Péter Török Sep 06 '11 at 08:57
  • @Fatal Can you please explain it in a bit detail ? – Sachin Sep 06 '11 at 08:58
  • @Péter Even then he have to be prepared that the queue returns null when you checked before its emptiness. Checking for emptiness and retrieving have to be in one sync-scope. The advantage of the CLQ is that you won't get an exception and you can get rid of checking for emptiness and therefore checking for null returned. – Fabian Barney Sep 06 '11 at 08:59
  • @Peter Yes, the linked list is used only as queue. – Sachin Sep 06 '11 at 09:00
  • Note that [`ConcurrentLinkedQueue.poll`](http://download.oracle.com/javase/6/docs/api/java/util/concurrent/ConcurrentLinkedQueue.html#poll%28%29) is (supposed to be) an atomic operation, although the Javadoc doesn't explicitly mention this. – Péter Török Sep 06 '11 at 09:02
  • @Péter This is right, but NO synchronized version of Collection API makes sync for you when needed for having more than one method call in the same sync scope. You always have to do it on your own. Method calls are atomic but that's not enough when you rely on results from the call before. – Fabian Barney Sep 06 '11 at 09:16
  • Indeed, but my point is, using `poll` there's *no need* to rely on results from a previous call, as it `Retrieves and removes the head of this queue, or returns null if this queue is empty.` – Péter Török Sep 06 '11 at 09:19
  • Yes, I said this, too. You can get rid of the emptiness check. But I strongly believe that Sachin do not know about since switching from LinkedList to CLQ with the reason of getting a `NoSuchElementException` is really strange. And there is a good chance to get a `NPE` instead of `NoSuchElementException` instead when Sachin does not know about the real issue in his code and that he have to check for nullness of poll()'s result. – Fabian Barney Sep 06 '11 at 09:24
  • @Fatal You said method calls are atomic. There is a call in my method like linkedList.poll(); Since the method call is atomic, poll method should either return a null (incase list is empty) or the element at the top of the list. It should not throw a NoSuchElementException. Right or am i missing something here ? – Sachin Sep 06 '11 at 09:37
  • @Sachin You mean linkedList.poll()? Then not as long as you are not using a `Collections.synchronizedList(...)` wrapper. I thought you mean `queue.poll()` where queue is an instance of CLQ. That's what these synchronized versions are about. LinkedList is not thread-safe by default. CLQ is thread-safe. – Fabian Barney Sep 06 '11 at 09:54
  • @Fatal Yes, i was talking about linkedlist.poll() method. So, i need to use synchronized block to avoid the NoSuchElementException. Even if i use the CLQ, i might get an exception while polling. – Sachin Sep 06 '11 at 10:15
  • No, look at the examples i provided. When you use a thread-safe implementation like CLQ then you simply can use poll() and check for null afterwards. (Example 4 and the prefered one in your scenario as far as I can see.) But if you're using a not-thread-safe version like LinkedList then you have to synchronize yourself even for a single method call. For multiple method calls where you rely on results from a call before you have to synchronize in BOTH cases on your own. – Fabian Barney Sep 06 '11 at 10:17
  • Yeah, got it now. Thanks Fatal. Appreciate your time. – Sachin Sep 06 '11 at 10:20
8

ConcurrentLinkedQueue [is] an unbounded, thread-safe, FIFO-ordered queue. It uses a linked structure, similar to those we saw in Section 13.2.2 as the basis for skip lists, and in Section 13.1.1 for hash table overflow chaining. We noticed there that one of the main attractions of linked structures is that the insertion and removal operations implemented by pointer rearrangements perform in constant time. This makes them especially useful as queue implementations, where these operations are always required on cells at the ends of the structure, that is, cells that do not need to be located using the slow sequential search of linked structures.

ConcurrentLinkedQueue uses a CAS-based wait-free algorithm that is, one that guarantees that any thread can always complete its current operation, regardless of the state of other threads accessing the queue. It executes queue insertion and removal operations in constant time, but requires linear time to execute size. This is because the algorithm, which relies on co-operation between threads for insertion and removal, does not keep track of the queue size and has to iterate over the queue to calculate it when it is required.

From Java Generics and Collections, ch. 14.2.

Note that ConcurrentLinkedQueue does not implement the List interface, so it suffices as a replacement for LinkedList only if the latter was used purely as a queue. In this case, ConcurrentLinkedQueue is obviously a better choice. There should be no big performance issue unless its size is frequently queried. But as a disclaimer, you can only be sure about performance if you measure it within your own concrete environment and program.

Community
  • 1
  • 1
Péter Török
  • 114,404
  • 31
  • 268
  • 329
  • That was just downvoted inadvertently for much less a second. Clicked it while reshreshing `new answers` and corrected immediatly. :) – Fabian Barney Sep 06 '11 at 08:48
  • @Fatal, it was someone else then, because the downvote is still there. – Péter Török Sep 06 '11 at 09:04
  • 1
    @Peter I was the downvoter, and have now reverted my downvote. At the time I downvoted, the answer was a quote block with no indication as the the source of the quote, nor any supporting text. – razlebe Sep 06 '11 at 09:34
  • Can you suggest a concurrent datastructure which implements the `List` interface? – Adam Arold Dec 19 '13 at 12:56