replace `getIntVolatile(Object var1, long var2)` with `getInt(Object var1, long var2)` in the implementation of incrementAndGet() in java

Viewed 108

According the jdk ,the implementation of the method incrementAndGet() of AtomicInteger class is as follows:

public final int incrementAndGet() {
    return unsafe.getAndAddInt(this, valueOffset, 1) + 1;
}
public final int getAndAddInt(Object var1, long var2, int var4) {
    int var5;
    do {
        var5 = this.getIntVolatile(var1, var2);
    } while(!this.compareAndSwapInt(var1, var2, var5, var5 + var4));

    return var5;
}

What happens if the getIntVolatile(Object var1, long var2) method is replaced by getInt(Object var1, long var2) method?

What I think is that the getIntVolatile(Object var1, long var2) method is unnecessary, as the operated variable is a volatile variable,the getInt(Object var1, long var2) is able to read the latest value.

2 Answers

Look at what incrementAndGet does:

return unsafe.getAndAddInt(this, valueOffset, 1) + 1;

unsafe has no idea that at the valueOffset there is a volatile field and it must treat it as such. So, this (I am not using var2 and the like, but the real source code):

v = getIntVolatile(o, offset);

surely looks like protects against some possible re-orderings. Let's say you replace that getIntVolatile with getInt:

 do {
      v = getInt(o, offset);
 } while (!weakCompareAndSetInt(o, offset, v, v + delta));

Since there are no volatile semantics here, the compiler can move the read out of the loop:

 int v = getInt(o, offset);
 do {
 } while (!weakCompareAndSetInt(o, offset, v, v + delta));

If it does that, you can end-up in an infinite loop, pretty trivially.

Plain getXXX/putXXX methods in the Unsafe class, do not take the volatile keyword into account. They always perform a plain load/store, without memory visibility constraints.

Related