Skip to content

Commit

Permalink
Merge pull request #11716 from pontusmelke/3.5-q23-regession
Browse files Browse the repository at this point in the history
Minor performance improvements
  • Loading branch information
systay committed May 9, 2018
2 parents 33a2690 + dac07c1 commit 84395d2
Show file tree
Hide file tree
Showing 18 changed files with 141 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public static boolean nodeHasLabel( Read read, NodeCursor nodeCursor, long node,
}
singleNode( read, nodeCursor, node );

return nodeCursor.labels().contains( label );
return nodeCursor.hasLabel( label );
}

public static RelationshipSelectionCursor nodeGetRelationships( Read read, CursorFactory cursors, NodeCursor node,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ final class TransactionBoundQueryContext(tc: TransactionalContextWrapper, val re
val cursor = nodeCursor
reads().singleNode(node, cursor)
if (!cursor.next()) false
else cursor.labels().contains(label)
else cursor.hasLabel(label)
}

def getOrCreateLabelId(labelName: String) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import org.neo4j.cypher.internal.compiler.v3_1.spi._
import org.neo4j.cypher.internal.frontend.v3_1.SemanticDirection.{BOTH, INCOMING, OUTGOING}
import org.neo4j.cypher.internal.frontend.v3_1.{Bound, EntityNotFoundException, FailedIndexException, SemanticDirection}
import org.neo4j.cypher.internal.javacompat.GraphDatabaseCypherService
import org.neo4j.cypher.internal.runtime.interpreted.{JavaConversionSupport, ResourceManager}
import org.neo4j.cypher.internal.runtime.interpreted.ResourceManager
import org.neo4j.cypher.internal.spi.v3_1.TransactionBoundQueryContext.IndexSearchMonitor
import org.neo4j.cypher.internal.spi.{CursorIterator, PrimitiveCursorIterator}
import org.neo4j.graphalgo.impl.path.ShortestPath
Expand Down Expand Up @@ -176,7 +176,7 @@ final class TransactionBoundQueryContext(txContext: TransactionalContextWrapper,
val cursor = nodeCursor
reads().singleNode(node, cursor)
if (!cursor.next()) false
else cursor.labels().contains(label)
else cursor.hasLabel(label)
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import org.neo4j.cypher.internal.codegen.CompiledCursorUtils.{nodeGetProperty, n
import org.neo4j.cypher.internal.util.v3_5.test_helpers.CypherFunSuite
import org.neo4j.internal.kernel.api.exceptions.EntityNotFoundException
import org.neo4j.internal.kernel.api.{NodeCursor, PropertyCursor, Read, RelationshipScanCursor}
import org.neo4j.kernel.impl.newapi.Labels
import org.neo4j.values.storable.Values.{NO_VALUE, stringValue}

class CompiledCursorUtilsTest extends CypherFunSuite {
Expand Down Expand Up @@ -113,7 +112,8 @@ class CompiledCursorUtilsTest extends CypherFunSuite {
// Given
val nodeCursor = mock[NodeCursor]
when(nodeCursor.next()).thenReturn(true)
when(nodeCursor.labels()).thenReturn(Labels.from(Array(1337L, 42L, 13L)))
when(nodeCursor.hasLabel(1337)).thenReturn(true)
when(nodeCursor.hasLabel(1980)).thenReturn(false)

// Then
nodeHasLabel(mock[Read], nodeCursor, 1L, 1337) shouldBe true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ sealed class TransactionBoundQueryContext(val transactionalContext: Transactiona

val context = transactionalContext.tc.asInstanceOf[Neo4jTransactionalContext]
val newTx = transactionalContext.graph.beginTransaction(context.transactionType, context.securityContext)
val neo4jTransactionalContext = context.copyFrom(context.graph, guard, statementProvider, locker, newTx, statementProvider.get(), query)
val neo4jTransactionalContext = context.copyFrom(context.graph, guard, statementProvider, locker, newTx,
statementProvider.get(), query)
new TransactionBoundQueryContext(TransactionalContextWrapper(neo4jTransactionalContext))
}

Expand Down Expand Up @@ -163,10 +164,13 @@ sealed class TransactionBoundQueryContext(val transactionalContext: Transactiona


override def isLabelSetOnNode(label: Int, node: Long): Boolean = {
val cursor = nodeCursor
reads().singleNode(node, cursor)
if (!cursor.next()) false
else cursor.labels().contains(label)
if (label == StatementConstants.NO_SUCH_LABEL) false
else {
val cursor = nodeCursor
reads().singleNode(node, cursor)
if (!cursor.next()) false
else cursor.hasLabel(label)
}
}

override def getOrCreateLabelId(labelName: String): Int = {
Expand All @@ -179,13 +183,15 @@ sealed class TransactionBoundQueryContext(val transactionalContext: Transactiona
types: Option[Array[Int]]): Iterator[RelationshipValue] = {
val read = reads()
val cursor = nodeCursor
val cursors = transactionalContext.cursors

read.singleNode(node, cursor)
if (!cursor.next())Iterator.empty
if (!cursor.next()) Iterator.empty
else {
val selectionCursor = dir match {
case OUTGOING => outgoingCursor(transactionalContext.kernelTransaction.cursors(), cursor, types.orNull)
case INCOMING => incomingCursor(transactionalContext.kernelTransaction.cursors(), cursor, types.orNull)
case BOTH => allCursor(transactionalContext.kernelTransaction.cursors(), cursor, types.orNull)
case OUTGOING => outgoingCursor(cursors, cursor, types.orNull)
case INCOMING => incomingCursor(cursors, cursor, types.orNull)
case BOTH => allCursor(cursors, cursor, types.orNull)
}
new CursorIterator[RelationshipValue] {
override protected def close(): Unit = selectionCursor.close()
Expand All @@ -204,13 +210,14 @@ sealed class TransactionBoundQueryContext(val transactionalContext: Transactiona
types: Option[Array[Int]]): RelationshipIterator = {
val read = reads()
val cursor = nodeCursor
val cursors = transactionalContext.cursors
read.singleNode(node, cursor)
if (!cursor.next()) RelationshipIterator.EMPTY
else {
val selectionCursor = dir match {
case OUTGOING => outgoingCursor(transactionalContext.kernelTransaction.cursors(), cursor, types.orNull)
case INCOMING => incomingCursor(transactionalContext.kernelTransaction.cursors(), cursor, types.orNull)
case BOTH => allCursor(transactionalContext.kernelTransaction.cursors(), cursor, types.orNull)
case OUTGOING => outgoingCursor(cursors, cursor, types.orNull)
case INCOMING => incomingCursor(cursors, cursor, types.orNull)
case BOTH => allCursor(cursors, cursor, types.orNull)
}
new RelationshipCursorIterator(selectionCursor)
}
Expand All @@ -219,13 +226,15 @@ sealed class TransactionBoundQueryContext(val transactionalContext: Transactiona
override def getRelationshipsCursor(node: Long, dir: SemanticDirection,
types: Option[Array[Int]]): RelationshipSelectionCursor = {
val read = reads()
read.singleNode(node, nodeCursor)
if (!nodeCursor.next()) RelationshipSelectionCursor.EMPTY
val cursor = nodeCursor
val cursors = transactionalContext.cursors
read.singleNode(node, cursor)
if (!cursor.next()) RelationshipSelectionCursor.EMPTY
else {
dir match {
case OUTGOING => outgoingCursor(transactionalContext.kernelTransaction.cursors(), nodeCursor, types.orNull)
case INCOMING => incomingCursor(transactionalContext.kernelTransaction.cursors(), nodeCursor, types.orNull)
case BOTH => allCursor(transactionalContext.kernelTransaction.cursors(), nodeCursor, types.orNull)
case OUTGOING => outgoingCursor(cursors, cursor, types.orNull)
case INCOMING => incomingCursor(cursors, cursor, types.orNull)
case BOTH => allCursor(cursors, cursor, types.orNull)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class TransactionBoundQueryContextTest extends CypherFunSuite {
outerTx = mock[InternalTransaction]
val kernelTransaction = mock[KernelTransactionImplementation]
when(kernelTransaction.securityContext()).thenReturn(AUTH_DISABLED)
when(kernelTransaction.acquireStatement()).thenReturn(statement)
val storeStatement = mock[StorageStatement]
val operations = mock[StatementOperationParts](RETURNS_DEEP_STUBS)
statement = new KernelStatement(kernelTransaction, null, storeStatement, LockTracer.NONE, operations, new ClockContext(), EmptyVersionContextSupplier.EMPTY)
Expand All @@ -90,7 +91,7 @@ class TransactionBoundQueryContextTest extends CypherFunSuite {
val transaction = mock[KernelTransaction]
when(transaction.cursors()).thenReturn(new DefaultCursors())
when(bridge.getKernelTransactionBoundToThisThread(true)).thenReturn(transaction)
val tc = new Neo4jTransactionalContext(graph, null, bridge, locker, outerTx, statement, null, null)
val tc = new Neo4jTransactionalContext(graph, null, bridge, locker, outerTx, statement,null, null)
val transactionalContext = TransactionalContextWrapper(tc)
val context = new TransactionBoundQueryContext(transactionalContext)(indexSearchMonitor)
// WHEN
Expand All @@ -111,9 +112,10 @@ class TransactionBoundQueryContextTest extends CypherFunSuite {
when(outerTx.securityContext()).thenReturn(AUTH_DISABLED)
val bridge = mock[ThreadToStatementContextBridge]
val transaction = mock[KernelTransaction]
when(transaction.acquireStatement()).thenReturn(statement)
when(transaction.cursors()).thenReturn(new DefaultCursors())
when(bridge.getKernelTransactionBoundToThisThread(true)).thenReturn(transaction)
val tc = new Neo4jTransactionalContext(graph, null, bridge, locker, outerTx, statement, null, null)
val tc = new Neo4jTransactionalContext(graph, null, bridge, locker, outerTx, statement,null, null)
val transactionalContext = TransactionalContextWrapper(tc)
val context = new TransactionBoundQueryContext(transactionalContext)(indexSearchMonitor)
// WHEN
Expand Down Expand Up @@ -213,6 +215,7 @@ class TransactionBoundQueryContextTest extends CypherFunSuite {
// GIVEN
val bridge = mock[ThreadToStatementContextBridge]
val transaction = mock[KernelTransaction]
when(transaction.acquireStatement()).thenReturn(statement)
when(transaction.cursors()).thenReturn(new DefaultCursors())
when(bridge.getKernelTransactionBoundToThisThread(true)).thenReturn(transaction)
val tc = new Neo4jTransactionalContext(graph, null, bridge, locker, outerTx, statement, null, null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ public interface NodeCursor extends Cursor

LabelSet labels();

boolean hasLabel( int label );

boolean hasProperties();

void relationships( RelationshipGroupCursor cursor );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ public void shouldReadLabels()
labels = nodes.labels();
assertEquals( "number of labels", 1, labels.numberOfLabels() );
int fooLabel = labels.label( 0 );
assertTrue( nodes.hasLabel( fooLabel ) );
assertFalse( "should only access a single node", nodes.next() );

// when
Expand All @@ -151,6 +152,8 @@ public void shouldReadLabels()
labels = nodes.labels();
assertEquals( "number of labels", 1, labels.numberOfLabels() );
int barLabel = labels.label( 0 );
assertFalse( nodes.hasLabel( fooLabel ) );
assertTrue( nodes.hasLabel( barLabel ) );
assertFalse( "should only access a single node", nodes.next() );

// when
Expand All @@ -161,6 +164,9 @@ public void shouldReadLabels()
labels = nodes.labels();
assertEquals( "number of labels", 1, labels.numberOfLabels() );
int bazLabel = labels.label( 0 );
assertFalse( nodes.hasLabel( fooLabel ) );
assertFalse( nodes.hasLabel( barLabel ) );
assertTrue( nodes.hasLabel( bazLabel ) );
assertFalse( "should only access a single node", nodes.next() );

assertNotEquals( "distinct labels", fooLabel, barLabel );
Expand All @@ -183,6 +189,10 @@ public void shouldReadLabels()
assertEquals( bazLabel, labels.label( 0 ) );
assertEquals( barLabel, labels.label( 1 ) );
}
assertFalse( nodes.hasLabel( fooLabel ) );
assertTrue( nodes.hasLabel( barLabel ) );
assertTrue( nodes.hasLabel( bazLabel ) );

assertFalse( "should only access a single node", nodes.next() );

// when
Expand All @@ -192,6 +202,9 @@ public void shouldReadLabels()
assertTrue( "should access defined node", nodes.next() );
labels = nodes.labels();
assertEquals( "number of labels", 0, labels.numberOfLabels() );
assertFalse( nodes.hasLabel( fooLabel ) );
assertFalse( nodes.hasLabel( barLabel ) );
assertFalse( nodes.hasLabel( bazLabel ) );
assertFalse( "should only access a single node", nodes.next() );
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ public void shouldSeeNewLabeledNodeInTransaction() throws Exception
LabelSet labels = node.labels();
assertEquals( 1, labels.numberOfLabels() );
assertEquals( labelId, labels.label( 0 ) );
assertTrue( node.hasLabel( labelId ) );
assertFalse( node.hasLabel( labelId + 1 ) );
assertFalse( "should only find one node", node.next() );
}
tx.success();
Expand Down Expand Up @@ -139,6 +141,10 @@ public void shouldSeeLabelChangesInTransaction() throws Exception
assertTrue( "should access node", node.next() );

assertLabels( node.labels(), toRetain, toAdd );
assertTrue( node.hasLabel( toAdd ) );
assertTrue( node.hasLabel( toRetain ) );
assertFalse( node.hasLabel( toDelete ) );
assertFalse( node.hasLabel( toRegret ) );
assertFalse( "should only find one node", node.next() );
}
tx.success();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ public LabelSet labels()
return offset >= 0 && offset < nodes.size() ? nodes.get( offset ).labelSet() : LabelSet.NONE;
}

@Override
public boolean hasLabel( int label )
{
return labels().contains( label );
}

@Override
public boolean hasProperties()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ public boolean hasLabel( Label label )
return false;
}
transaction.dataRead().singleNode( nodeId, nodes );
return nodes.next() && nodes.labels().contains( labelId );
return nodes.next() && nodes.hasLabel( labelId );
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.neo4j.kernel.impl.store.RecordCursor;
import org.neo4j.kernel.impl.store.record.DynamicRecord;
import org.neo4j.kernel.impl.store.record.NodeRecord;
import org.neo4j.storageengine.api.txstate.ReadableDiffSets;

import static java.util.Collections.emptySet;

Expand Down Expand Up @@ -129,6 +130,36 @@ public LabelSet labels()
}
}

@Override
public boolean hasLabel( int label )
{
if ( hasChanges() )
{
TransactionState txState = read.txState();
ReadableDiffSets<Integer> diffSets = txState.nodeStateLabelDiffSets( getId() );
if ( diffSets.getAdded().contains( label ) )
{
return true;
}
if ( diffSets.getRemoved().contains( label ) )
{
return false;
}
}

//Get labels from store and put in intSet, unfortunately we get longs back
long[] longs = NodeLabelsField.get( this, labelCursor() );
for ( long labelToken : longs )
{
if ( labelToken == label )
{
assert (int) labelToken == labelToken : "value too big to be represented as and int";
return true;
}
}
return false;
}

@Override
public boolean hasProperties()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ public boolean contains( int labelToken )
//label sizes (≤100 labels)
for ( long label : labels )
{
assert (int) label == label : "value too big to be represented as and int";
if ( label == labelToken )
{
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ static <SUPPLIER extends SchemaDescriptorSupplier, EXCEPTION extends Exception>
{
SUPPLIER schemaSupplier = schemaSuppliers.next();
SchemaDescriptor schema = schemaSupplier.schema();
if ( node.labels().contains( schema.keyId() ) )
if ( node.hasLabel( schema.keyId() ) )
{
if ( nodePropertyIds == null )
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.neo4j.internal.kernel.api.Write;
import org.neo4j.internal.kernel.api.exceptions.EntityNotFoundException;
import org.neo4j.internal.kernel.api.exceptions.KernelException;
import org.neo4j.internal.kernel.api.exceptions.TransactionFailureException;
import org.neo4j.internal.kernel.api.exceptions.explicitindex.AutoIndexingKernelException;
import org.neo4j.internal.kernel.api.exceptions.explicitindex.ExplicitIndexNotFoundKernelException;
import org.neo4j.internal.kernel.api.exceptions.schema.ConstraintValidationException;
Expand All @@ -54,7 +55,6 @@
import org.neo4j.internal.kernel.api.schema.constraints.ConstraintDescriptor;
import org.neo4j.kernel.api.SilentTokenNameLookup;
import org.neo4j.kernel.api.StatementConstants;
import org.neo4j.internal.kernel.api.exceptions.TransactionFailureException;
import org.neo4j.kernel.api.exceptions.index.IndexEntryConflictException;
import org.neo4j.kernel.api.exceptions.index.IndexNotApplicableKernelException;
import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException;
Expand Down Expand Up @@ -227,7 +227,7 @@ public boolean nodeAddLabel( long node, int nodeLabel )
ktx.assertOpen();
singleNode( node );

if ( nodeCursor.labels().contains( nodeLabel ) )
if ( nodeCursor.hasLabel( nodeLabel ) )
{
//label already there, nothing to do
return false;
Expand Down Expand Up @@ -446,7 +446,7 @@ public boolean nodeRemoveLabel( long node, int nodeLabel ) throws EntityNotFound

singleNode( node );

if ( !nodeCursor.labels().contains( nodeLabel ) )
if ( !nodeCursor.hasLabel( nodeLabel ) )
{
//the label wasn't there, nothing to do
return false;
Expand Down

0 comments on commit 84395d2

Please sign in to comment.