Hi Mikael,
Can't really comment on the bit-twiddling details.
A couple of minor style nits:
- don't put "return" on a line by itself, include the first part of the
return expression
- spaces after commas in template definitions/instantiation
The JVM_ENTRY_FROM_LEAF etc was a little mind twisting but seems okay.
Otherwise hotspot and JDK code appear okay.
Thanks,
David
On 3/02/2016 5:25 AM, Mikael Vidstedt wrote:
Please review this change which introduces a Copy::conjoint_swap and an
Unsafe.copySwapMemory method to call it from Java, along with the
necessary changes to have java.nio.Bits call it instead of the Bits.c code.
http://cr.openjdk.java.net/~mikael/webrevs/8141491/webrev.03/hotspot/webrev/
http://cr.openjdk.java.net/~mikael/webrevs/8141491/webrev.03/jdk/webrev/
On the jdk/ side I don't think there should be a lot of surprises.
Bits.c is gone and that required a mapfile-vers to be changed
accordingly. I also added a relatively extensive
jdk/internal/misc/Unsafe/CopySwap.java test which exercises all the
various copySwap configurations and verifies that the resulting data is
correct. There are also a handful of negative tests in there.
On the hotspot/ side:
* the copy logic in copy.cpp is leveraging templates to help the C++
compiler produce tight copy loops for the various configurations
{element type, copy direction, src aligned, dst aligned}.
* Unsafe_CopySwapMemory is a leaf to not stall safe points more than
necessary. Only if needed (THROW, copy involves heap objects) will it
enter VM using a new JVM_ENTRY_FROM_LEAF macro.
* JVM_ENTRY_FROM_LEAF calls a new VM_ENTRY_BASE_FROM_LEAF helper macro,
which mimics what VM_ENTRY_BASE does, but also does a
debug_only(ResetNoHandleMark __rnhm;) - this is because
JVM_LEAF/VM_LEAF_BASE does debug_only(NoHandleMark __hm;).
I'm in the process of getting the last performance numbers, but from
what I've seen so far this will outperform the earlier implementation.
Cheers,
Mikeal
On 2016-01-27 17:13, Mikael Vidstedt wrote:
Just an FYI:
I'm working on moving all of this to the Hotspot Copy class and
bridging to it via jdk.internal.misc.Unsafe, removing Bits.c
altogether. The implementation is working, and the preliminary
performance numbers beat the pants off of any of the suggested Bits.c
implementations (yay!).
I'm currently in the progress of getting some unit tests in place for
it all to make sure it covers all the corner cases and then I'll run
some real benchmarks to see if it actually lives up to the expectations.
Cheers,
Mikael
On 2016-01-26 11:13, John Rose wrote:
On Jan 26, 2016, at 11:08 AM, Andrew Haley <a...@redhat.com> wrote:
On 01/26/2016 07:04 PM, John Rose wrote:
Unsafe.copyMemory bottoms out to Copy::conjoint_memory_atomic.
IMO that's a better starting point than memcpy. Perhaps it can be
given an additional parameter (or overloading) to specify a swap size.
OK, but conjoint_memory_atomic doesn't guarantee that destination
words won't be torn if their source is misaligned: in fact it
guarantees that they will will be.
That's a good point, and argues for a new function with the
stronger guarantee. Actually, it would be perfectly reasonable
to strengthen the guarantee on the existing function. I don't
think anyone will care about the slight performance change,
especially since it is probably favorable. Since it's Unsafe,
they are not supposed to care, either.
— John
