Skip to content

Commit

Permalink
Uses HashSet<Long> for removals instead of a RelIdArray, it was a mis…
Browse files Browse the repository at this point in the history
…use of RelIdArray all along and the RelIdArray->HashSet conversion can be skipped when getting relationships
  • Loading branch information
tinwelint authored and systay committed Jul 27, 2011
1 parent d509e31 commit e6d9154
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 43 deletions.
Expand Up @@ -19,6 +19,7 @@
*/
package org.neo4j.kernel.impl.core;

import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
Expand Down Expand Up @@ -127,7 +128,7 @@ else if ( newSrc.couldBeNeedingUpdate() )
RelIdIterator itr = newRels.get( type );
if ( itr == null )
{
RelIdArray remove = nodeManager.getCowRelationshipRemoveMap( fromNode, type );
Collection<Long> remove = nodeManager.getCowRelationshipRemoveMap( fromNode, type );
itr = remove == null ? ids.iterator( direction ) :
RelIdArray.from( ids, null, remove ).iterator( direction );
newRels.put( type, itr );
Expand Down
20 changes: 10 additions & 10 deletions kernel/src/main/java/org/neo4j/kernel/impl/core/LockReleaser.java
Expand Up @@ -20,6 +20,8 @@
package org.neo4j.kernel.impl.core;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map.Entry;
import java.util.Set;
Expand Down Expand Up @@ -85,7 +87,7 @@ private static class CowNodeElement
boolean deleted = false;

ArrayMap<String,RelIdArray> relationshipAddMap = null;
ArrayMap<String,RelIdArray> relationshipRemoveMap = null;
ArrayMap<String,Collection<Long>> relationshipRemoveMap = null;
ArrayMap<Integer,PropertyData> propertyAddMap = null;
ArrayMap<Integer,PropertyData> propertyRemoveMap = null;
}
Expand Down Expand Up @@ -199,7 +201,7 @@ private Transaction getTransaction()
}
}

public RelIdArray getCowRelationshipRemoveMap( NodeImpl node, String type )
public Collection<Long> getCowRelationshipRemoveMap( NodeImpl node, String type )
{
PrimitiveElement primitiveElement = cowMap.get( getTransaction() );
if ( primitiveElement != null )
Expand All @@ -215,7 +217,7 @@ public RelIdArray getCowRelationshipRemoveMap( NodeImpl node, String type )
return null;
}

public RelIdArray getCowRelationshipRemoveMap( NodeImpl node, String type,
public Collection<Long> getCowRelationshipRemoveMap( NodeImpl node, String type,
boolean create )
{
if ( !create )
Expand All @@ -233,12 +235,12 @@ public RelIdArray getCowRelationshipRemoveMap( NodeImpl node, String type,
}
if ( element.relationshipRemoveMap == null )
{
element.relationshipRemoveMap = new ArrayMap<String,RelIdArray>();
element.relationshipRemoveMap = new ArrayMap<String,Collection<Long>>();
}
RelIdArray set = element.relationshipRemoveMap.get( type );
Collection<Long> set = element.relationshipRemoveMap.get( type );
if ( set == null )
{
set = new RelIdArray( null );
set = new HashSet<Long>();
element.relationshipRemoveMap.put( type, set );
}
return set;
Expand Down Expand Up @@ -827,11 +829,9 @@ private void populateNodeRelEvent( PrimitiveElement element,
{
for ( String type : nodeElement.relationshipRemoveMap.keySet() )
{
RelIdArray deletedRels =
nodeElement.relationshipRemoveMap.get( type );
for ( RelIdIterator iterator = deletedRels.iterator( DirectionWrapper.BOTH ); iterator.hasNext(); )
Collection<Long> deletedRels = nodeElement.relationshipRemoveMap.get( type );
for ( long relId : deletedRels )
{
long relId = iterator.next();
if ( nodeManager.relCreated( relId ) )
{
continue;
Expand Down
17 changes: 9 additions & 8 deletions kernel/src/main/java/org/neo4j/kernel/impl/core/NodeImpl.java
Expand Up @@ -21,6 +21,7 @@

import static org.neo4j.kernel.impl.util.RelIdArray.empty;

import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
Expand Down Expand Up @@ -126,7 +127,7 @@ List<RelIdIterator> getAllRelationships( NodeManager nodeManager, DirectionWrapp
for ( RelIdArray src : relationships )
{
String type = src.getType();
RelIdArray remove = null;
Collection<Long> remove = null;
RelIdArray add = null;
RelIdIterator iterator = null;
if ( hasModifications )
Expand All @@ -150,7 +151,7 @@ List<RelIdIterator> getAllRelationships( NodeManager nodeManager, DirectionWrapp
{
if ( getRelIdArray( type ) == null )
{
RelIdArray remove = nodeManager.getCowRelationshipRemoveMap( this, type );
Collection<Long> remove = nodeManager.getCowRelationshipRemoveMap( this, type );
RelIdArray add = addMap.get( type );
relTypeList.add( new CombinedRelIdIterator( type, direction, null, add, remove ) );
}
Expand All @@ -169,7 +170,7 @@ List<RelIdIterator> getAllRelationshipsOfType( NodeManager nodeManager,
{
String typeName = type.name();
RelIdArray src = getRelIdArray( typeName );
RelIdArray remove = null;
Collection<Long> remove = null;
RelIdArray add = null;
RelIdIterator iterator = null;
if ( hasModifications )
Expand Down Expand Up @@ -307,9 +308,9 @@ void addRelationship( NodeManager nodeManager, RelationshipType type, long relId
// a relationship delete is invoked.
void removeRelationship( NodeManager nodeManager, RelationshipType type, long relId )
{
RelIdArray relationshipSet = nodeManager.getCowRelationshipRemoveMap(
Collection<Long> relationshipSet = nodeManager.getCowRelationshipRemoveMap(
this, type.name(), true );
relationshipSet.add( relId, DirectionWrapper.OUTGOING );
relationshipSet.add( relId );
}

private void ensureRelationshipMapNotNull( NodeManager nodeManager )
Expand Down Expand Up @@ -577,7 +578,7 @@ public boolean hasRelationship( NodeManager nodeManager, RelationshipType type,

protected void commitRelationshipMaps(
ArrayMap<String,RelIdArray> cowRelationshipAddMap,
ArrayMap<String,RelIdArray> cowRelationshipRemoveMap )
ArrayMap<String,Collection<Long>> cowRelationshipRemoveMap )
{
if ( relationships == null )
{
Expand All @@ -592,7 +593,7 @@ protected void commitRelationshipMaps(
for ( String type : cowRelationshipAddMap.keySet() )
{
RelIdArray add = cowRelationshipAddMap.get( type );
RelIdArray remove = null;
Collection<Long> remove = null;
if ( cowRelationshipRemoveMap != null )
{
remove = cowRelationshipRemoveMap.get( type );
Expand All @@ -613,7 +614,7 @@ protected void commitRelationshipMaps(
RelIdArray src = getRelIdArray( type );
if ( src != null )
{
RelIdArray remove = cowRelationshipRemoveMap.get( type );
Collection<Long> remove = cowRelationshipRemoveMap.get( type );
putRelIdArray( RelIdArray.from( src, null, remove ) );
}
}
Expand Down
Expand Up @@ -19,6 +19,7 @@
*/
package org.neo4j.kernel.impl.core;

import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.locks.ReentrantLock;
Expand Down Expand Up @@ -881,12 +882,12 @@ void relRemoveProperty( RelationshipImpl rel, long propertyId )
persistenceManager.relRemoveProperty( rel.getId(), propertyId );
}

public RelIdArray getCowRelationshipRemoveMap( NodeImpl node, String type )
public Collection<Long> getCowRelationshipRemoveMap( NodeImpl node, String type )
{
return lockReleaser.getCowRelationshipRemoveMap( node, type );
}

public RelIdArray getCowRelationshipRemoveMap( NodeImpl node, String type,
public Collection<Long> getCowRelationshipRemoveMap( NodeImpl node, String type,
boolean create )
{
return lockReleaser.getCowRelationshipRemoveMap( node, type, create );
Expand Down
Expand Up @@ -36,14 +36,14 @@ public class CombinedRelIdIterator implements RelIdIterator
private long nextElement;

public CombinedRelIdIterator( String type, DirectionWrapper direction, RelIdArray src,
RelIdArray add, RelIdArray remove )
RelIdArray add, Collection<Long> remove )
{
this.type = type;
this.direction = direction;
this.srcIterator = src != null ? src.iterator( direction ) : RelIdArray.EMPTY.iterator( direction );
this.addIterator = add != null ? add.iterator( direction ) : RelIdArray.EMPTY.iterator( direction );
this.currentIterator = srcIterator;
this.removed = remove != null ? remove.asSet() : null;
this.removed = remove;
}

@Override
Expand Down
22 changes: 5 additions & 17 deletions kernel/src/main/java/org/neo4j/kernel/impl/util/RelIdArray.java
Expand Up @@ -19,9 +19,8 @@
*/
package org.neo4j.kernel.impl.util;

import java.util.HashSet;
import java.util.Collection;
import java.util.NoSuchElementException;
import java.util.Set;

import org.neo4j.graphdb.Direction;

Expand Down Expand Up @@ -777,7 +776,7 @@ public long next()
}
}

public static RelIdArray from( RelIdArray src, RelIdArray add, RelIdArray remove )
public static RelIdArray from( RelIdArray src, RelIdArray add, Collection<Long> remove )
{
if ( remove == null )
{
Expand All @@ -801,12 +800,11 @@ public static RelIdArray from( RelIdArray src, RelIdArray add, RelIdArray remove
return null;
}
RelIdArray newArray = null;
Set<Long> removedSet = remove.asSet();
if ( src != null )
{
newArray = src.newSimilarInstance();
newArray.addAll( src );
evictExcluded( newArray, removedSet );
evictExcluded( newArray, remove );
}
else
{
Expand All @@ -818,7 +816,7 @@ public static RelIdArray from( RelIdArray src, RelIdArray add, RelIdArray remove
for ( RelIdIteratorImpl fromIterator = (RelIdIteratorImpl) add.iterator( DirectionWrapper.BOTH ); fromIterator.hasNext();)
{
long value = fromIterator.next();
if ( !removedSet.contains( value ) )
if ( !remove.contains( value ) )
{
newArray.add( value, fromIterator.currentDirection );
}
Expand All @@ -828,7 +826,7 @@ public static RelIdArray from( RelIdArray src, RelIdArray add, RelIdArray remove
}
}

private static void evictExcluded( RelIdArray ids, Set<Long> excluded )
private static void evictExcluded( RelIdArray ids, Collection<Long> excluded )
{
for ( RelIdIteratorImpl iterator = (RelIdIteratorImpl) DirectionWrapper.BOTH.iterator( ids ); iterator.hasNext(); )
{
Expand Down Expand Up @@ -856,16 +854,6 @@ private static void evictExcluded( RelIdArray ids, Set<Long> excluded )
}
}
}

public Set<Long> asSet()
{
Set<Long> set = new HashSet<Long>();
for ( RelIdIterator iterator = DirectionWrapper.BOTH.iterator( this ); iterator.hasNext(); )
{
set.add( iterator.next() );
}
return set;
}

/**
* Optimization in the lazy loading of relationships for a node.
Expand Down
Expand Up @@ -90,9 +90,9 @@ public void testWithAddRemove() throws Exception
add.add( 5, OUTGOING );
add.add( 6, OUTGOING );
add.add( 7, OUTGOING );
RelIdArray remove = new RelIdArray( null );
remove.add( 2, OUTGOING );
remove.add( 6, OUTGOING );
Collection<Long> remove = new HashSet<Long>();
remove.add( 2L );
remove.add( 6L );
List<Long> allIds = asList( RelIdArray.from( source, add, remove ) );
Collections.sort( allIds );
assertEquals( Arrays.asList( 1L, 3L, 4L, 5L, 7L ), allIds );
Expand Down

0 comments on commit e6d9154

Please sign in to comment.