VarHandles & LMAX Disruptor

[Michael Barker]

Hi Paul,

I've had a look at the patch that you mentioned and AFAICT it doesn't seem
to be able to eliminate the bounds check, even if I remove the array
padding.  Just to confirm my analysis the assembler around the aaload
instruction looks like:

0x00007f627d6b3aea: cmp    %ebx,%edx
0x00007f627d6b3aec: jae    0x00007f627d6b3e0c
0x00007f627d6b3af2: mov    %r8,%r13
0x00007f627d6b3af5: mov    %r11d,%r8d
0x00007f627d6b3af8: mov    0x10(%rax,%rdx,4),%r11d  ;*aaload

I'm guessing that the cmp/jae pair is the bounds check?

A quick note on the patch, it merged, but didn't compile.  There seemed to
be a signature change on the signature of ConINode::make, so I changed line
1311 in subnode.cpp from 'return ConINode::make(phase->C, 1);' to 'return
ConINode::make(1);'.  That let it compile, but I don't really understand
the code so I'm not sure if it is semantically correct.

Mike.







On 28 July 2015 at 03:58, Paul Sandoz <paul.sandoz at oracle.com> wrote:

> Hi Mike,
>
> Thanks for doing this! LMAX was on my radar to try out as i suspected it
> would likely be very sensitive to any changes, perhaps especially so for
> array bounds checks.
>
> I believe code in RingBuffer was using Unsafe to avoid bounds checks,
> correct? if so I wonder if the patch in
> https://bugs.openjdk.java.net/browse/JDK-8003585 would help with strength
> reducing such checks? I don’t know if that patch also works with a constant
> offset as used in RingBufferFields, but it should work for
> MultiProducerSequencer. The patch should apply cleanly to the VarHandles
> branch in the sandbox if you want to try it out.
>
> Ideally what you also need is a public equivalent of @Contended that also
> supports arrays, that would make the code a little cleaner, but would be a
> separate project :-)
>
>
> On 25 Jul 2015, at 04:29, Michael Barker <mikeb01 at gmail.com> wrote:
>
> > Hi,
> >
> > I've just ported the Disruptor across from Unsafe to VarHandles[0].
> > Initially I ran into a whole bunch of issues, but after some time digging
> > realised that they were all of my own making.  All of my unit tests pass
> > and the performance tests I've run show very similar results.  I think
> > there is a small slowdown (maybe a few %) with VarHandles, but my laptop
> > has a high run to run variance so I can't really be sure until I do some
> > testing on a more stable platform.  Even if that is the case, that level
> is
> > tolerable and I'll most likely release and use internally the VarHandles
> > implementation when JDK9 is available.
> >
> > Excellent work, thanks to Paul Sandoz (and anyone else who worked on the
> > implementation) and Alesky Shiplev for the sandbox instructions.
> >
>
> Also a big thanks to Aleksey who is also working on the implementation and
> performance measurements.
>
> Paul.
>
> > Couple of notes:
> >
> > - The Disruptor is not a heavy (ab)user of the Unsafe - there's no off
> heap
> > stuff there.  The use cases were primarily avoiding the additional costs
> of
> > AtomicIntArray and AtomicLongFieldUpdater.
> > - I'm a big fan of the style of the API where the use of a concurrent
> > operation is visible at the call site.  I think this improves readability
> > and makes it easier to reason about concurrent code.  Having to jump to
> the
> > type declaration to figure out if an assignment operation can affect the
> > visibility/ordering of the code around it increases cognitive load.
> > - I think VarHandle.set/get should be called setRelaxed/getRelaxed as it
> > would make it more obvious to a user and a reader what those methods are
> > going to do.  My initial assumption was that they were no different from
> a
> > normal write/read of a field.
> >
> > Mike.
> >
> > [0] https://github.com/LMAX-Exchange/disruptor/tree/jdk9/
>
>