Skip to content

Commit

Permalink
Reverts previous addition of batching argument to LSS#writer
Browse files Browse the repository at this point in the history
since the only implementation using it (NativeLabelScanStore) saw virtually
no use of it anymore.

Also keeps one NativeLabelScanWriter and initializes it for every tree writer,
instead of creating new instance. This works because the underlying tree works
exactly the same way and this should produce zero garbage.
  • Loading branch information
tinwelint committed Jan 12, 2017
1 parent c5f68e7 commit 2a4863f
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 32 deletions.
Expand Up @@ -95,19 +95,7 @@ public void rebuilt( long roughNodeCount )
* *
* @return {@link LabelScanWriter} which can modify the {@link LabelScanStore}. * @return {@link LabelScanWriter} which can modify the {@link LabelScanStore}.
*/ */
default LabelScanWriter newWriter() LabelScanWriter newWriter();
{
return newWriter( false );
}

/**
* Acquire a writer for updating the store.
*
* @param batching {@code true} means that the writes to this writer will be in batch-style,
* typically a store scan, so sequentially ordered and lots of them.
* @return {@link LabelScanWriter} which can modify the {@link LabelScanStore}.
*/
LabelScanWriter newWriter( boolean batching );


/** /**
* Forces all changes to disk. Called at certain points from within Neo4j for example when * Forces all changes to disk. Called at certain points from within Neo4j for example when
Expand Down
Expand Up @@ -102,7 +102,7 @@ public static FullStoreChangeStream fullStoreLabelUpdateStream( Supplier<IndexSt


public static long rebuild( LabelScanStore store, FullStoreChangeStream fullStoreStream ) throws IOException public static long rebuild( LabelScanStore store, FullStoreChangeStream fullStoreStream ) throws IOException
{ {
try ( LabelScanWriter writer = store.newWriter( true ) ) try ( LabelScanWriter writer = store.newWriter() )
{ {
return fullStoreStream.applyTo( writer ); return fullStoreStream.applyTo( writer );
} }
Expand Down
Expand Up @@ -25,7 +25,6 @@
import org.neo4j.cursor.RawCursor; import org.neo4j.cursor.RawCursor;
import org.neo4j.graphdb.ResourceIterator; import org.neo4j.graphdb.ResourceIterator;
import org.neo4j.index.Hit; import org.neo4j.index.Hit;
import org.neo4j.index.IndexWriter;
import org.neo4j.index.gbptree.GBPTree; import org.neo4j.index.gbptree.GBPTree;
import org.neo4j.index.gbptree.Layout; import org.neo4j.index.gbptree.Layout;
import org.neo4j.io.pagecache.IOLimiter; import org.neo4j.io.pagecache.IOLimiter;
Expand Down Expand Up @@ -113,6 +112,8 @@ public class NativeLabelScanStore implements LabelScanStore
*/ */
private boolean recoveryStarted; private boolean recoveryStarted;


private final NativeLabelScanWriter singleWriter;

public NativeLabelScanStore( PageCache pageCache, File storeDir, int rangeSize, int pageSize, public NativeLabelScanStore( PageCache pageCache, File storeDir, int rangeSize, int pageSize,
FullStoreChangeStream fullStoreChangeStream ) FullStoreChangeStream fullStoreChangeStream )
{ {
Expand All @@ -121,6 +122,7 @@ public NativeLabelScanStore( PageCache pageCache, File storeDir, int rangeSize,
this.fullStoreChangeStream = fullStoreChangeStream; this.fullStoreChangeStream = fullStoreChangeStream;
this.storeFile = new File( storeDir, DEFAULT_NAME + ".labelscanstore.db" ); this.storeFile = new File( storeDir, DEFAULT_NAME + ".labelscanstore.db" );
this.rangeSize = rangeSize; this.rangeSize = rangeSize;
this.singleWriter = new NativeLabelScanWriter( rangeSize, 1_000 );
} }


/** /**
Expand All @@ -139,14 +141,12 @@ public LabelScanReader newReader()
* Returns {@link LabelScanWriter} capable of making changes to this {@link LabelScanStore}. * Returns {@link LabelScanWriter} capable of making changes to this {@link LabelScanStore}.
* Only a single writer is allowed at any given point in time. * Only a single writer is allowed at any given point in time.
* *
* @param batching whether or not the returned writer will be used for batch-insertion where
* updates, usually a large amount, will arrive in sorted order.
* @return {@link LabelScanWriter} capable of making changes to this {@link LabelScanStore}. * @return {@link LabelScanWriter} capable of making changes to this {@link LabelScanStore}.
* @throws IllegalStateException if someone else has already acquired a writer and hasn't yet * @throws IllegalStateException if someone else has already acquired a writer and hasn't yet
* called {@link LabelScanWriter#close()}. * called {@link LabelScanWriter#close()}.
*/ */
@Override @Override
public LabelScanWriter newWriter( boolean batching ) public LabelScanWriter newWriter()
{ {
try try
{ {
Expand All @@ -158,8 +158,7 @@ public LabelScanWriter newWriter( boolean batching )
recoveryStarted = true; recoveryStarted = true;
} }


IndexWriter<LabelScanKey,LabelScanValue> inserter = index.writer(); return singleWriter.initialize( index.writer() );
return new NativeLabelScanWriter( inserter, rangeSize, batching ? 10_000 : 1_000 );
} }
catch ( IOException e ) catch ( IOException e )
{ {
Expand Down
Expand Up @@ -69,9 +69,9 @@ class NativeLabelScanWriter implements LabelScanWriter


/** /**
* {@link IndexWriter} acquired when acquiring this {@link NativeLabelScanWriter}, * {@link IndexWriter} acquired when acquiring this {@link NativeLabelScanWriter},
* acquired from {@link GBPTree#writer(org.neo4j.index.IndexWriter.Options)}. * acquired from {@link GBPTree#writer()}.
*/ */
private final IndexWriter<LabelScanKey,LabelScanValue> inserter; private IndexWriter<LabelScanKey,LabelScanValue> writer;


/** /**
* Instance of {@link LabelScanKey} acting as place to read keys into and also to set for each applied update. * Instance of {@link LabelScanKey} acting as place to read keys into and also to set for each applied update.
Expand Down Expand Up @@ -118,15 +118,23 @@ class NativeLabelScanWriter implements LabelScanWriter
* For each round the current round tries to figure out which is the closest higher labelId to apply * For each round the current round tries to figure out which is the closest higher labelId to apply
* in the next round. This variable keeps track of that next labelId. * in the next round. This variable keeps track of that next labelId.
*/ */
private long lowestLabelId = Long.MAX_VALUE; private long lowestLabelId;


NativeLabelScanWriter( IndexWriter<LabelScanKey,LabelScanValue> inserter, int rangeSize, int batchSize ) NativeLabelScanWriter( int rangeSize, int batchSize )
{ {
this.inserter = inserter;
this.rangeSize = rangeSize; this.rangeSize = rangeSize;
this.pendingUpdates = new NodeLabelUpdate[batchSize]; this.pendingUpdates = new NodeLabelUpdate[batchSize];
} }


NativeLabelScanWriter initialize( IndexWriter<LabelScanKey,LabelScanValue> writer )
{
this.writer = writer;
this.pendingUpdatesCursor = 0;
this.addition = false;
this.lowestLabelId = Long.MAX_VALUE;
return this;
}

/** /**
* Queues a {@link NodeLabelUpdate} to this writer for applying when batch gets full, * Queues a {@link NodeLabelUpdate} to this writer for applying when batch gets full,
* or when {@link #close() closing}. * or when {@link #close() closing}.
Expand Down Expand Up @@ -241,7 +249,7 @@ private void flushPendingRange() throws IOException
if ( value.bits != 0 ) if ( value.bits != 0 )
{ {
// There are changes in the current range, flush them // There are changes in the current range, flush them
inserter.merge( key, value, addition ? ADD_MERGER : REMOVE_MERGER ); writer.merge( key, value, addition ? ADD_MERGER : REMOVE_MERGER );
// TODO: after a remove we could check if the tree value is empty and if so remove it from the index // TODO: after a remove we could check if the tree value is empty and if so remove it from the index
// hmm, or perhaps that could be a feature of ValueAmender? // hmm, or perhaps that could be a feature of ValueAmender?
value.clear(); value.clear();
Expand All @@ -266,7 +274,7 @@ public void close() throws IOException
} }
finally finally
{ {
inserter.close(); writer.close();
} }
} }
} }
Expand Up @@ -189,7 +189,7 @@ public void shutdown()
} }


@Override @Override
public LabelScanWriter newWriter( boolean batching ) public LabelScanWriter newWriter()
{ {
return new InMemoryLabelScanWriter(); return new InMemoryLabelScanWriter();
} }
Expand Down
Expand Up @@ -65,8 +65,10 @@ public void shouldAddLabels() throws Exception
// GIVEN // GIVEN
ControlledInserter inserter = new ControlledInserter(); ControlledInserter inserter = new ControlledInserter();
long[] expected = new long[NODE_COUNT]; long[] expected = new long[NODE_COUNT];
try ( NativeLabelScanWriter writer = new NativeLabelScanWriter( inserter, RANGE_SIZE, max( 5, NODE_COUNT/100 ) ) ) try ( NativeLabelScanWriter writer = new NativeLabelScanWriter( RANGE_SIZE, max( 5, NODE_COUNT/100 ) ) )
{ {
writer.initialize( inserter );

// WHEN // WHEN
for ( int i = 0; i < NODE_COUNT * 3; i++ ) for ( int i = 0; i < NODE_COUNT * 3; i++ )
{ {
Expand All @@ -90,8 +92,10 @@ public void shouldNotAcceptUnsortedLabels() throws Exception
// GIVEN // GIVEN
ControlledInserter inserter = new ControlledInserter(); ControlledInserter inserter = new ControlledInserter();
boolean failed = false; boolean failed = false;
try ( NativeLabelScanWriter writer = new NativeLabelScanWriter( inserter, RANGE_SIZE, 1 ) ) try ( NativeLabelScanWriter writer = new NativeLabelScanWriter( RANGE_SIZE, 1 ) )
{ {
writer.initialize( inserter );

// WHEN // WHEN
writer.write( NodeLabelUpdate.labelChanges( 0, EMPTY_LONG_ARRAY, new long[] {2, 1} ) ); writer.write( NodeLabelUpdate.labelChanges( 0, EMPTY_LONG_ARRAY, new long[] {2, 1} ) );
// we can't do the usual "fail( blabla )" here since the actual write will happen // we can't do the usual "fail( blabla )" here since the actual write will happen
Expand Down
Expand Up @@ -1503,7 +1503,7 @@ public void shutdown() throws IOException
} }


@Override @Override
public LabelScanWriter newWriter( boolean batching ) public LabelScanWriter newWriter()
{ {
writersCreated++; writersCreated++;
return new LabelScanWriter() return new LabelScanWriter()
Expand Down
Expand Up @@ -148,7 +148,7 @@ public void shutdown() throws IOException
} }


@Override @Override
public LabelScanWriter newWriter( boolean batching ) public LabelScanWriter newWriter()
{ {
return luceneIndex.getLabelScanWriter(); return luceneIndex.getLabelScanWriter();
} }
Expand Down

0 comments on commit 2a4863f

Please sign in to comment.