Reduces List instantiations and their iterators in and around records

- PropertyRecord#deletedRecords was always instantiated now only instantiated on the first deleted dynamic record. - PropertyRecord#blockRecords was a List where looping through them, which was done everytime blocks were accessed, created a new iterator. Now the PropertyRecord itself is an Iterable<PropertyBlock> and Iterator<PropertyBlock> and the blocks kept in a PropertyBlock[] instead. This produces less garbage - Some other places where List objects was temporarily instantiated and only kept a short time inside internal methods. Replaced with something more efficient and less littering.
neo4j · Feb 11, 2015 · 5194c5b · 5194c5b
1 parent 889eac0
commit 5194c5b
Show file tree

Hide file tree

Showing 34 changed files with 308 additions and 105 deletions.
diff --git a/community/consistency-check/src/main/java/org/neo4j/consistency/checking/ChainCheck.java b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/ChainCheck.java
@@ -57,7 +57,7 @@ private static int[] keys( PropertyRecord property )
     {
         int[] toStartWith = new int[ MAX_BLOCK_PER_RECORD_COUNT ];
         int index = 0;
-        for ( PropertyBlock propertyBlock : property.getPropertyBlocks() )
+        for ( PropertyBlock propertyBlock : property )
         {
             toStartWith[index++] = propertyBlock.getKeyIndexId();
         }

diff --git a/...y/consistency-check/src/main/java/org/neo4j/consistency/checking/PropertyRecordCheck.java b/...y/consistency-check/src/main/java/org/neo4j/consistency/checking/PropertyRecordCheck.java
@@ -60,7 +60,7 @@ public void checkChange( PropertyRecord oldRecord, PropertyRecord newRecord,
         // Previously referenced dynamic records should either still be referenced, or be deleted
         Map<Long, PropertyBlock> prevStrings = new HashMap<>();
         Map<Long, PropertyBlock> prevArrays = new HashMap<>();
-        for ( PropertyBlock block : oldRecord.getPropertyBlocks() )
+        for ( PropertyBlock block : oldRecord )
         {
             PropertyType type = block.getType();
             if ( type != null )
@@ -76,7 +76,7 @@ public void checkChange( PropertyRecord oldRecord, PropertyRecord newRecord,
                 }
             }
         }
-        for ( PropertyBlock block : newRecord.getPropertyBlocks() )
+        for ( PropertyBlock block : newRecord )
         {
             PropertyType type = block.getType();
             if ( type != null )
@@ -121,7 +121,7 @@ public void check( PropertyRecord record,
         {
             field.checkConsistency( record, engine, records );
         }
-        for ( PropertyBlock block : record.getPropertyBlocks() )
+        for ( PropertyBlock block : record )
         {
             checkDataBlock( block, engine, records );
         }

diff --git a/...unity/consistency-check/src/main/java/org/neo4j/consistency/checking/full/OwnerCheck.java b/...unity/consistency-check/src/main/java/org/neo4j/consistency/checking/full/OwnerCheck.java
@@ -55,6 +55,7 @@
 import org.neo4j.kernel.impl.store.record.TokenRecord;
 
 import static java.util.Collections.unmodifiableMap;
+
 import static org.neo4j.consistency.RecordType.ARRAY_PROPERTY;
 import static org.neo4j.consistency.RecordType.PROPERTY_KEY_NAME;
 import static org.neo4j.consistency.RecordType.RELATIONSHIP_TYPE_NAME;
@@ -212,8 +213,7 @@ public void check( PropertyRecord record,
                     }
                     if ( dynamics != null )
                     {
-                        List<PropertyBlock> blocks = record.getPropertyBlocks();
-                        for ( PropertyBlock block : blocks )
+                        for ( PropertyBlock block : record )
                         {
                             RecordType type = recordType( block.forceGetType() );
                             if ( type != null )

diff --git a/...y/consistency-check/src/main/java/org/neo4j/consistency/checking/full/PropertyReader.java b/...y/consistency-check/src/main/java/org/neo4j/consistency/checking/full/PropertyReader.java
@@ -52,7 +52,10 @@ public List<PropertyBlock> propertyBlocks( NodeRecord nodeRecord )
         List<PropertyBlock> propertyBlocks = new ArrayList<>();
         for ( PropertyRecord record : records )
         {
-            propertyBlocks.addAll( record.getPropertyBlocks() );
+            for ( PropertyBlock block : record )
+            {
+                propertyBlocks.add( block );
+            }
         }
         return propertyBlocks;
     }

diff --git a/community/consistency-check/src/main/java/org/neo4j/consistency/store/DiffStore.java b/community/consistency-check/src/main/java/org/neo4j/consistency/store/DiffStore.java
@@ -150,7 +150,7 @@ record = getPropertyStore().forceGetRaw( record );
 
     private void updateDynamic( PropertyRecord record )
     {
-        for ( PropertyBlock block : record.getPropertyBlocks() )
+        for ( PropertyBlock block : record )
         {
             updateDynamic( block.getValueRecords() );
         }

diff --git a/...nel/src/main/java/org/neo4j/kernel/impl/api/index/PropertyPhysicalToLogicalConverter.java b/...nel/src/main/java/org/neo4j/kernel/impl/api/index/PropertyPhysicalToLogicalConverter.java
@@ -128,7 +128,7 @@ private long equalCheck( long nodeId, long expectedNodeId )
 
     private void mapBlocks( PropertyRecord record, Map<Integer, PropertyBlock> blocks )
     {
-        for ( PropertyBlock block : record.getPropertyBlocks() )
+        for ( PropertyBlock block : record )
         {
             blocks.put( block.getKeyIndexId(), block );
         }

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/DiskLayer.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/DiskLayer.java
@@ -611,7 +611,7 @@ private Iterator<DefinedProperty> loadAllPropertiesOf( PrimitiveRecord primitive
         List<DefinedProperty> properties = new ArrayList<>();
         for ( PropertyRecord record : records )
         {
-            for ( PropertyBlock block : record.getPropertyBlocks() )
+            for ( PropertyBlock block : record )
             {
                 properties.add( block.getType().readProperty( block.getKeyIndexId(), block, propertyStoreProvider ) );
             }

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/DynamicArrayStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/DynamicArrayStore.java
@@ -22,10 +22,8 @@
 import java.io.File;
 import java.lang.reflect.Array;
 import java.nio.ByteBuffer;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
-import java.util.List;
 
 import org.neo4j.helpers.Pair;
 import org.neo4j.io.fs.FileSystemAbstraction;
@@ -133,12 +131,13 @@ public static void allocateFromNumbers( Collection<DynamicRecord> target, Object
     private static void allocateFromString( Collection<DynamicRecord> target, String[] array,
             Iterator<DynamicRecord> recordsToUseFirst, DynamicRecordAllocator recordAllocator )
     {
-        List<byte[]> stringsAsBytes = new ArrayList<>();
+        byte[][] stringsAsBytes = new byte[array.length][];
         int totalBytesRequired = STRING_HEADER_SIZE; // 1b type + 4b array length
-        for ( String string : array )
+        for ( int i = 0; i < array.length; i++ )
         {
+            String string = array[i];
             byte[] bytes = PropertyStore.encodeString( string );
-            stringsAsBytes.add( bytes );
+            stringsAsBytes[i] = bytes;
             totalBytesRequired += 4/*byte[].length*/ + bytes.length;
         }
 

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/PropertyStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/PropertyStore.java
@@ -213,7 +213,7 @@ private void updateRecord( PropertyRecord record, PageCursor cursor )
 
             // Then go through the blocks
             int longsAppended = 0; // For marking the end of blocks
-            for ( PropertyBlock block : record.getPropertyBlocks() )
+            for ( PropertyBlock block : record )
             {
                 long[] propBlockValues = block.getValueBlocks();
                 for ( long propBlockValue : propBlockValues )
@@ -242,7 +242,7 @@ private void updatePropertyBlocks( PropertyRecord record )
         if ( record.inUse() )
         {
             // Go through the blocks
-            for ( PropertyBlock block : record.getPropertyBlocks() )
+            for ( PropertyBlock block : record )
             {
                 /*
                  * For each block we need to update its dynamic record chain if
@@ -501,14 +501,14 @@ public static void encodeValue( PropertyBlock block, int keyId, Object value,
 
             // Fall back to dynamic string store
             byte[] encodedString = encodeString( string );
-            Collection<DynamicRecord> valueRecords = new ArrayList<>();
+            List<DynamicRecord> valueRecords = new ArrayList<>();
             allocateStringRecords( valueRecords, encodedString, stringAllocator );
             setSingleBlockValue( block, keyId, PropertyType.STRING, first( valueRecords ).getId() );
             for ( DynamicRecord valueRecord : valueRecords )
             {
                 valueRecord.setType( PropertyType.STRING.intValue() );
-                block.addValueRecord( valueRecord );
             }
+            block.setValueRecords( valueRecords );
         }
         else if ( value instanceof Integer )
         {
@@ -560,14 +560,14 @@ else if ( value.getClass().isArray() )
             }
 
             // Fall back to dynamic array store
-            Collection<DynamicRecord> arrayRecords = new ArrayList<>();
+            List<DynamicRecord> arrayRecords = new ArrayList<>();
             allocateArrayRecords( arrayRecords, value, arrayAllocator );
             setSingleBlockValue( block, keyId, PropertyType.ARRAY, first( arrayRecords ).getId() );
             for ( DynamicRecord valueRecord : arrayRecords )
             {
                 valueRecord.setType( PropertyType.ARRAY.intValue() );
-                block.addValueRecord( valueRecord );
             }
+            block.setValueRecords( arrayRecords );
         }
         else
         {

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/record/PropertyBlock.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/record/PropertyBlock.java
@@ -21,6 +21,7 @@
 
 import java.lang.reflect.Array;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
@@ -36,7 +37,7 @@ public class PropertyBlock implements Cloneable
     private static final long KEY_BITMASK = 0xFFFFFFL;
 
     private static final int MAX_ARRAY_TOSTRING_SIZE = 4;
-    private final List<DynamicRecord> valueRecords = new LinkedList<>();
+    private List<DynamicRecord> valueRecords;
     private long[] valueBlocks;
 
     public PropertyType getType()
@@ -70,17 +71,30 @@ public void setSingleBlock( long value )
     {
         valueBlocks = new long[1];
         valueBlocks[0] = value;
-        valueRecords.clear();
+        if ( valueRecords != null )
+        {
+            valueRecords.clear();
+        }
     }
 
     public void addValueRecord( DynamicRecord record )
     {
+        if ( valueRecords == null )
+        {
+            valueRecords = new LinkedList<>();
+        }
         valueRecords.add( record );
     }
 
+    public void setValueRecords( List<DynamicRecord> valueRecords )
+    {
+        assert this.valueRecords == null || this.valueRecords.isEmpty() : this.valueRecords.toString();
+        this.valueRecords = valueRecords;
+    }
+
     public List<DynamicRecord> getValueRecords()
     {
-        return valueRecords;
+        return valueRecords != null ? valueRecords : Collections.<DynamicRecord>emptyList();
     }
 
     public long getSingleValueBlock()
@@ -118,7 +132,7 @@ public long[] getValueBlocks()
 
     public boolean isLight()
     {
-        return valueRecords.isEmpty();
+        return valueRecords == null || valueRecords.isEmpty();
     }
 
     public void setValueBlocks( long[] blocks )
@@ -127,7 +141,10 @@ public void setValueBlocks( long[] blocks )
         assert ( blocks == null || blocks.length <= expectedPayloadSize) : (
                 "I was given an array of size " + blocks.length +", but I wanted it to be " + expectedPayloadSize );
         this.valueBlocks = blocks;
-        valueRecords.clear();
+        if ( valueRecords != null )
+        {
+            valueRecords.clear();
+        }
     }
 
     /**
@@ -212,9 +229,12 @@ public PropertyBlock clone()
         {
             result.valueBlocks = valueBlocks.clone();
         }
-        for ( DynamicRecord valueRecord : valueRecords )
+        if ( valueRecords != null )
         {
-            result.valueRecords.add( valueRecord.clone() );
+            for ( DynamicRecord valueRecord : valueRecords )
+            {
+                result.addValueRecord( valueRecord.clone() );
+            }
         }
         return result;
     }