Simplify diff sets

neo4j · Aug 29, 2018 · 89b9a93 · 89b9a93
1 parent f15dac2
commit 89b9a93
Show file tree

Hide file tree

Showing 16 changed files with 392 additions and 639 deletions.
diff --git a/...eengine/api/txstate/ReadableDiffSets.java → ...j/storageengine/api/txstate/DiffSets.java b/...eengine/api/txstate/ReadableDiffSets.java → ...j/storageengine/api/txstate/DiffSets.java
@@ -25,27 +25,46 @@
 import java.util.function.Predicate;
 
 /**
- * {@link SuperReadableDiffSets} with added method for filtering added items.
+ * Given a sequence of add and removal operations, instances of DiffSets track
+ * which elements need to actually be added and removed at minimum from some
+ * hypothetical target collection such that the result is equivalent to just
+ * executing the sequence of additions and removals in order
+ *
+ * @param <T> type of elements
  */
-public interface ReadableDiffSets<T> extends SuperReadableDiffSets<T>
+public interface DiffSets<T>
 {
-    @Override
-    ReadableDiffSets<T> filterAdded( Predicate<T> addedFilter );
 
-    final class Empty<T> implements ReadableDiffSets<T>
+    boolean isAdded( T elem );
+
+    boolean isRemoved( T elem );
+
+    Set<T> getAdded();
+
+    Set<T> getRemoved();
+
+    boolean isEmpty();
+
+    Iterator<T> apply( Iterator<? extends T> source );
+
+    int delta();
+
+    DiffSets<T> filterAdded( Predicate<T> addedFilter );
+
+    final class Empty<T> implements DiffSets<T>
     {
         @SuppressWarnings( "unchecked" )
-        public static <T> ReadableDiffSets<T> instance()
+        public static <T> DiffSets<T> instance()
         {
             return INSTANCE;
         }
 
-        public static <T> ReadableDiffSets<T> ifNull( ReadableDiffSets<T> diffSets )
+        public static <T> DiffSets<T> ifNull( DiffSets<T> diffSets )
         {
-            return diffSets == null ? Empty.instance() : diffSets;
+            return diffSets == null ? INSTANCE : diffSets;
         }
 
-        private static final ReadableDiffSets INSTANCE = new Empty();
+        private static final DiffSets INSTANCE = new Empty();
 
         private Empty()
         {
@@ -95,14 +114,9 @@ public int delta()
         }
 
         @Override
-        public ReadableDiffSets<T> filterAdded( Predicate<T> addedFilter )
+        public DiffSets<T> filterAdded( Predicate<T> addedFilter )
         {
             return this;
         }
-
-        @Override
-        public void accept( DiffSetsVisitor<T> visitor )
-        {
-        }
     }
 }
diff --git a/community/kernel-api/src/main/java/org/neo4j/storageengine/api/txstate/NodeState.java b/community/kernel-api/src/main/java/org/neo4j/storageengine/api/txstate/NodeState.java
@@ -26,8 +26,8 @@
 /**
  * Represents the transactional changes to a node:
  * <ul>
- * <li>{@linkplain #labelDiffSets() Labels} that have been {@linkplain ReadableDiffSets#getAdded() added}
+ * <li>{@linkplain #labelDiffSets() Labels} that have been {@linkplain DiffSets#getAdded() added}
- * or {@linkplain ReadableDiffSets#getRemoved() removed}.</li>
+ * or {@linkplain DiffSets#getRemoved() removed}.</li>
  * <li>Added and removed relationships.</li>
  * <li>{@linkplain PropertyContainerState Changes to properties}.</li>
  * </ul>

diff --git a/...ernel-api/src/main/java/org/neo4j/storageengine/api/txstate/ReadableTransactionState.java b/...ernel-api/src/main/java/org/neo4j/storageengine/api/txstate/ReadableTransactionState.java
@@ -90,21 +90,21 @@ public interface ReadableTransactionState
 
     // SCHEMA RELATED
 
-    ReadableDiffSets<IndexDescriptor> indexDiffSetsByLabel( int labelId );
+    DiffSets<IndexDescriptor> indexDiffSetsByLabel( int labelId );
 
-    ReadableDiffSets<IndexDescriptor> indexDiffSetsBySchema( SchemaDescriptor schema );
+    DiffSets<IndexDescriptor> indexDiffSetsBySchema( SchemaDescriptor schema );
 
-    ReadableDiffSets<IndexDescriptor> indexChanges();
+    DiffSets<IndexDescriptor> indexChanges();
 
     Iterable<IndexDescriptor> constraintIndexesCreatedInTx();
 
-    ReadableDiffSets<ConstraintDescriptor> constraintsChanges();
+    DiffSets<ConstraintDescriptor> constraintsChanges();
 
-    ReadableDiffSets<ConstraintDescriptor> constraintsChangesForLabel( int labelId );
+    DiffSets<ConstraintDescriptor> constraintsChangesForLabel( int labelId );
 
-    ReadableDiffSets<ConstraintDescriptor> constraintsChangesForSchema( SchemaDescriptor descriptor );
+    DiffSets<ConstraintDescriptor> constraintsChangesForSchema( SchemaDescriptor descriptor );
 
-    ReadableDiffSets<ConstraintDescriptor> constraintsChangesForRelationshipType( int relTypeId );
+    DiffSets<ConstraintDescriptor> constraintsChangesForRelationshipType( int relTypeId );
 
     Long indexCreatedForConstraint( ConstraintDescriptor constraint );
 

diff --git a/...y/kernel-api/src/main/java/org/neo4j/storageengine/api/txstate/SuperReadableDiffSets.java b/...y/kernel-api/src/main/java/org/neo4j/storageengine/api/txstate/SuperReadableDiffSets.java
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/TxState.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/TxState.java
@@ -44,17 +44,17 @@
 import org.neo4j.kernel.api.txstate.TransactionState;
 import org.neo4j.kernel.impl.util.collection.CollectionsFactory;
 import org.neo4j.kernel.impl.util.collection.OnHeapCollectionsFactory;
-import org.neo4j.kernel.impl.util.diffsets.DiffSets;
+import org.neo4j.kernel.impl.util.diffsets.MutableDiffSets;
+import org.neo4j.kernel.impl.util.diffsets.MutableDiffSetsImpl;
 import org.neo4j.kernel.impl.util.diffsets.MutableLongDiffSets;
 import org.neo4j.kernel.impl.util.diffsets.MutableLongDiffSetsImpl;
 import org.neo4j.storageengine.api.RelationshipDirection;
 import org.neo4j.storageengine.api.RelationshipVisitor;
 import org.neo4j.storageengine.api.schema.IndexDescriptor;
-import org.neo4j.storageengine.api.txstate.DiffSetsVisitor;
+import org.neo4j.storageengine.api.txstate.DiffSets;
 import org.neo4j.storageengine.api.txstate.GraphState;
 import org.neo4j.storageengine.api.txstate.LongDiffSets;
 import org.neo4j.storageengine.api.txstate.NodeState;
-import org.neo4j.storageengine.api.txstate.ReadableDiffSets;
 import org.neo4j.storageengine.api.txstate.RelationshipState;
 import org.neo4j.storageengine.api.txstate.TxStateVisitor;
 import org.neo4j.values.storable.Value;
@@ -90,8 +90,8 @@ public class TxState implements TransactionState, RelationshipVisitor.Home
     private MutableLongObjectMap<String> createdRelationshipTypeTokens;
 
     private GraphStateImpl graphState;
-    private DiffSets<IndexDescriptor> indexChanges;
+    private MutableDiffSets<IndexDescriptor> indexChanges;
-    private DiffSets<ConstraintDescriptor> constraintsChanges;
+    private MutableDiffSets<ConstraintDescriptor> constraintsChanges;
 
     private RemovalsCountingDiffSets nodes;
     private RemovalsCountingDiffSets relationships;
@@ -114,8 +114,7 @@ public TxState( CollectionsFactory collectionsFactory )
     }
 
     @Override
-    public void accept( final TxStateVisitor visitor )
+    public void accept( final TxStateVisitor visitor ) throws ConstraintValidationException, CreateConstraintFailureException
-            throws ConstraintValidationException, CreateConstraintFailureException
     {
         if ( nodes != null )
         {
@@ -173,7 +172,11 @@ public void accept( final TxStateVisitor visitor )
 
         if ( constraintsChanges != null )
         {
-            constraintsChanges.accept( new ConstraintDiffSetsVisitor( visitor ) );
+            for ( ConstraintDescriptor added : constraintsChanges.getAdded() )
+            {
+                visitor.visitAddedConstraint( added );
+            }
+            constraintsChanges.getRemoved().forEach( visitor::visitRemovedConstraint );
         }
 
         if ( createdLabelTokens != null )
@@ -511,7 +514,7 @@ public LongDiffSets nodesWithLabelChanged( long label )
     @Override
     public void indexDoAdd( IndexDescriptor descriptor )
     {
-        DiffSets<IndexDescriptor> diff = indexChangesDiffSets();
+        MutableDiffSets<IndexDescriptor> diff = indexChangesDiffSets();
         if ( !diff.unRemove( descriptor ) )
         {
             diff.add( descriptor );
@@ -533,28 +536,28 @@ public boolean indexDoUnRemove( IndexDescriptor descriptor )
     }
 
     @Override
-    public ReadableDiffSets<IndexDescriptor> indexDiffSetsByLabel( int labelId )
+    public DiffSets<IndexDescriptor> indexDiffSetsByLabel( int labelId )
     {
         return indexChangesDiffSets().filterAdded( SchemaDescriptorPredicates.hasLabel( labelId ) );
     }
 
     @Override
-    public ReadableDiffSets<IndexDescriptor> indexDiffSetsBySchema( SchemaDescriptor schema )
+    public DiffSets<IndexDescriptor> indexDiffSetsBySchema( SchemaDescriptor schema )
     {
         return indexChangesDiffSets().filterAdded( indexDescriptor -> indexDescriptor.schema().equals( schema ) );
     }
 
     @Override
-    public ReadableDiffSets<IndexDescriptor> indexChanges()
+    public DiffSets<IndexDescriptor> indexChanges()
     {
-        return ReadableDiffSets.Empty.ifNull( indexChanges );
+        return DiffSets.Empty.ifNull( indexChanges );
     }
 
-    private DiffSets<IndexDescriptor> indexChangesDiffSets()
+    private MutableDiffSets<IndexDescriptor> indexChangesDiffSets()
     {
         if ( indexChanges == null )
         {
-            indexChanges = new DiffSets<>();
+            indexChanges = new MutableDiffSetsImpl<>();
         }
         return indexChanges;
     }
@@ -638,34 +641,34 @@ public void constraintDoAdd( ConstraintDescriptor constraint )
     }
 
     @Override
-    public ReadableDiffSets<ConstraintDescriptor> constraintsChangesForLabel( int labelId )
+    public DiffSets<ConstraintDescriptor> constraintsChangesForLabel( int labelId )
     {
         return constraintsChangesDiffSets().filterAdded( SchemaDescriptorPredicates.hasLabel( labelId ) );
     }
 
     @Override
-    public ReadableDiffSets<ConstraintDescriptor> constraintsChangesForSchema( SchemaDescriptor descriptor )
+    public DiffSets<ConstraintDescriptor> constraintsChangesForSchema( SchemaDescriptor descriptor )
     {
         return constraintsChangesDiffSets().filterAdded( SchemaDescriptor.equalTo( descriptor ) );
     }
 
     @Override
-    public ReadableDiffSets<ConstraintDescriptor> constraintsChangesForRelationshipType( int relTypeId )
+    public DiffSets<ConstraintDescriptor> constraintsChangesForRelationshipType( int relTypeId )
     {
         return constraintsChangesDiffSets().filterAdded( SchemaDescriptorPredicates.hasRelType( relTypeId ) );
     }
 
     @Override
-    public ReadableDiffSets<ConstraintDescriptor> constraintsChanges()
+    public DiffSets<ConstraintDescriptor> constraintsChanges()
     {
-        return ReadableDiffSets.Empty.ifNull( constraintsChanges );
+        return DiffSets.Empty.ifNull( constraintsChanges );
     }
 
-    private DiffSets<ConstraintDescriptor> constraintsChangesDiffSets()
+    private MutableDiffSets<ConstraintDescriptor> constraintsChangesDiffSets()
     {
         if ( constraintsChanges == null )
         {
-            constraintsChanges = new DiffSets<>();
+            constraintsChanges = new MutableDiffSetsImpl<>();
         }
         return constraintsChanges;
     }
@@ -692,7 +695,7 @@ public Iterable<IndexDescriptor> constraintIndexesCreatedInTx()
     {
         if ( createdConstraintIndexesByConstraint != null && !createdConstraintIndexesByConstraint.isEmpty() )
         {
-            return map( this::getIndexForIndexBackedConstraint, createdConstraintIndexesByConstraint.keySet() );
+            return map( TxState::getIndexForIndexBackedConstraint, createdConstraintIndexesByConstraint.keySet() );
         }
         return Iterables.empty();
     }
@@ -784,7 +787,7 @@ public void indexDoUpdateEntry( SchemaDescriptor descriptor, long nodeId,
         }
     }
 
-    private MutableLongDiffSets getOrCreateIndexUpdatesForSeek( Map<ValueTuple,MutableLongDiffSets> updates, ValueTuple values )
+    private static MutableLongDiffSets getOrCreateIndexUpdatesForSeek( Map<ValueTuple, MutableLongDiffSets> updates, ValueTuple values )
     {
         return updates.computeIfAbsent( values, value -> new MutableLongDiffSetsImpl() );
     }
@@ -807,7 +810,7 @@ private MutableObjectLongMap<IndexBackedConstraintDescriptor> createdConstraintI
         return createdConstraintIndexesByConstraint;
     }
 
-    private IndexDescriptor getIndexForIndexBackedConstraint( IndexBackedConstraintDescriptor constraint )
+    private static IndexDescriptor getIndexForIndexBackedConstraint( IndexBackedConstraintDescriptor constraint )
     {
         return constraint.ownedIndexDescriptor();
     }
@@ -824,30 +827,8 @@ public boolean hasDataChanges()
         return hasDataChanges;
     }
 
-    private static class ConstraintDiffSetsVisitor implements DiffSetsVisitor<ConstraintDescriptor>
-    {
-        private final TxStateVisitor visitor;
-
-        ConstraintDiffSetsVisitor( TxStateVisitor visitor )
-        {
-            this.visitor = visitor;
-        }
-
-        @Override
-        public void visitAdded( ConstraintDescriptor constraint ) throws CreateConstraintFailureException
-        {
-            visitor.visitAddedConstraint( constraint );
-        }
-
-        @Override
-        public void visitRemoved( ConstraintDescriptor constraint )
-        {
-            visitor.visitRemovedConstraint( constraint );
-        }
-    }
-
     /**
-     * This class works around the fact that create-delete in the same transaction is a no-op in {@link DiffSets},
+     * This class works around the fact that create-delete in the same transaction is a no-op in {@link MutableDiffSetsImpl},
      * whereas we need to know total number of explicit removals.
      */
     private class RemovalsCountingDiffSets extends MutableLongDiffSetsImpl