Thread-safe implementation of max

as if you didn't have your pick of answers, here's mine:

// while the update appears bigger than the atomic, try to update the atomic.
private void max(AtomicDouble atomicDouble, double update) {
    double expect = atomicDouble.get();
    while (update > expect) {
        atomicDouble.weakCompareAndSet(expect, update);
        expect = atomicDouble.get();
    }
}

it's more or less the same as the accepted answer, but doesn't use break or while(true) which I personally don't like.

EDIT: just discovered DoubleAccumulator in java 8. the documentation even says this is for summary statistics problems like yours:

DoubleAccumulator max = new DoubleAccumulator(Double::max, Double.NEGATIVE_INFINITY);
parallelStream.forEach(max::accumulate);
max.get();

With Java 8 you can take advantage of functional interfaces and a simple lamda expression to solve this with one line and no looping:

private void updateMax(long sample) {
    max.updateAndGet(curMax -> (sample > curMax) ? sample : curMax);
}

The solution uses the updateAndGet(LongUnaryOperator) method. The current value is contained in curMax and using the conditional operator a simple test is performed replacing the current max value with the sample value if the sample value is greater than the current max value.


As of Java 8, LongAccumulator has been introduced. It is advised as

This class is usually preferable to AtomicLong when multiple threads update a common value that is used for purposes such as collecting statistics, not for fine-grained synchronization control. Under low update contention, the two classes have similar characteristics. But under high contention, expected throughput of this class is significantly higher, at the expense of higher space consumption.

You can use it as follows:

LongAccumulator maxId = new LongAccumulator(Long::max, 0); //replace 0 with desired initial value
maxId.accumulate(newValue); //from each thread

I think it's correct, but I'd probably rewrite it a little for clarity, and definitely add comments:

private void updateMax(long sample) {
    while (true) {
        long curMax = max.get();
        if (curMax >= sample) {
            // Current max is higher, so whatever other threads are
            // doing, our current sample can't change max.
            break;
        }

        // Try updating the max value, but only if it's equal to the
        // one we've just seen. We don't want to overwrite a potentially
        // higher value which has been set since our "get" call.
        boolean setSuccessful = max.compareAndSet(curMax, sample);

        if (setSuccessful) {
            // We managed to update the max value; no other threads
            // got in there first. We're definitely done.
            break;
        }

        // Another thread updated the max value between our get and
        // compareAndSet calls. Our sample can still be higher than the
        // new value though - go round and try again.
    }
}

EDIT: Usually I'd at least try the synchronized version first, and only go for this sort of lock-free code when I'd found that it was causing a problem.