Skip to content

Commit

Permalink
Code cleanup, like using constant for -1
Browse files Browse the repository at this point in the history
  • Loading branch information
tinwelint committed Aug 31, 2016
1 parent 7252edb commit 339366a
Show file tree
Hide file tree
Showing 15 changed files with 78 additions and 72 deletions.
Expand Up @@ -83,6 +83,11 @@ public int intValue()
return intValue;
}

public long longValue()
{
return intValue;
}

public boolean is( long value )
{
return value == intValue;
Expand Down
Expand Up @@ -27,6 +27,8 @@
import org.neo4j.unsafe.impl.batchimport.staging.ForkedProcessorStep;
import org.neo4j.unsafe.impl.batchimport.staging.StageControl;

import static org.neo4j.unsafe.impl.batchimport.cache.idmapping.IdMapper.ID_NOT_FOUND;

/**
* Increments counts for each visited relationship, once for start node and once for end node
* (unless for loops). This to be able to determine which nodes are dense before starting to import relationships.
Expand All @@ -53,8 +55,8 @@ protected void forkedProcess( int id, int processors, Batch<InputRelationship,Re
long startNodeId = batch.ids[idIndex++];
long endNodeId = batch.ids[idIndex++];
processNodeId( id, processors, startNodeId, relationship, relationship.startNode() );
if ( startNodeId != endNodeId || // avoid counting loops twice
startNodeId == -1 || endNodeId == -1 ) // although always collect bad relationships
if ( startNodeId != endNodeId || // avoid counting loops twice
startNodeId == ID_NOT_FOUND ) // although always collect bad relationships
{
// Loops only counts as one
processNodeId( id, processors, endNodeId, relationship, relationship.endNode() );
Expand All @@ -65,9 +67,9 @@ protected void forkedProcess( int id, int processors, Batch<InputRelationship,Re
private void processNodeId( int id, int processors, long nodeId,
InputRelationship relationship, Object inputId )
{
if ( nodeId == -1 )
if ( nodeId == ID_NOT_FOUND )
{
if ( id == 0 )
if ( id == MAIN )
{
// Only let the processor with id=0 (which always exists) report the bad relationships
badCollector.collectBadRelationship( relationship, inputId );
Expand Down
Expand Up @@ -34,7 +34,6 @@ public class CalculateRelationshipsStep extends ProcessorStep<Batch<InputRelatio
{
private final RelationshipStore relationshipStore;
private long numberOfRelationships;
private long maxSpecific;

public CalculateRelationshipsStep( StageControl control, Configuration config, RelationshipStore relationshipStore )
{
Expand All @@ -53,7 +52,7 @@ protected void process( Batch<InputRelationship,RelationshipRecord> batch, Batch
protected void done()
{
long highestId = relationshipStore.getHighId() + numberOfRelationships;
relationshipStore.setHighestPossibleIdInUse( Math.max( highestId, maxSpecific ) );
relationshipStore.setHighestPossibleIdInUse( highestId );
super.done();
}
}
Expand Up @@ -62,7 +62,7 @@ static RecordIdIterator allInReversed( RecordStore<? extends AbstractBaseRecord>
return backwards( store.getNumberOfReservedLowIds(), store.getHighId(), config );
}

static class Forwards extends PrimitiveLongBaseIterator implements RecordIdIterator
class Forwards extends PrimitiveLongBaseIterator implements RecordIdIterator
{
private final long lowIncluded;
private final long highExcluded;
Expand All @@ -73,7 +73,8 @@ static class Forwards extends PrimitiveLongBaseIterator implements RecordIdItera

public Forwards( long lowIncluded, long highExcluded, Configuration config )
{
this.lowIncluded = this.nextId = lowIncluded;
this.lowIncluded = lowIncluded;
this.nextId = lowIncluded;
this.highExcluded = highExcluded;
this.batchSize = config.batchSize();
}
Expand Down Expand Up @@ -119,7 +120,7 @@ public String toString()
}
}

static class Backwards extends PrimitiveLongBaseIterator implements RecordIdIterator
class Backwards extends PrimitiveLongBaseIterator implements RecordIdIterator
{
private final int batchSize;
private final long lowIncluded;
Expand All @@ -134,7 +135,10 @@ public Backwards( long lowIncluded, long highExcluded, Configuration config )
{
this.lowIncluded = lowIncluded;
this.batchSize = config.batchSize();
this.highExcluded = this.nextId = this.nextRoofId = this.floorId = highExcluded;
this.highExcluded = highExcluded;
this.nextId = highExcluded;
this.nextRoofId = highExcluded;
this.floorId = highExcluded;
}

@Override
Expand Down
Expand Up @@ -53,7 +53,7 @@ protected void forkedProcess( int id, int processors, Batch<InputRelationship,Re
RelationshipRecord relationship = batch.records[i];
long startNode = relationship.getFirstNode();
long endNode = relationship.getSecondNode();
if ( startNode == -1 || endNode == -1 )
if ( !relationship.inUse() )
{ // This means that we here have a relationship that refers to missing nodes.
// It also means that we tolerate some amount of bad relationships and CalculateDenseNodesStep
// already have reported this to the bad collector.
Expand Down
Expand Up @@ -22,6 +22,7 @@
import org.neo4j.graphdb.Direction;
import org.neo4j.kernel.impl.store.record.RelationshipRecord;
import org.neo4j.unsafe.impl.batchimport.cache.NodeRelationshipCache;
import static org.neo4j.unsafe.impl.batchimport.cache.idmapping.IdMapper.ID_NOT_FOUND;

/**
* Links the {@code previous} fields in {@link RelationshipRecord relationship records}. This is done after
Expand Down Expand Up @@ -50,7 +51,7 @@ public boolean process( RelationshipRecord record )
{
long prevRel = cache.getAndPutRelationship( record.getFirstNode(),
Direction.BOTH, record.getId(), false );
if ( prevRel == -1 )
if ( prevRel == ID_NOT_FOUND )
{ // First one
record.setFirstInFirstChain( true );
record.setFirstInSecondChain( true );
Expand All @@ -68,7 +69,7 @@ public boolean process( RelationshipRecord record )
{
long firstPrevRel = cache.getAndPutRelationship( record.getFirstNode(),
Direction.OUTGOING, record.getId(), false );
if ( firstPrevRel == -1 )
if ( firstPrevRel == ID_NOT_FOUND )
{ // First one
record.setFirstInFirstChain( true );
firstPrevRel = cache.getCount( record.getFirstNode(), Direction.OUTGOING );
Expand All @@ -83,7 +84,7 @@ public boolean process( RelationshipRecord record )
{
long secondPrevRel = cache.getAndPutRelationship( record.getSecondNode(),
Direction.INCOMING, record.getId(), false );
if ( secondPrevRel == -1 )
if ( secondPrevRel == ID_NOT_FOUND )
{ // First one
record.setFirstInSecondChain( true );
secondPrevRel = cache.getCount( record.getSecondNode(), Direction.INCOMING );
Expand Down
Expand Up @@ -26,6 +26,8 @@
import org.neo4j.unsafe.impl.batchimport.staging.ForkedProcessorStep;
import org.neo4j.unsafe.impl.batchimport.staging.StageControl;

import static org.neo4j.unsafe.impl.batchimport.cache.idmapping.IdMapper.ID_NOT_FOUND;

/**
* Links relationship chains together, the "prev" pointers of them. "next" pointers are set when
* initially creating the relationship records. Setting prev pointers at that time would incur
Expand Down Expand Up @@ -84,7 +86,7 @@ public boolean process( RelationshipRecord record, int id, int processors )
{
long prevRel = cache.getAndPutRelationship( record.getFirstNode(),
Direction.BOTH, record.getId(), false );
if ( prevRel == -1 )
if ( prevRel == ID_NOT_FOUND )
{ // First one
record.setFirstInFirstChain( true );
record.setFirstInSecondChain( true );
Expand All @@ -105,7 +107,7 @@ public boolean process( RelationshipRecord record, int id, int processors )
{
long firstPrevRel = cache.getAndPutRelationship( record.getFirstNode(),
Direction.OUTGOING, record.getId(), false );
if ( firstPrevRel == -1 )
if ( firstPrevRel == ID_NOT_FOUND )
{ // First one
record.setFirstInFirstChain( true );
firstPrevRel = cache.getCount( record.getFirstNode(), Direction.OUTGOING );
Expand All @@ -123,7 +125,7 @@ public boolean process( RelationshipRecord record, int id, int processors )
{
long secondPrevRel = cache.getAndPutRelationship( record.getSecondNode(),
Direction.INCOMING, record.getId(), false );
if ( secondPrevRel == -1 )
if ( secondPrevRel == ID_NOT_FOUND )
{ // First one
record.setFirstInSecondChain( true );
secondPrevRel = cache.getCount( record.getSecondNode(), Direction.INCOMING );
Expand Down
Expand Up @@ -28,6 +28,8 @@
import org.neo4j.unsafe.impl.batchimport.staging.StageControl;
import org.neo4j.unsafe.impl.batchimport.store.BatchingTokenRepository.BatchingRelationshipTypeTokenRepository;

import static org.neo4j.unsafe.impl.batchimport.cache.idmapping.IdMapper.ID_NOT_FOUND;

/**
* Creates and initializes {@link RelationshipRecord} batches to later be filled with actual data
* and pointers. This is a separate step to remove work from main step.
Expand All @@ -54,7 +56,7 @@ protected void process( Batch<InputRelationship,RelationshipRecord> batch, Batch
InputRelationship batchRelationship = batch.input[i];
long startNodeId = batch.ids[idIndex++];
long endNodeId = batch.ids[idIndex++];
if ( startNodeId == -1 || endNodeId == -1 )
if ( startNodeId == ID_NOT_FOUND || endNodeId == ID_NOT_FOUND )
{
relationship.setInUse( false );
}
Expand Down
Expand Up @@ -28,6 +28,9 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
import java.util.stream.Stream;

import org.neo4j.kernel.impl.store.record.RelationshipRecord;
import org.neo4j.unsafe.impl.batchimport.input.InputRelationship;
import org.neo4j.unsafe.impl.batchimport.staging.BatchSender;
Expand All @@ -36,12 +39,15 @@
import org.neo4j.unsafe.impl.batchimport.staging.StageControl;
import org.neo4j.unsafe.impl.batchimport.store.BatchingTokenRepository.BatchingRelationshipTypeTokenRepository;

import static java.lang.Thread.currentThread;

/**
* Counts relationships per type to later be able to provide all types, even sorted in descending order
* of number of relationships per type.
*/
public class RelationshipTypeCheckerStep extends ProcessorStep<Batch<InputRelationship,RelationshipRecord>>
{
private static final Function<Object,MutableLong> NEW_MUTABLE_LONG = type -> new MutableLong();
private static final Comparator<Map.Entry<Object,MutableLong>> SORT_BY_COUNT_DESC =
(e1,e2) -> Long.compare( e2.getValue().longValue(), e1.getValue().longValue() );
private static final Comparator<Map.Entry<Object,MutableLong>> SORT_BY_ID_DESC =
Expand All @@ -60,22 +66,10 @@ public RelationshipTypeCheckerStep( StageControl control, Configuration config,
@Override
protected void process( Batch<InputRelationship,RelationshipRecord> batch, BatchSender sender ) throws Throwable
{
Map<Object,MutableLong> types = typeCheckers.get( Thread.currentThread() );
if ( types == null )
{
typeCheckers.put( Thread.currentThread(), types = new HashMap<>() );
}

for ( InputRelationship relationship : batch.input )
{
Object type = relationship.typeAsObject();
MutableLong count = types.get( type );
if ( count == null )
{
types.put( type, count = new MutableLong() );
}
count.increment();
}
Map<Object,MutableLong> typeMap = typeCheckers.computeIfAbsent( currentThread(), ( t ) -> new HashMap<>() );
Stream.of( batch.input )
.map( InputRelationship::typeAsObject )
.forEach( type -> typeMap.computeIfAbsent( type, NEW_MUTABLE_LONG ).increment() );
sender.send( batch );
}

Expand All @@ -84,18 +78,9 @@ protected void process( Batch<InputRelationship,RelationshipRecord> batch, Batch
protected void done()
{
Map<Object,MutableLong> mergedTypes = new HashMap<>();
for ( Map<Object,MutableLong> localTypes : typeCheckers.values() )
{
for ( Map.Entry<Object,MutableLong> localType : localTypes.entrySet() )
{
MutableLong count = mergedTypes.get( localType.getKey() );
if ( count == null )
{
mergedTypes.put( localType.getKey(), count = new MutableLong() );
}
count.add( localType.getValue().longValue() );
}
}
typeCheckers.forEach( (thread,localTypes) ->
localTypes.forEach( (type,localCount) ->
mergedTypes.computeIfAbsent( type, t -> new MutableLong() ).add( localCount.longValue() ) ) );

sortedTypes = mergedTypes.entrySet().toArray( new Map.Entry[mergedTypes.size()] );
if ( sortedTypes.length > 0 )
Expand Down
Expand Up @@ -412,7 +412,7 @@ private static class RelGroupCache implements AutoCloseable, MemoryStatsVisitor.
{
this.base = base;
assert chunkSize > 0;
this.array = arrayFactory.newDynamicByteArray( 1_000_000,
this.array = arrayFactory.newDynamicByteArray( chunkSize,
minusOneBytes( ID_SIZE/*next*/ + (ID_AND_COUNT_SIZE) * Direction.values().length ) );
this.nextFreeId = new AtomicLong( base );
}
Expand Down
Expand Up @@ -33,6 +33,8 @@
*/
public interface IdMapper extends MemoryStatsVisitor.Visitable
{
long ID_NOT_FOUND = -1;

/**
* Maps an {@code inputId} to an actual node id.
* @param inputId an id of an unknown type, coming from input.
Expand Down

0 comments on commit 339366a

Please sign in to comment.