51

I found the following Java code.

for (int type = 0; type < typeCount; type++)
    synchronized(result) {
        result[type] += parts[type];
    }
}

where result and parts are double[].

I know basic operations on primitive types are thread-safe, but I am not sure about +=. If the above synchronized is necessary, is there maybe a better class to handle such operation?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Piotr Stapp
  • 19,392
  • 11
  • 68
  • 116
  • 8
    The operation is not _atomic_, which is why external synchronization may be required. – Mick Mnemonic Sep 20 '15 at 07:12
  • 15
    You're also slightly confused about what you refer to as basic operations (I assume you mean reads and writes). It is true that a read of a primitive type, say an int, is atomic, but this does NOT make it thread-safe. Thread-safety also concerns visibility. So even though thread A atomically sets the value of your integer to 42, there is no guarantee that this value will be visible to thread B when it performs an atomic read afterwards. – Janus Varmarken Sep 20 '15 at 09:12
  • 1
    @JanusVarmarken: Re: "It is true that a read of a primitive type, say an int, is atomic": Yes, except for longs and doubles; see https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.7. – ruakh Sep 21 '15 at 05:25
  • Putting the `synchronized` around the loop instead would probably help quite a bit - right now, you're entering and leaving the critical section every iteration of the loop, which is likely going to dominate the runtime entirely for this piece of code. – Luaan Sep 21 '15 at 07:56
  • @assylias: That's not a sufficient reasoning for atomicity, and it's actually at least two reads, possibly even more (read address, read offset, load value, and that twice). The language standard could still mandate atomicity, even for whole expressions or complete loops. – Sebastian Mach Sep 21 '15 at 10:46

4 Answers4

67

No. The += operation is not thread-safe. It requires locking and / or a proper chain of "happens-before" relationships for any expression involving assignment to a shared field or array element to be thread-safe.

(With a field declared as volatile, the "happens-before" relationships exist ... but only on read and write operations. The += operation consists of a read and a write. These are individually atomic, but the sequence isn't. And most assignment expressions using = involve both one or more reads (on the right hand side) and a write. That sequence is not atomic either.)

For the complete story, read JLS 17.4 ... or the relevant chapter of "Java Concurrency in Action" by Brian Goetz et al.

As I know basic operations on primitive types are thread-safe ...

Actually, that is an incorrect premise:

  • consider the case of arrays
  • consider that expressions are typically composed of a sequence of operations, and that a sequence of atomic operations is not guaranteed to be atomic.

There is an additional issue for the double type. The JLS (17.7) says this:

"For the purposes of the Java programming language memory model, a single write to a non-volatile long or double value is treated as two separate writes: one to each 32-bit half. This can result in a situation where a thread sees the first 32 bits of a 64-bit value from one write, and the second 32 bits from another write."

"Writes and reads of volatile long and double values are always atomic."


In a comment, you asked:

So what type I should use to avoid global synchronization, which stops all threads inside this loop?

In this case (where you are updating a double[], there is no alternative to synchronization with locks or primitive mutexes.

If you had an int[] or a long[] you could replace them with AtomicIntegerArray or AtomicLongArray and make use of those classes' lock-free update. However there is no AtomicDoubleArray class, or even an AtomicDouble class.

(UPDATE - someone pointed out that Guava provides an AtomicDoubleArray class, so that would be an option. A good one actually.)

One way of avoiding a "global lock" and massive contention problems might be to divide the array into notional regions, each with its own lock. That way, one thread only needs to block another thread if they are using the same region of the array. (Single writer / multiple reader locks could help too ... if the vast majority of accesses are reads.)

Community
  • 1
  • 1
Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • It would be awesome if you would mention `Atomic`s to your answer. – Hannes Sep 20 '15 at 07:28
  • So what type I should use to avoid global synchronization, which stops all threads inside this loop – Piotr Stapp Sep 20 '15 at 07:29
  • @Hannes - Atomic is not applicable to the OP's use-case (an array of doubles). And it doesn't work with operators like '+=' or '='. It is basically irrelevant to solving the OP's problem. However, feel free to write an Answer yourself :-) – Stephen C Sep 20 '15 at 07:44
  • 6
    Guava has an [`AtomicDoubleArray`](http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/util/concurrent/AtomicDoubleArray.html) class which could be used here to work around the lack of an equivalent in the JDK. Additionally, Java 8 introduced the [`DoubleAdder`](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/atomic/DoubleAdder.html) class, which is basically an atomic `double`. – Mick Mnemonic Sep 20 '15 at 08:16
  • The repeat "synchronized" *inside* the loop is evil. Better have all the threads hand over their `parts` arrays to a single "gatherer" thread which has sole responsibilty for writing to a temporary `result` array and which then replaces the common `result` array in a single operation. If that fits the specification. Also, thanks Stephen for mentioning the [happens before](http://preshing.com/20130702/the-happens-before-relation/) magic words. – David Tonhofer Sep 20 '15 at 08:20
  • 1
    @StephenC I have used += in my multi threaded programs on data members that are volatile. Would that be causing issues? – Arya Sep 20 '15 at 08:38
  • 2
    @Arya Pretty sure that yes. "volatile" tells the JVM to go read/write the latest version of the value from distant memory as opposed to using locally cached versions. See: [A field may be declared volatile, in which case the Java Memory Model ensures that all threads see a consistent value for the variable](https://docs.oracle.com/javase/specs/jls/se8/html/jls-8.html#jls-8.3.1.4) As long as the various threads just _read_ the value written by a single other thread, that's fine. Once they read/modify/write the value in interleaved fashion, all bets are still off. Be safe, program defensively! – David Tonhofer Sep 20 '15 at 09:00
  • 2
    @Arya - Yes it would. Very subtle ones ... like a occasional lost updates. – Stephen C Sep 20 '15 at 09:22
  • 1
    With [Double.toLongbits](http://docs.oracle.com/javase/7/docs/api/java/lang/Double.html#doubleToLongBits(double)) and its complement you can use a AtomicLongArray (which I think guava uses behind the scenes) – ratchet freak Sep 20 '15 at 15:25
7

Despite of the fact that there is no AtomicDouble or AtomicDoubleArray in java, you can easily create your own based on AtomicLongArray.

static class AtomicDoubleArray {
    private final AtomicLongArray inner;

    public AtomicDoubleArray(int length) {
        inner = new AtomicLongArray(length);
    }

    public int length() {
        return inner.length();
    }

    public double get(int i) {
        return Double.longBitsToDouble(inner.get(i));
    }

    public void set(int i, double newValue) {
        inner.set(i, Double.doubleToLongBits(newValue));
    }

    public void add(int i, double delta) {
        long prevLong, nextLong;
        do {
            prevLong = inner.get(i);
            nextLong = Double.doubleToLongBits(Double.longBitsToDouble(prevLong) + delta);
        } while (!inner.compareAndSet(i, prevLong, nextLong));
    }
}

As you can see, I use Double.doubleToLongBits and Double.longBitsToDouble to store Doubles as Longs in AtomicLongArray. They both have the same size in bits, so precision is not lost (except for -NaN, but I don't think it is important).

In Java 8 the implementation of add can be even easier, as you can use accumulateAndGet method of AtomicLongArray that was added in java 1.8.

Upd: It appears that I virtually re-implemented guava's AtomicDoubleArray.

Aivean
  • 10,692
  • 25
  • 39
6

Even the normal 'double' data type is not thread-safe (because it is not atomic) in 32-bit JVMs as it takes eight bytes in Java (which involves 2*32 bit operations).

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Vasu
  • 21,832
  • 11
  • 51
  • 67
  • this is the frist time I heard of this. please explain to me why thread safety in double datatype is dependant on JVM or OS – Sharon Ben Asher Sep 20 '15 at 07:12
  • 1
    You can have a look at this link: http://stackoverflow.com/questions/9278764/are-primitive-datatypes-thread-safe-in-java – Vasu Sep 20 '15 at 07:13
  • 1
    @sharonbn http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.7 @VA31 this answer does not mention other primitives, let alone the `+=` operation. It seems like it should be a comment rather than an answer. – Vince Sep 20 '15 at 07:18
  • 4
    @VinceEmigh - He is right. The JLS says so. See my answer. – Stephen C Sep 20 '15 at 07:26
3

As it's already explained, this code is not thread safe. One possible solution to avoid synchronization in Java-8 is to use new DoubleAdder class which is capable to maintain the sum of double numbers in thread-safe manner.

Create array of DoubleAdder objects before parallelizing:

DoubleAdder[] adders = Stream.generate(DoubleAdder::new)
                             .limit(typeCount).toArray(DoubleAdder[]::new);

Then accumulate the sum in parallel threads like this:

for(int type = 0; type < typeCount; type++) 
    adders[type].add(parts[type]);
}

Finally get the result after parallel subtasks finished:

double[] result = Arrays.stream(adders).mapToDouble(DoubleAdder::sum).toArray();
Tagir Valeev
  • 97,161
  • 19
  • 222
  • 334