From 7dd9f0bcf815f5cc9b9c2f49050daf2b6d93d7a1 Mon Sep 17 00:00:00 2001 From: Tobias Lindaaker Date: Fri, 26 Dec 2014 15:54:33 +0100 Subject: [PATCH] Immutable information in RelationshipProxy Since the start node, end node, and type of a relationship never changes, this information can be kept within the RelationshipProxy object. Doing so does not require significant storage overhead, with the current address sizes (nodeId=35bit, relationshipId=35bit, typeId=16bit) this information (together with the id of the relationship) occupies 212 bits, which can be stored in 16 bytes (128 bits). The implementation in this changeset packs the high bits of the start/end node ids and the relationship id into separate nibbles of a 16 bit (short) fields. This limits the number of roundtrips required down to the store layer. --- .../kernel/InternalAbstractGraphDatabase.java | 37 +++--- .../neo4j/kernel/impl/core/NodeManager.java | 25 ++++ .../org/neo4j/kernel/impl/core/NodeProxy.java | 12 +- .../kernel/impl/core/RelationshipData.java | 52 --------- .../kernel/impl/core/RelationshipProxy.java | 109 +++++++++++++----- .../TxStateTransactionDataSnapshot.java | 4 +- .../neo4j/tooling/GlobalGraphOperations.java | 2 +- .../test/java/org/neo4j/kernel/TestGuard.java | 2 +- .../TxStateTransactionDataViewTest.java | 2 +- 9 files changed, 136 insertions(+), 109 deletions(-) delete mode 100644 community/kernel/src/main/java/org/neo4j/kernel/impl/core/RelationshipData.java diff --git a/community/kernel/src/main/java/org/neo4j/kernel/InternalAbstractGraphDatabase.java b/community/kernel/src/main/java/org/neo4j/kernel/InternalAbstractGraphDatabase.java index 16a0830058360..853e66f8e3d48 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/InternalAbstractGraphDatabase.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/InternalAbstractGraphDatabase.java @@ -110,7 +110,6 @@ import org.neo4j.kernel.impl.core.NodeProxy.NodeActions; import org.neo4j.kernel.impl.core.PropertyKeyTokenHolder; import org.neo4j.kernel.impl.core.ReadOnlyTokenCreator; -import org.neo4j.kernel.impl.core.RelationshipData; import org.neo4j.kernel.impl.core.RelationshipProxy.RelationshipActions; import org.neo4j.kernel.impl.core.RelationshipTypeTokenHolder; import org.neo4j.kernel.impl.core.StartupStatistics; @@ -131,6 +130,8 @@ import org.neo4j.kernel.impl.locking.ResourceTypes; import org.neo4j.kernel.impl.locking.community.CommunityLockManger; import org.neo4j.kernel.impl.pagecache.LifecycledPageCache; +import org.neo4j.kernel.impl.query.QueryEngineProvider; +import org.neo4j.kernel.impl.query.QueryExecutionEngine; import org.neo4j.kernel.impl.query.QueryExecutionKernelException; import org.neo4j.kernel.impl.store.NeoStore; import org.neo4j.kernel.impl.store.StoreFactory; @@ -168,11 +169,10 @@ import org.neo4j.kernel.logging.Logging; import org.neo4j.kernel.logging.RollingLogMonitor; import org.neo4j.kernel.monitoring.Monitors; -import org.neo4j.kernel.impl.query.QueryEngineProvider; -import org.neo4j.kernel.impl.query.QueryExecutionEngine; import org.neo4j.tooling.GlobalGraphOperations; import static java.lang.String.format; + import static org.neo4j.collection.primitive.PrimitiveLongCollections.map; import static org.neo4j.helpers.Settings.STRING; import static org.neo4j.helpers.Settings.setting; @@ -773,21 +773,6 @@ public Node newNodeProxy( long nodeId ) return nodeManager.newNodeProxyById( nodeId ); } - @Override - public RelationshipData getRelationshipData( long relationshipId ) - { - try ( Statement statement = threadToTransactionBridge.instance() ) - { - RelationshipData data = new RelationshipData(); - statement.readOperations().relationshipVisit( relationshipId, data ); - return data; - } - catch ( EntityNotFoundException e ) - { - throw new NotFoundException( e ); - } - } - @Override public RelationshipType getRelationshipTypeById( int type ) { @@ -833,10 +818,22 @@ public void failTransaction() } @Override - public Relationship newRelationshipProxy( long id ) + public Relationship lazyRelationshipProxy( long id ) { return nodeManager.newRelationshipProxyById( id ); } + + @Override + public Relationship newRelationshipProxy( long id ) + { + return nodeManager.newRelationshipProxy( id ); + } + + @Override + public Relationship newRelationshipProxy( long id, long startNodeId, int typeId, long endNodeId ) + { + return nodeManager.newRelationshipProxy( id, startNodeId, typeId, endNodeId ); + } }; } @@ -1101,7 +1098,7 @@ public Relationship getRelationshipById( long id ) throw new NotFoundException( format( "Relationship %d not found", id ) ); } - return nodeManager.newRelationshipProxyById( id ); + return nodeManager.newRelationshipProxy( id ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/core/NodeManager.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/core/NodeManager.java index 4830285b90a2c..6140ee6b995a1 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/core/NodeManager.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/core/NodeManager.java @@ -23,8 +23,11 @@ import java.util.concurrent.CopyOnWriteArrayList; import org.neo4j.graphdb.Node; +import org.neo4j.graphdb.NotFoundException; import org.neo4j.graphdb.Relationship; import org.neo4j.kernel.PropertyTracker; +import org.neo4j.kernel.api.Statement; +import org.neo4j.kernel.api.exceptions.EntityNotFoundException; import org.neo4j.kernel.lifecycle.LifecycleAdapter; import static java.lang.System.currentTimeMillis; @@ -69,12 +72,34 @@ public NodeProxy newNodeProxyById( long id ) return new NodeProxy( nodeActions, id ); } + /** Returns a "lazy" proxy, where additional fields are initialized on access. */ @Override public RelationshipProxy newRelationshipProxyById( long id ) { return new RelationshipProxy( relationshipActions, id ); } + /** Returns a fully initialized proxy. */ + public RelationshipProxy newRelationshipProxy( long id ) + { + try ( Statement statement = threadToTransactionBridge.instance() ) + { + RelationshipProxy proxy = new RelationshipProxy( relationshipActions, id ); + statement.readOperations().relationshipVisit( id, proxy ); + return proxy; + } + catch ( EntityNotFoundException e ) + { + throw new NotFoundException( e ); + } + } + + /** Returns a fully initialized proxy. */ + public RelationshipProxy newRelationshipProxy( long id, long startNodeId, int typeId, long endNodeId ) + { + return new RelationshipProxy( relationshipActions, id, startNodeId, typeId, endNodeId ); + } + @Override public GraphPropertiesImpl newGraphProperties() { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/core/NodeProxy.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/core/NodeProxy.java index a7605b55ec47e..0bbf21f112d35 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/core/NodeProxy.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/core/NodeProxy.java @@ -83,7 +83,11 @@ public interface NodeActions void failTransaction(); + Relationship lazyRelationshipProxy( long id ); + Relationship newRelationshipProxy( long id ); + + Relationship newRelationshipProxy( long id, long startNodeId, int typeId, long endNodeId ); } private final NodeActions actions; @@ -431,16 +435,16 @@ public Relationship createRelationshipTo( Node otherNode, RelationshipType type throw new IllegalArgumentException( "Other node is null." ); } // TODO: This is the checks we would like to do, but we have tests that expect to mix nodes... - //if ( !(otherNode instanceof NodeProxy) || (((NodeProxy) otherNode).nodeLookup != nodeLookup) ) + //if ( !(otherNode instanceof NodeProxy) || (((NodeProxy) otherNode).actions != actions) ) //{ // throw new IllegalArgumentException( "Nodes do not belong to same graph database." ); //} try ( Statement statement = actions.statement() ) { int relationshipTypeId = statement.tokenWriteOperations().relationshipTypeGetOrCreateForName( type.name() ); - return actions.newRelationshipProxy( - statement.dataWriteOperations() - .relationshipCreate( relationshipTypeId, nodeId, otherNode.getId() ) ); + long relationshipId = statement.dataWriteOperations() + .relationshipCreate( relationshipTypeId, nodeId, otherNode.getId() ); + return actions.newRelationshipProxy( relationshipId, nodeId, relationshipTypeId, otherNode.getId() ); } catch ( IllegalTokenNameException | RelationshipTypeIdNotFoundKernelException e ) { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/core/RelationshipData.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/core/RelationshipData.java deleted file mode 100644 index d117f982e0094..0000000000000 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/core/RelationshipData.java +++ /dev/null @@ -1,52 +0,0 @@ -/** - * Copyright (c) 2002-2015 "Neo Technology," - * Network Engine for Objects in Lund AB [http://neotechnology.com] - * - * This file is part of Neo4j. - * - * Neo4j is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ -package org.neo4j.kernel.impl.core; - -import org.neo4j.kernel.impl.api.RelationshipVisitor; - -public class RelationshipData implements RelationshipVisitor -{ - private long startNode; - private long endNode; - private int type; - - public long getStartNode() - { - return startNode; - } - - public long getEndNode() - { - return endNode; - } - - public int getType() - { - return type; - } - - @Override - public void visit( long relIdIgnored, int type, long startNode, long endNode ) - { - this.startNode = startNode; - this.endNode = endNode; - this.type = type; - } -} diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/core/RelationshipProxy.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/core/RelationshipProxy.java index a79c8deeb4877..9fe8956ad02e8 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/core/RelationshipProxy.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/core/RelationshipProxy.java @@ -39,9 +39,10 @@ import org.neo4j.kernel.api.exceptions.schema.IllegalTokenNameException; import org.neo4j.kernel.api.properties.DefinedProperty; import org.neo4j.kernel.api.properties.Property; +import org.neo4j.kernel.impl.api.RelationshipVisitor; import org.neo4j.kernel.impl.api.operations.KeyReadOperations; -public class RelationshipProxy implements Relationship +public class RelationshipProxy implements Relationship, RelationshipVisitor { public interface RelationshipActions { @@ -49,8 +50,6 @@ public interface RelationshipActions Node newNodeProxy( long nodeId ); - RelationshipData getRelationshipData( long relationshipId ); - RelationshipType getRelationshipTypeById( int type ); GraphDatabaseService getGraphDatabaseService(); @@ -61,18 +60,76 @@ public interface RelationshipActions } private final RelationshipActions actions; - private final long relId; + /** [unused,target][source,id] **/ + private short hiBits; + private short type; + private int loId, loSource, loTarget; - public RelationshipProxy( RelationshipActions actions, long relId ) + public RelationshipProxy( RelationshipActions actions, long id, long startNode, int type, long endNode ) { - this.relId = relId; this.actions = actions; + visit( id, type, startNode, endNode ); + } + + public RelationshipProxy( RelationshipActions actions, long id ) + { + this.actions = actions; + assert (0xFFFF_FFF0_0000_0000L & id) == 0; + this.hiBits = (short) (0xF000 | (id >> 32)); + this.loId = (int) id; + } + + @Override + public void visit( long id, int type, long startNode, long endNode ) throws RuntimeException + { + assert (0xFFFF_FFF0_0000_0000L & id) == 0 && // 36 bits + (0xFFFF_FFF0_0000_0000L & startNode) == 0 && // 36 bits + (0xFFFF_FFF0_0000_0000L & endNode) == 0 && // 36 bits + (0xFFFF_0000 & type) == 0; // 16 bits + this.hiBits = (short) ((id >> 32) | ((startNode >> 28) & 0x00F0) | ((startNode >> 28) & 0x0F00)); + this.type = (short) type; + this.loId = (int) id; + this.loSource = (int) startNode; + this.loTarget = (int) endNode; + } + + private void initializeData() + { + if ( (hiBits & 0xF000) != 0 ) + { + try ( Statement statement = actions.statement() ) + { + statement.readOperations().relationshipVisit( getId(), this ); + } + catch ( EntityNotFoundException e ) + { + throw new NotFoundException( e ); + } + } } @Override public long getId() { - return relId; + return hiBits == 0 ? loId : ((hiBits & 0x000FL) << 32 | loId); + } + + private int typeId() + { + initializeData(); + return type & 0xFFFF; + } + + private long sourceId() + { + initializeData(); + return hiBits == 0 ? loSource : ((hiBits & 0x00F0L) << 28 | loSource); + } + + private long targetId() + { + initializeData(); + return hiBits == 0 ? loTarget : ((hiBits & 0x0F00L) << 24 | loTarget); } @Override @@ -95,7 +152,7 @@ public void delete() catch ( EntityNotFoundException e ) { throw new IllegalStateException( "Unable to delete relationship[" + - relId + "] since it is already deleted." ); + getId() + "] since it is already deleted." ); } } @@ -103,24 +160,22 @@ public void delete() public Node[] getNodes() { assertInUnterminatedTransaction(); - RelationshipData data = actions.getRelationshipData( relId ); return new Node[]{ - actions.newNodeProxy( data.getStartNode() ), - actions.newNodeProxy( data.getEndNode() )}; + actions.newNodeProxy( sourceId() ), + actions.newNodeProxy( targetId() )}; } @Override public Node getOtherNode( Node node ) { assertInUnterminatedTransaction(); - RelationshipData data = actions.getRelationshipData( relId ); - if ( data.getStartNode() == node.getId() ) + if ( sourceId() == node.getId() ) { - return actions.newNodeProxy( data.getEndNode() ); + return actions.newNodeProxy( targetId() ); } - if ( data.getEndNode() == node.getId() ) + if ( targetId() == node.getId() ) { - return actions.newNodeProxy( data.getStartNode() ); + return actions.newNodeProxy( sourceId() ); } throw new NotFoundException( "Node[" + node.getId() + "] not connected to this relationship[" + getId() + "]" ); @@ -130,22 +185,21 @@ public Node getOtherNode( Node node ) public Node getStartNode() { assertInUnterminatedTransaction(); - return actions.newNodeProxy( actions.getRelationshipData( relId ).getStartNode() ); + return actions.newNodeProxy( sourceId() ); } @Override public Node getEndNode() { assertInUnterminatedTransaction(); - return actions.newNodeProxy( actions.getRelationshipData( relId ).getEndNode() ); + return actions.newNodeProxy( targetId() ); } @Override public RelationshipType getType() { assertInUnterminatedTransaction(); - int type = actions.getRelationshipData( relId ).getType(); - return actions.getRelationshipTypeById( type ); + return actions.getRelationshipTypeById( typeId() ); } @Override @@ -189,7 +243,7 @@ public Object getProperty( String key ) { throw new NotFoundException( String.format( "No such property, '%s'.", key ) ); } - return statement.readOperations().relationshipGetProperty( relId, propertyId ).value(); + return statement.readOperations().relationshipGetProperty( getId(), propertyId ).value(); } catch ( EntityNotFoundException | PropertyNotFoundException e ) { @@ -210,7 +264,7 @@ public Object getProperty( String key, Object defaultValue ) try ( Statement statement = actions.statement() ) { int propertyId = statement.readOperations().propertyKeyGetForName( key ); - return statement.readOperations().relationshipGetProperty( relId, propertyId ).value( defaultValue ); + return statement.readOperations().relationshipGetProperty( getId(), propertyId ).value( defaultValue ); } catch ( EntityNotFoundException e ) { @@ -230,7 +284,7 @@ public boolean hasProperty( String key ) { int propertyId = statement.readOperations().propertyKeyGetForName( key ); return propertyId != KeyReadOperations.NO_SUCH_PROPERTY_KEY && - statement.readOperations().relationshipGetProperty( relId, propertyId ).isDefined(); + statement.readOperations().relationshipGetProperty( getId(), propertyId ).isDefined(); } catch ( EntityNotFoundException e ) { @@ -244,7 +298,7 @@ public void setProperty( String key, Object value ) try ( Statement statement = actions.statement() ) { int propertyKeyId = statement.tokenWriteOperations().propertyKeyGetOrCreateForName( key ); - statement.dataWriteOperations().relationshipSetProperty( relId, Property.property( propertyKeyId, value ) ); + statement.dataWriteOperations().relationshipSetProperty( getId(), Property.property( propertyKeyId, value ) ); } catch ( IllegalArgumentException e ) { @@ -273,7 +327,7 @@ public Object removeProperty( String key ) try ( Statement statement = actions.statement() ) { int propertyId = statement.tokenWriteOperations().propertyKeyGetOrCreateForName( key ); - return statement.dataWriteOperations().relationshipRemoveProperty( relId, propertyId ).value( null ); + return statement.dataWriteOperations().relationshipRemoveProperty( getId(), propertyId ).value( null ); } catch ( EntityNotFoundException e ) { @@ -294,8 +348,7 @@ public Object removeProperty( String key ) public boolean isType( RelationshipType type ) { assertInUnterminatedTransaction(); - int typeId = actions.getRelationshipData( relId ).getType(); - return actions.getRelationshipTypeById( typeId ).name().equals( type.name() ); + return actions.getRelationshipTypeById( typeId() ).name().equals( type.name() ); } public int compareTo( Object rel ) @@ -326,7 +379,7 @@ public boolean equals( Object o ) @Override public int hashCode() { - return (int) ((relId >>> 32) ^ relId); + return (int) ((getId() >>> 32) ^ getId()); } @Override diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataSnapshot.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataSnapshot.java index dfadd291607cb..107b4d46646a6 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataSnapshot.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataSnapshot.java @@ -246,7 +246,7 @@ private Iterable map2Rels( Iterable added ) @Override public Relationship apply( Long id ) { - return nodeActions.newRelationshipProxy( id ); + return nodeActions.lazyRelationshipProxy( id ); } }, added); } @@ -355,7 +355,7 @@ public RelationshipPropertyEntryView( long relId, String key, Object newValue, O @Override public Relationship entity() { - return nodeActions.newRelationshipProxy( relId ); + return nodeActions.lazyRelationshipProxy( relId ); } @Override diff --git a/community/kernel/src/main/java/org/neo4j/tooling/GlobalGraphOperations.java b/community/kernel/src/main/java/org/neo4j/tooling/GlobalGraphOperations.java index 69ed5095e2eee..6d80a92418ae6 100644 --- a/community/kernel/src/main/java/org/neo4j/tooling/GlobalGraphOperations.java +++ b/community/kernel/src/main/java/org/neo4j/tooling/GlobalGraphOperations.java @@ -136,7 +136,7 @@ public void close() @Override protected Relationship fetchNextOrNull() { - return ids.hasNext() ? nodeManager.newRelationshipProxyById( ids.next() ) : null; + return ids.hasNext() ? nodeManager.newRelationshipProxy( ids.next() ) : null; } }; } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/TestGuard.java b/community/kernel/src/test/java/org/neo4j/kernel/TestGuard.java index 4268eb0ffe717..ba71f82c2be56 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/TestGuard.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/TestGuard.java @@ -104,7 +104,7 @@ public void testGuardOnDifferentGraphOps() ignore( position ); } Guard.OperationsCount ops4 = getGuard( db ).stop(); - assertEquals( 3, ops4.getOpsCount() ); + assertEquals( 4, ops4.getOpsCount() ); } db.shutdown(); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataViewTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataViewTest.java index a3f17352edc58..dbdf83853cb34 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataViewTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataViewTest.java @@ -283,7 +283,7 @@ private TxStateTransactionDataSnapshot snapshot() { NodeProxy.NodeActions nodeActions = mock( NodeProxy.NodeActions.class ); final RelationshipProxy.RelationshipActions relActions = mock( RelationshipProxy.RelationshipActions.class ); - when( nodeActions.newRelationshipProxy( anyLong() ) ).thenAnswer( new Answer() + when( nodeActions.lazyRelationshipProxy( anyLong() ) ).thenAnswer( new Answer() { @Override public RelationshipProxy answer( InvocationOnMock invocation ) throws Throwable