3

I need a fast way to read/write from/to an int array, concurrently. The write is needed per index. The reading operation is only needed for the complete array after all writes are done.

I started with a synchronized version:

public final class SynchronizedIntArray {
    private final int[] elements = new int[1000000];

    public synchronized void set(int index, int value)
    {
        elements[index] = value;
    }

    public synchronized int[] getAll()
    {
        return elements;
    }
}

The performance isn't good. I searched for a better solution online and found AtomicIntegerArray

public final class ConcurrentIntArray
{
    private final AtomicIntegerArray elements = new AtomicIntegerArray(1000000);

    public void set(int index, int value)
    {
        elements.set(index, value);
    }

    public int[] getAll()
    {
        int[] intArray = new int[elements.length()];
        for (int i = 0; i < intArray.length(); i++) {
            intArray[i] = elements.get(i);
        }
        return intArray;
    }
}

But i am wondering why there is no method to get the complete int array from the AtomicIntegerArray at once. The performance is still much better than the synchronized version, but do i really need to copy it in this way?

Is there even a faster alternative than AtomicIntegerArray for my problem?

coder
  • 1,415
  • 2
  • 12
  • 15
  • you state that 'The performance isn't good.' - where is it no good. I do not think you will better than `elements[index] = value;` have a look at the code for `AtomicIntegerArray` and see what they are doing. – Scary Wombat Jan 21 '14 at 01:09
  • @user2310289 Actually, according to the [source](http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/util/concurrent/atomic/AtomicIntegerArray.java#AtomicIntegerArray.0unsafe), they are outsourcing the storage to a native method using the sun.misc.Unsafe class, which depending on the implementation may or may not be faster... – Sinkingpoint Jan 21 '14 at 01:12
  • @user2310289: The performance benefit isn't in the array assignment, but in avoiding the `synchronized`, which locks the entire array for a single assignment. – Louis Wasserman Jan 21 '14 at 01:14
  • Thanks - I should take my own advise. – Scary Wombat Jan 21 '14 at 01:18

3 Answers3

4

Yes, you really do need to copy it in this way. AtomicIntegerArray's implementation means that there's no way to lock the entire array and get it without any other threads making changes at the same time: you can only atomically get one element at a time.

Louis Wasserman
  • 191,574
  • 25
  • 345
  • 413
1

I am no Java programmer, but as a real-time programmer I think both of your implementations are conceptually wrong.
I might have misunderstood how Java synchronization works, though. In that case, please disregard this post and accept my apologies.

Your first solution

makes sure there is no concurrency between writers because you lock the access to the only function that allows to modify one element.
You also lock access to the array as a whole, but only between readers (which is useless since the readers will not modify the data, and thus could safely read the array concurrently).

However, you do not prevent concurrent access between readers and writers. It is possible that one (single) reader reads the value of an element that is being modified by a (single) writer.

Besides, you return a reference to your member array instead of a copy, so the caller will access the actual data that are likely to be overwritten at any time by the writers.
As a consequence, the array is not protected at all against read accesses during writes, even after you called your read accessor.

The code might work if for some reason the actual access to an element of the array is atomic (which I very doubt, but again Java is not my cup of tea) or more likely because potential inconsistencies are rare and not easily detectable. Your application could go haywire if you run on a different environment or even change your code slightly so that it increases the occurences of concurrent read/writes or becomes more vulnerable to these inconsistencies.

Your second solution

uses the AtomicArray, that does the same as your precedent code, but adds a protection to concurrent reads on a single array value.
It gets rid of the potential inconstistencies of the first solution (also because this time your reader function is forced to return a copy of the actual array), but reading the whole array is terribly inefficient, because your AtomicArray object will take and release the lock for each value read.

A possible approach

What you must protect is the array as a whole.

In a general case, you might want to use a reader/writer lock. That would allow multiple readers to access the array concurrently.
However, if I understood correctly, in your case there is only one reader, so better use a simple lock.

The code for your single write and global read would be as follows:

public final class SynchronizedIntArray {
    private final int[] elements = new int[1000000];
    private final Lock lock = new ReentrantLock();

    public void set(int index, int value)
    {
        lock.lock();
        elements[index] = value;
        lock.unlock();
    }

    public int[] getAll()
    {
        int[] copy;
        lock.lock();
        copy = elements.clone();
        lock.unlock();
        return copy;
    }
}
Community
  • 1
  • 1
kuroi neko
  • 8,479
  • 1
  • 19
  • 43
-1

yes there is a very good implementation better than AtomicIntegerArray for you use case. Key things here are reference and int assignments are atomic. volatile writes are not optimized by the compiler.

//This class is thread safe 
public final class BetterIntArray {

private volatile int[] elements = new int[1000000]; // Don't make it finall

public void set(int index, int value)
{
    elements[index] = value; //assignment is atomic
    elements =  elements; // guarantees that changes are visible to other thread
}

public int[] getAll()
{
    return elements ;
}    

}

veritas
  • 2,444
  • 1
  • 21
  • 30
  • Please do also mention the reason for down voting – veritas Jan 23 '14 at 00:41
  • The assignment to the array `elements[index]` is not really atomic (volatile). Only assignment to the `elements` variable itself is volatile. See, e.g., [here](http://stackoverflow.com/q/2236184/2032064) – Mifeet Mar 08 '17 at 13:54