0

I share a list between many thread. All the data needed by the thread are there before they start, I don't add any other value to the list. Each tread take a value in list, value is removed from the list and there is a distant call done. This block is synchronized.

    while (!contactList.isEmpty()) {

        Contact contact = null;

        synchronized (contactList) {

            if (!contactList.isEmpty()) {

                contact = contactList.get(0);
                contactList.remove(0);
            }
        }
        //call the service with contact
    }

Is there a more efficient way to do the job?

For the moment, it's faster to take all contact and split them to many individual program.

robert trudel
  • 5,283
  • 17
  • 72
  • 124
  • Do you care about getting anything from *any other spot* in the list besides the first entry? – Makoto Apr 27 '15 at 17:35
  • `remove(0)` will shift all the remaining elements of the list to left. Removing the last element is one way of improving it. – async Apr 27 '15 at 17:38
  • so i could read last element and removing it. – robert trudel Apr 27 '15 at 17:50
  • How do you conclude that the sychronized access is the problem? From the small code excerpt I would presume its *unlikely* synchronization causes any noticeably performance impact here. More likely that remove(0) is to blame (for ArrayList that a O(N) complexity operation). – Durandal Apr 27 '15 at 18:40

1 Answers1

3

If you genuinely only want to pull elements from the front of your list and you want to be sure that they're there, then you should look into a ConcurrentLinkedQueue instead. This also guarantees O(1) insertion and removal, since insertion occurs at the end of the queue, and retrieval occurs at its head without the need to shift elements down.

Because you state that the data exists prior to entering this block, a call to poll will only return null if the list itself is empty. From there, you'd have to decide what it means to return null from the queue.

A (very rough) example:

ConcurrentLinkedQueue<Contact> contactQueue = new ConcurrentLinkedQueue<>();
Contact contact = contactQueue.poll(); // will return null if empty
Makoto
  • 104,088
  • 27
  • 192
  • 230
  • thread don't add value, it's just i don't want thread take the same value, that why i put a synchronisez block and a i remove the value. – robert trudel Apr 27 '15 at 17:49
  • Okay, so if the data's already there before you get to this block of code (somehow), then there's a different data structure that can be used. – Makoto Apr 27 '15 at 17:54
  • just want to take a value for a datastructure and don't take same from many thread... – robert trudel Apr 27 '15 at 17:55
  • I've revised my answer. You can use a `ConcurrentLinkedQueue` which accomplishes the same thing without any blocking. – Makoto Apr 27 '15 at 17:56
  • any need to synchronized the block? – robert trudel Apr 27 '15 at 18:08
  • No. `ConcurrentLinkedQueue` is thread safe. You don't have to synchronize anything by hand when using it. – Makoto Apr 27 '15 at 18:11
  • if (!contactList.isEmpty()) { System.out.println(contactList.size()); contactList= contactList.poll(); } with this code, same value is something displayed many time.. – robert trudel Apr 27 '15 at 18:22