VarHandles & LMAX Disruptor

[Vitaly Davidovich]

Yes, that looks like range check - can you show the assembly above these
lines?

What does the java code look like for this test?

I think your adjustment to make it compile is fine;
http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/ae45df3285c9 removed
Compile* parameter noting it's unused.

sent from my phone
On Jul 31, 2015 1:43 AM, "Michael Barker" <mikeb01 at gmail.com> wrote:

> 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/
> >
> >
>