On Mar 8, 2015, at 4:13 AM, Andrew Haley <a...@redhat.com> wrote: > > On 03/07/2015 09:23 PM, Peter Levart wrote: > >> I see you're not using Unsafe.unalignedAccess() method in Unsafe >> code at all. So the approach to use this information and decide >> whether to use Unsafe.getXXX() or Unsafe.getXXXUnaligned() is left >> to the user of Unsafe API. > > I don't think that there is any reason to keep library code from being > able to discover whether this machine can do unaligned loads. > However, there is now no reason to use Unsafe.getXXX() on an unaligned > word. unalignedAccess() is for tuning, not correctness.
+1 >> That's OK if you think this approach is a better division of >> concerns (Unsafe being low-level), but in Heap-X-Buffer you are only >> using Unsafe.get|putXXXUnaligned() and don't consider using >> get|putXXX directly when unalignedAccess() == true. > > Right, because there's no point in doing so when unalignedAccess() == > true because the exact same code will be generated. In my Utopian > world nothing except Unsafe would have to deal with issues such as > this. However, I think that people writing library code (e.g. array > comparison intrinsics) might still need to know. +1 Unsafe exposes some low-level information like this, rather than hiding it. (For the next level of detail, see https://bugs.openjdk.java.net/browse/JDK-8014710.) > The problem is that in reality unalignedAccess() isn't as simple as a > binary yes/no. For example, on some machines unaligned accessess > might be significantly slower than aligned ones but still work; so, > unalignedAccess() will (?) return true even though it might not be > ideal. Library code is then to have to use some sort of heuristic to > decide whether to use e.g. getLongUnaligned() or getInt() (when > comparing e.g. int arrays). Hardware and OS designers play trapping games sometimes to handle misaligned. That's not on the table now. The present design keeps options open. > I wonder if on all machines it's worth using getLongUnaligned() for > int array comparisons. I don't know. I hope it is, but I suspect it > isn't. Best performance will come from vectorization, and we have a better chance to do this if we have portably-named intrinsics for the unaligned accesses, instead of random looking byte-peek-poke. That's the main benefit from this patch, I think: Having portable names for the unaligned accesses. That's the reason I'm happy to see it. It will require additional work, in the JIT, to optimize fully. > >> In java.nio.Bits, the Unsafe.unalignedAccess() is used to lazily >> initialize the Bits.unaligned static field. This lazy initialization >> is unnecessary now and 'unaligned' field can be made final. > > I agree, but there are quite a few other places which could also be > improved by the use of this API, and every patch must stop somewhere. > This one is the first. We need to capture the information in your head about those quite a few other places, and attach it to a followup RFE. If you want you can dump some notes to email. (Preferably in a message separate from this thread). The compiler vectorization requirement is either a third bug or else part of fulfilling the efficient array comparison intrinsic RFE. Thanks for doing this work. — John
