Respects aligned memory access constraint (#8329)

* Respects aligned memory access constraint

specifically in off-heap versions of primitive-collections and NumberArray.
UnsafeUtil provides means of knowing about aligned memory access constraint,
but doesn't automatically conforms access by it. Need for aligned memory access
is hardware dependent.

Currently the muninn page cache has some logic for this. Other users of
UnsafeUtil are the primitive-collections and NumberArray. They have different
constraints themselves than the page cache do and so they have been changed
to conform to alignment constraints, but can get away with doing less adaptations
than the page cache does. This is the primary reason UnsafeUtil doesn't do
these things automatically, because there are different optimizations that
apply to different use cases.

Original issue came up when noticing that using primitive-collections or
NumberArray on Solaris could crash the JVM.

* Fixed some typos and better method naming in UnsafeUtil

community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/OffHeapByteArray.java

@@ -98,28 +98,54 @@ public byte getByte( long index, int offset )
     @Override
     public short getShort( long index, int offset )
     {
-        return UnsafeUtil.getShort( address( index, offset ) );
+        return getShort( address( index, offset ) );
+    }
+
+    private short getShort( long p )
+    {
+        if ( UnsafeUtil.allowUnalignedMemoryAccess )
+        {
+            return UnsafeUtil.getShort( p );
+        }
+
+        return UnsafeUtil.getShortByteWiseLittleEndian( p );
     }
 
     @Override
     public int getInt( long index, int offset )
     {
-        return UnsafeUtil.getInt( address( index, offset ) );
+        return getInt( address( index, offset ) );
+    }
+
+    private int getInt( long p )
+    {
+        if ( UnsafeUtil.allowUnalignedMemoryAccess )
+        {
+            return UnsafeUtil.getInt( p );
+        }
+
+        return UnsafeUtil.getIntByteWiseLittleEndian( p );
     }
 
     @Override
     public long get6ByteLong( long index, int offset )
     {
         long address = address( index, offset );
-        long low4b = (UnsafeUtil.getInt( address )) & 0xFFFFFFFFL;
-        long high2b = UnsafeUtil.getShort( address + Integer.BYTES );
+        long low4b = getInt( address ) & 0xFFFFFFFFL;
+        long high2b = getShort( address + Integer.BYTES );
         return low4b | (high2b << 32);
     }
 
     @Override
     public long getLong( long index, int offset )
     {
-        return UnsafeUtil.getLong( address( index, offset ) );
+        long p = address( index, offset );
+        if ( UnsafeUtil.allowUnalignedMemoryAccess )
+        {
+            return UnsafeUtil.getLong( p );
+        }
+
+        return UnsafeUtil.getLongByteWiseLittleEndian( p );
     }
 
     @Override
@@ -141,27 +167,59 @@ public void setByte( long index, int offset, byte value )
     @Override
     public void setShort( long index, int offset, short value )
     {
-        UnsafeUtil.putShort( address( index, offset ), value );
+        putShort( address( index, offset ), value );
+    }
+
+    private void putShort( long p, short value )
+    {
+        if ( UnsafeUtil.allowUnalignedMemoryAccess )
+        {
+            UnsafeUtil.putShort( p, value );
+        }
+        else
+        {
+            UnsafeUtil.putShortByteWiseLittleEndian( p, value );
+        }
     }
 
     @Override
     public void setInt( long index, int offset, int value )
     {
-        UnsafeUtil.putInt( address( index, offset ), value );
+        putInt( address( index, offset ), value );
+    }
+
+    private void putInt( long p, int value )
+    {
+        if ( UnsafeUtil.allowUnalignedMemoryAccess )
+        {
+            UnsafeUtil.putInt( p, value );
+        }
+        else
+        {
+            UnsafeUtil.putIntByteWiseLittleEndian( p, value );
+        }
     }
 
     @Override
     public void set6ByteLong( long index, int offset, long value )
     {
         long address = address( index, offset );
-        UnsafeUtil.putInt( address, (int) value );
-        UnsafeUtil.putShort( address + Integer.BYTES, (short) (value >>> 32) );
+        putInt( address, (int) value );
+        putShort( address + Integer.BYTES, (short) (value >>> 32) );
     }
 
     @Override
     public void setLong( long index, int offset, long value )
     {
-        UnsafeUtil.putLong( address( index, offset ), value );
+        long p = address( index, offset );
+        if ( UnsafeUtil.allowUnalignedMemoryAccess )
+        {
+            UnsafeUtil.putLong( p, value );
+        }
+        else
+        {
+            UnsafeUtil.putLongByteWiseLittleEndian( p, value );
+        }
     }
 
     private long address( long index, int offset )

community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/OffHeapNumberArray.java

@@ -23,6 +23,7 @@
 
 public abstract class OffHeapNumberArray<N extends NumberArray<N>> extends BaseNumberArray<N>
 {
+    private final long allocatedAddress;
     protected final long address;
     protected final long length;
     private boolean closed;
@@ -32,7 +33,23 @@ protected OffHeapNumberArray( long length, int itemSize, long base )
         super( itemSize, base );
         UnsafeUtil.assertHasUnsafe();
         this.length = length;
-        this.address = UnsafeUtil.allocateMemory( length * itemSize );
+
+        long dataSize = length * itemSize;
+        boolean itemSizeIsPowerOfTwo = Integer.bitCount( itemSize ) == 1;
+        if ( UnsafeUtil.allowUnalignedMemoryAccess || !itemSizeIsPowerOfTwo )
+        {
+            // we can end up here even if we require aligned memory access. Reason is that item size
+            // isn't power of two anyway and so we have to fallback to safer means of accessing the memory,
+            // i.e. byte for byte.
+            this.allocatedAddress = this.address = UnsafeUtil.allocateMemory( dataSize );
+        }
+        else
+        {
+            // the item size is a power of two and we're required to access memory aligned
+            // so we can allocate a bit more to ensure we can get an aligned memory address to start from.
+            this.allocatedAddress = UnsafeUtil.allocateMemory( dataSize + itemSize - 1 );
+            this.address = UnsafeUtil.alignedMemory( allocatedAddress, itemSize );
+        }
     }
 
     @Override
@@ -55,7 +72,7 @@ public void close()
             if ( length > 0 )
             {
                 // Allocating 0 bytes actually returns address 0
-                UnsafeUtil.free( address );
+                UnsafeUtil.free( allocatedAddress );
             }
             closed = true;
         }

community/primitive-collections/src/main/java/org/neo4j/collection/primitive/hopscotch/LongKeyLongValueUnsafeTable.java

@@ -19,8 +19,6 @@
  */
 package org.neo4j.collection.primitive.hopscotch;
 
-import org.neo4j.unsafe.impl.internal.dragons.UnsafeUtil;
-
 public class LongKeyLongValueUnsafeTable extends UnsafeTable<long[]>
 {
     public LongKeyLongValueUnsafeTable( int capacity )
@@ -31,30 +29,30 @@ public LongKeyLongValueUnsafeTable( int capacity )
     @Override
     protected long internalKey( long keyAddress )
     {
-        return UnsafeUtil.getLong( keyAddress );
+        return alignmentSafeGetLongAsTwoInts( keyAddress );
     }
 
     @Override
     protected void internalPut( long keyAddress, long key, long[] value )
     {
-        UnsafeUtil.putLong( keyAddress, key );
-        UnsafeUtil.putLong( keyAddress+8, value[0] );
+        alignmentSafePutLongAsTwoInts( keyAddress, key );
+        alignmentSafePutLongAsTwoInts( keyAddress + 8, value[0] );
     }
 
     @Override
     protected long[] internalRemove( long keyAddress )
     {
-        valueMarker[0] = UnsafeUtil.getLong( keyAddress+8 );
-        UnsafeUtil.putLong( keyAddress, -1 );
+        valueMarker[0] = alignmentSafeGetLongAsTwoInts( keyAddress+8 );
+        alignmentSafePutLongAsTwoInts( keyAddress, -1 );
         return valueMarker;
     }
 
     @Override
     public long[] putValue( int index, long[] value )
     {
         long valueAddress = valueAddress( index );
-        long oldValue = UnsafeUtil.getLong( valueAddress );
-        UnsafeUtil.putLong( valueAddress, value[0] );
+        long oldValue = alignmentSafeGetLongAsTwoInts( valueAddress );
+        alignmentSafePutLongAsTwoInts( valueAddress, value[0] );
         return pack( oldValue );
     }
 
@@ -72,7 +70,7 @@ private long valueAddress( int index )
     @Override
     public long[] value( int index )
     {
-        long value = UnsafeUtil.getLong( valueAddress( index ) );
+        long value = alignmentSafeGetLongAsTwoInts( valueAddress( index ) );
         return pack( value );
     }
 

community/primitive-collections/src/main/java/org/neo4j/collection/primitive/hopscotch/LongKeyUnsafeTable.java

@@ -19,8 +19,6 @@
  */
 package org.neo4j.collection.primitive.hopscotch;
 
-import org.neo4j.unsafe.impl.internal.dragons.UnsafeUtil;
-
 public class LongKeyUnsafeTable<VALUE> extends UnsafeTable<VALUE>
 {
     public LongKeyUnsafeTable( int capacity, VALUE valueMarker )
@@ -31,13 +29,13 @@ public LongKeyUnsafeTable( int capacity, VALUE valueMarker )
     @Override
     protected long internalKey( long keyAddress )
     {
-        return UnsafeUtil.getLong( keyAddress );
+        return alignmentSafeGetLongAsTwoInts( keyAddress );
     }
 
     @Override
     protected void internalPut( long keyAddress, long key, VALUE value )
     {
-        UnsafeUtil.putLong( keyAddress, key );
+        alignmentSafePutLongAsTwoInts( keyAddress, key );
     }
 
     @Override

community/primitive-collections/src/main/java/org/neo4j/collection/primitive/hopscotch/UnsafeTable.java

@@ -19,14 +19,16 @@
  */
 package org.neo4j.collection.primitive.hopscotch;
 
-
 import org.neo4j.unsafe.impl.internal.dragons.UnsafeUtil;
 
 public abstract class UnsafeTable<VALUE> extends PowerOfTwoQuantizedTable<VALUE>
 {
     private final int bytesPerKey;
     private final int bytesPerEntry;
     private final long dataSize;
+    // address which should be free when closing
+    private final long allocatedAddress;
+    // address which should be used to access the table, the address where the table actually starts at
     private final long address;
     protected final VALUE valueMarker;
 
@@ -38,7 +40,39 @@ protected UnsafeTable( int capacity, int bytesPerKey, VALUE valueMarker )
         this.bytesPerEntry = 4+bytesPerKey;
         this.valueMarker = valueMarker;
         this.dataSize = (long)this.capacity*bytesPerEntry;
-        this.address = UnsafeUtil.allocateMemory( dataSize );
+
+        // Below is a piece of code which ensures that allocated memory is aligned to 4-byte boundary
+        // if memory system requires aligned memory access. The reason we pick 4-byte boundary is that
+        // it's the lowest common denominator and the size of our hop-bits field for every entry.
+        // So even for a table which would only deal with, say longs (8-byte), it would still need to
+        // read and write 4-byte hop-bits fields. Therefore this table can, if required to, read anything
+        // bigger than 4-byte fields as multiple 4-byte fields. This way it can play well with aligned
+        // memory access requirements.
+
+        assert bytesPerEntry % Integer.BYTES == 0 : "Bytes per entry needs to be divisible by 4, this constraint " +
+                "is checked because on memory systems requiring aligned memory access this would otherwise break.";
+
+        if ( UnsafeUtil.allowUnalignedMemoryAccess )
+        {
+            this.allocatedAddress = this.address = UnsafeUtil.allocateMemory( dataSize );
+        }
+        else
+        {
+            // There's an assertion above also verifying this, but it's only an actual problem if our memory system
+            // requires aligned access, which seems to be the case right here and now.
+            if ( (bytesPerEntry % Integer.BYTES) != 0 )
+            {
+                throw new IllegalArgumentException( "Memory system requires aligned memory access and " +
+                        getClass().getSimpleName() + " was designed to cope with this requirement by " +
+                        "being able to accessing data in 4-byte chunks, if needed to. " +
+                        "Although this table tried to be constructed with bytesPerKey:" + bytesPerKey +
+                        " yielding a bytesPerEntry:" + bytesPerEntry + ", which isn't 4-byte aligned." );
+            }
+
+            this.allocatedAddress = UnsafeUtil.allocateMemory( dataSize + Integer.BYTES - 1 );
+            this.address = UnsafeUtil.alignedMemory( allocatedAddress, Integer.BYTES );
+        }
+
         clearMemory();
     }
 
@@ -156,6 +190,33 @@ public void removeHopBit( int index, int hd )
     @Override
     public void close()
     {
-        UnsafeUtil.free( address );
+        UnsafeUtil.free( allocatedAddress );
+    }
+
+    protected static void alignmentSafePutLongAsTwoInts( long address, long value )
+    {
+        if ( UnsafeUtil.allowUnalignedMemoryAccess )
+        {
+            UnsafeUtil.putLong( address, value );
+        }
+        else
+        {
+            // See javadoc in constructor as to why we do this
+            UnsafeUtil.putInt( address, (int) value );
+            UnsafeUtil.putInt( address + Integer.BYTES, (int) (value >>> Integer.SIZE) );
+        }
+    }
+
+    protected static long alignmentSafeGetLongAsTwoInts( long address )
+    {
+        if ( UnsafeUtil.allowUnalignedMemoryAccess )
+        {
+            return UnsafeUtil.getLong( address );
+        }
+
+        // See javadoc in constructor as to why we do this
+        long lsb = UnsafeUtil.getInt( address ) & 0xFFFFFFFFL;
+        long msb = UnsafeUtil.getInt( address + Integer.BYTES ) & 0xFFFFFFFFL;
+        return lsb | (msb << Integer.SIZE);
     }
 }

community/primitive-collections/src/test/java/org/neo4j/collection/primitive/hopscotch/BasicTableTest.java

@@ -25,18 +25,25 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Random;
 
 import org.neo4j.collection.primitive.Primitive;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assume.assumeTrue;
+
+import static java.lang.System.currentTimeMillis;
+
 import static org.neo4j.collection.primitive.Primitive.VALUE_MARKER;
 
 @RunWith( Parameterized.class )
 public class BasicTableTest
 {
     private final TableFactory factory;
 
+    private static final long seed = currentTimeMillis();
+    private static final Random random = new Random( seed );
+
     @Parameterized.Parameters
     public static Collection<Object[]> data()
     {
@@ -138,7 +145,7 @@ public boolean supportsLongs()
             @Override
             public Object sampleValue()
             {
-                return new int[] {4};
+                return new int[] {random.nextInt( Integer.MAX_VALUE )};
             }
         } } );
         result.add( new Object[] { new TableFactory()
@@ -158,7 +165,7 @@ public boolean supportsLongs()
             @Override
             public Object sampleValue()
             {
-                return new long[] {1458489572354L};
+                return new long[] {Math.abs( random.nextLong() )};
             }
         } } );
         result.add( new Object[] { new TableFactory()
@@ -178,7 +185,7 @@ public boolean supportsLongs()
             @Override
             public Object sampleValue()
             {
-                return new long[] {1458489572354L};
+                return new long[] {Math.abs( random.nextLong() )};
             }
         } } );
         return result;