Added the PaddedAtomicLong

[pveentjer]

A padded version of the java.util.atomic.AtomicLong.

[franz1981]

Thanks for the contribution! Let me know when it can be reviewed :)

[nitsanw]

Thanks for the contribution. Please consider the comments. As mentioned this needs some consideration from a compatibility standpoint, or might need to be handling similar to MPSC linked queue CAS method.

[nitsanw]

This means no pre-8 compatibility, which is not necessarily bad, but needs consideration.

[pveentjer]

I had a look at the Java 6 code

 public final long getAndAdd(long delta) {
        while (true) {
            long current = get();
            long next = current + delta;
            if (compareAndSet(current, next))
                return current;
        }
    }

So Unsafe.getAndSetLong has been replaced by something I expect to be a lot slower.

I made quick JMH benchmark:

@Fork(value = 2)
@Warmup(iterations = 1)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@State(Scope.Benchmark)
@Threads(4)
public class GetAndAddBenchmark {

    public AtomicLong a;

    @Setup
    public void setup(){
        a=new AtomicLong();
    }

    @Benchmark
    public long slow() {
        long delta = 10;
        while (true) {
            long current = a.get();
            long next = current + delta;
            if (a.compareAndSet(current, next))
                return current;
        }

    }

    @Benchmark
    public long fast() {
        long delta = 10;
        return a.getAndAdd(delta);
    }
}

And there is a significant performance loss with the slow approach:

Benchmark                     Mode  Cnt    Score    Error  Units
GetAndAddBenchmark.fast       avgt   10  118.111 ±  9.630  ns/op
GetAndAddBenchmark.slow       avgt   10  231.833 ± 30.669  ns/op

I had a peek at the generated Assembly (perfasm) and key difference is that the slow approach does a volatile read and then a 'lock cmpxchg' and the fast approach only needs a 'lock xadd'. The issue is that the former approach can run into a cas failure, while this can't happen with the lock xadd (hence latter approach is more performant).

In other words: there is a very significant performance loss if I would do it in the Java 6 way. Do you see any way to do it in a Java 6 compatible way but without the performance impact? One option would be to add a switch that chooses the appropriate logic based on the existence of Unsafe.getAndSetLong

[pveentjer]

@nitsanw could you have another look?

[coveralls]

Pull Request Test Coverage Report for Build 717

• 48 of 54 (88.89%) changed or added relevant lines in 1 file are covered.
• 30 unchanged lines in 6 files lost coverage.
• Overall coverage decreased (-0.4%) to 85.831%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
jctools-core/src/main/java/org/jctools/util/PaddedAtomicLong.java	48	54	88.89%

Files with Coverage Reduction	New Missed Lines	%
jctools-core/src/main/java/org/jctools/maps/NonBlockingSetInt.java	1	77.09%
jctools-core/src/main/java/org/jctools/queues/MpmcUnboundedXaddArrayQueue.java	2	96.06%
jctools-core/src/main/java/org/jctools/maps/NonBlockingHashMap.java	4	78.38%
jctools-core/src/main/java/org/jctools/maps/NonBlockingHashMapLong.java	6	78.64%
jctools-core/src/main/java/org/jctools/maps/NonBlockingIdentityHashMap.java	6	76.31%
jctools-core/src/main/java/org/jctools/maps/ConcurrentAutoTable.java	11	47.69%

Totals
Change from base Build 716:	-0.4%
Covered Lines:	4822
Relevant Lines:	5618

---

💛 - Coveralls

[franz1981]

@pveentjer
Look at https://github.com/JCTools/JCTools/blob/master/jctools-core/src/main/java/org/jctools/queues/MpscLinkedQueue.java#L43

Probably for java 6 we don't have other options if not to accept it to be slower :)

[nitsanw]

@pveentjer I've pushed to your branch, please have a look, if it's all good please submit a unit test and rebase + squash commits to clean merge.
Sorry for the slow responses, and thanks for your contribution!

[pveentjer]

@nitsanw I'll get my act together today and finish up the work.

Once that is done I'll also add an PaddedAtomicReference.

[pveentjer]

I have added a unit tests and squashed the commits.

[franz1981]

@pveentjer There is a build failure on the CI
A couple of notes:

• getAndSet0 and getAndAdd0 could be copied directly on the callers, to save inlining budget
• it would be nice to improve UnsafeAccess to allow forcing the cmpAndSwap versions of getAndSet, both for benchmarking purposes and to increase coverage while using jdk 8

[nitsanw]

[2019-12-29 11:42:41] [autobuild] [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project jctools-core: Compilation failure: Compilation failure: 
[2019-12-29 11:42:41] [autobuild] [ERROR] /opt/src/jctools-core/src/main/java/org/jctools/util/PaddedAtomicLong.java:[322,25] cannot find symbol
[2019-12-29 11:42:41] [autobuild] [ERROR]   symbol:   variable SUPPORTS_GET_AND_SET
[2019-12-29 11:42:41] [autobuild] [ERROR]   location: class org.jctools.util.UnsafeAccess
[2019-12-29 11:42:41] [autobuild] [ERROR] /opt/src/jctools-core/src/main/java/org/jctools/util/PaddedAtomicLong.java:[322,12] illegal start of type
[2019-12-29 11:42:41] [autobuild] [ERROR] /opt/src/jctools-core/src/main/java/org/jctools/util/PaddedAtomicLong.java:[340,25] cannot find symbol
[2019-12-29 11:42:41] [autobuild] [ERROR]   symbol:   variable SUPPORTS_GET_AND_SET

[nitsanw]

@pveentjer currently the build is failing, looks like you need to rebase on top of latest code and adjust. If you are quick we might be able to ship this with the 3.0 release this week.

[pveentjer]

Could someone have another look at this PR?

I'm not sure of copying the content of the methods is the right approach because it increases code duplication and it isn't trivial code.

[nitsanw]

please use byte padding as per the rest of JCTools

[pveentjer]

Sure. Out of curiosity, is there any difference compared to using longs?

[nitsanw]

from JDK 15 the JVM started reordering fields to fill in gaps between object header and fields, resulting in long based padding fails.

[nitsanw]

The PR fits in with other work, I'm going to merge and work out the remaining bits. Thanks for the help @pveentjer !