VarHandles & LMAX Disruptor
vitalyd at gmail.com
Fri Jul 31 11:22:01 UTC 2015
Yes, that looks like range check - can you show the assembly above these
What does the java code look like for this test?
I think your adjustment to make it compile is fine;
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.
> 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
> > reducing such checks? I don’t know if that patch also works with a
> > 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
> > 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.
> > > Initially I ran into a whole bunch of issues, but after some time
> > > realised that they were all of my own making. All of my unit tests
> > > 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
> > > has a high run to run variance so I can't really be sure until I do
> > > testing on a more stable platform. Even if that is the case, that
> > is
> > > tolerable and I'll most likely release and use internally the
> > > implementation when JDK9 is available.
> > >
> > > Excellent work, thanks to Paul Sandoz (and anyone else who worked on
> > > implementation) and Alesky Shiplev for the sandbox instructions.
> > >
> > Also a big thanks to Aleksey who is also working on the implementation
> > 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
> > 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
> > > 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
> > > visibility/ordering of the code around it increases cognitive load.
> > > - I think VarHandle.set/get should be called setRelaxed/getRelaxed as
> > > would make it more obvious to a user and a reader what those methods
> > > going to do. My initial assumption was that they were no different
> > a
> > > normal write/read of a field.
> > >
> > > Mike.
> > >
> > >  https://github.com/LMAX-Exchange/disruptor/tree/jdk9/
More information about the valhalla-dev