VarHandles & LMAX Disruptor
mikeb01 at gmail.com
Fri Jul 31 05:43:09 UTC 2015
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 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.
> > 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
> > 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.
> > Couple of notes:
> > - The Disruptor is not a heavy (ab)user of the Unsafe - there's no off
> > stuff there. The use cases were primarily avoiding the additional costs
> > 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
> > 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
> > normal write/read of a field.
> > Mike.
> >  https://github.com/LMAX-Exchange/disruptor/tree/jdk9/
More information about the valhalla-dev