Skip to content

Commit

Permalink
Less IndexReaderFactory instances on StoreStatement
Browse files Browse the repository at this point in the history
so only created if they are actually used, when caller needs an
IndexReader or LabelScanReader.
  • Loading branch information
tinwelint committed Jan 22, 2016
1 parent 290c7ad commit a69d172
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 36 deletions.
Expand Up @@ -33,7 +33,6 @@
import org.neo4j.kernel.api.txstate.TxStateHolder; import org.neo4j.kernel.api.txstate.TxStateHolder;
import org.neo4j.kernel.impl.locking.Locks; import org.neo4j.kernel.impl.locking.Locks;
import org.neo4j.storageengine.api.StorageStatement; import org.neo4j.storageengine.api.StorageStatement;
import org.neo4j.storageengine.api.schema.LabelScanReader;


public class KernelStatement implements TxStateHolder, Statement public class KernelStatement implements TxStateHolder, Statement
{ {
Expand All @@ -42,12 +41,12 @@ public class KernelStatement implements TxStateHolder, Statement
private final StorageStatement storeStatement; private final StorageStatement storeStatement;
private final KernelTransactionImplementation transaction; private final KernelTransactionImplementation transaction;
private final OperationsFacade facade; private final OperationsFacade facade;
private LabelScanReader labelScanReader;
private int referenceCount; private int referenceCount;
private boolean closed; private boolean closed;


public KernelStatement( KernelTransactionImplementation transaction, TxStateHolder txStateHolder, public KernelStatement( KernelTransactionImplementation transaction,
Locks.Client locks, StatementOperationParts operations, StorageStatement storeStatement, Procedures procedures ) TxStateHolder txStateHolder, Locks.Client locks,
StatementOperationParts operations, StorageStatement storeStatement, Procedures procedures )
{ {
this.transaction = transaction; this.transaction = transaction;
this.locks = locks; this.locks = locks;
Expand Down Expand Up @@ -155,12 +154,6 @@ final void forceClose()
private void cleanupResources() private void cleanupResources()
{ {
storeStatement.close(); storeStatement.close();

if ( null != labelScanReader )
{
labelScanReader.close();
}

transaction.releaseStatement( this ); transaction.releaseStatement( this );
} }


Expand Down
Expand Up @@ -58,18 +58,19 @@ public class StoreStatement implements StorageStatement
private final NeoStores neoStores; private final NeoStores neoStores;
private final NodeStore nodeStore; private final NodeStore nodeStore;
private final RelationshipStore relationshipStore; private final RelationshipStore relationshipStore;
private final IndexReaderFactory indexReaderFactory; private final Supplier<IndexReaderFactory> indexReaderFactorySupplier;
private final Supplier<LabelScanReader> labelScanReaderSupplier; private IndexReaderFactory indexReaderFactory;
private final Supplier<LabelScanReader> labelScanStore;
private LabelScanReader labelScanReader; private LabelScanReader labelScanReader;
private boolean closed; private boolean closed;


public StoreStatement( final NeoStores neoStores, final LockService lockService, public StoreStatement( final NeoStores neoStores, final LockService lockService,
IndexReaderFactory indexReaderFactory, Supplier<IndexReaderFactory> indexReaderFactory,
Supplier<LabelScanReader> labelScanReaderSupplier ) Supplier<LabelScanReader> labelScanReaderSupplier )
{ {
this.neoStores = neoStores; this.neoStores = neoStores;
this.indexReaderFactory = indexReaderFactory; this.indexReaderFactorySupplier = indexReaderFactory;
this.labelScanReaderSupplier = labelScanReaderSupplier; this.labelScanStore = labelScanReaderSupplier;
this.nodeStore = neoStores.getNodeStore(); this.nodeStore = neoStores.getNodeStore();
this.relationshipStore = neoStores.getRelationshipStore(); this.relationshipStore = neoStores.getRelationshipStore();


Expand Down Expand Up @@ -155,7 +156,10 @@ public Cursor<RelationshipItem> relationshipsGetAllCursor()
public void close() public void close()
{ {
assert !closed; assert !closed;
indexReaderFactory.close(); if ( indexReaderFactory != null )
{
indexReaderFactory.close();
}
if ( labelScanReader != null ) if ( labelScanReader != null )
{ {
labelScanReader.close(); labelScanReader.close();
Expand Down Expand Up @@ -209,22 +213,25 @@ protected boolean fetchNext()
@Override @Override
public LabelScanReader getLabelScanReader() public LabelScanReader getLabelScanReader()
{ {
if ( labelScanReader == null ) return labelScanReader != null ?
{ labelScanReader : (labelScanReader = labelScanStore.get());
labelScanReader = labelScanReaderSupplier.get(); }
}
return labelScanReader; private IndexReaderFactory indexReaderFactory()
{
return indexReaderFactory != null ?
indexReaderFactory : (indexReaderFactory = indexReaderFactorySupplier.get());
} }


@Override @Override
public IndexReader getIndexReader( IndexDescriptor descriptor ) throws IndexNotFoundKernelException public IndexReader getIndexReader( IndexDescriptor descriptor ) throws IndexNotFoundKernelException
{ {
return indexReaderFactory.newReader( descriptor ); return indexReaderFactory().newReader( descriptor );
} }


@Override @Override
public IndexReader getFreshIndexReader( IndexDescriptor descriptor ) throws IndexNotFoundKernelException public IndexReader getFreshIndexReader( IndexDescriptor descriptor ) throws IndexNotFoundKernelException
{ {
return indexReaderFactory.newUnCachedReader( descriptor ); return indexReaderFactory().newUnCachedReader( descriptor );
} }
} }
Expand Up @@ -106,7 +106,6 @@
import org.neo4j.storageengine.api.StoreReadLayer; import org.neo4j.storageengine.api.StoreReadLayer;
import org.neo4j.storageengine.api.TransactionApplicationMode; import org.neo4j.storageengine.api.TransactionApplicationMode;
import org.neo4j.storageengine.api.lock.ResourceLocker; import org.neo4j.storageengine.api.lock.ResourceLocker;
import org.neo4j.storageengine.api.schema.LabelScanReader;
import org.neo4j.storageengine.api.schema.SchemaRule; import org.neo4j.storageengine.api.schema.SchemaRule;
import org.neo4j.storageengine.api.txstate.ReadableTransactionState; import org.neo4j.storageengine.api.txstate.ReadableTransactionState;
import org.neo4j.storageengine.api.txstate.TxStateVisitor; import org.neo4j.storageengine.api.txstate.TxStateVisitor;
Expand Down Expand Up @@ -218,6 +217,7 @@ public RecordStorageEngine(
schemaStorage, neoStores, indexingService, schemaStorage, neoStores, indexingService,
storeStatementSupplier( neoStores, config, lockService ) ); storeStatementSupplier( neoStores, config, lockService ) );
storeLayer = new CacheLayer( diskLayer, schemaCache ); storeLayer = new CacheLayer( diskLayer, schemaCache );

legacyIndexApplierLookup = new LegacyIndexApplierLookup.Direct( legacyIndexProviderLookup ); legacyIndexApplierLookup = new LegacyIndexApplierLookup.Direct( legacyIndexProviderLookup );


labelScanStoreSync = new WorkSync<>( labelScanStore::newWriter ); labelScanStoreSync = new WorkSync<>( labelScanStore::newWriter );
Expand Down Expand Up @@ -248,11 +248,12 @@ private Supplier<StorageStatement> storeStatementSupplier(
{ {
final LockService currentLockService = final LockService currentLockService =
config.get( use_read_locks_on_property_reads ) ? lockService : NO_LOCK_SERVICE; config.get( use_read_locks_on_property_reads ) ? lockService : NO_LOCK_SERVICE;
final Supplier<LabelScanReader> labelScanReaderSupplier = labelScanStore::newReader; final Supplier<IndexReaderFactory> indexReaderFactory = () -> {
return new IndexReaderFactory.Caching( indexingService );
};


return () -> { return () -> {
return new StoreStatement( neoStores, currentLockService, return new StoreStatement( neoStores, currentLockService, indexReaderFactory, labelScanStore::newReader );
new IndexReaderFactory.Caching( indexingService ), labelScanReaderSupplier );
}; };
} }


Expand Down
Expand Up @@ -37,7 +37,7 @@ public void shouldThrowTerminateExceptionWhenTransactionTerminated() throws Exce
when( transaction.shouldBeTerminated() ).thenReturn( true ); when( transaction.shouldBeTerminated() ).thenReturn( true );


KernelStatement statement = new KernelStatement( KernelStatement statement = new KernelStatement(
transaction, null, null, null, null, new Procedures() ); transaction, null, null, null, null, null );


statement.readOperations().nodeExists( 0 ); statement.readOperations().nodeExists( 0 );
} }
Expand Down
Expand Up @@ -24,6 +24,7 @@
import org.mockito.stubbing.Answer; import org.mockito.stubbing.Answer;


import java.util.Collection; import java.util.Collection;
import java.util.function.Supplier;


import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.KernelTransaction;
import org.neo4j.kernel.api.exceptions.TransactionFailureException; import org.neo4j.kernel.api.exceptions.TransactionFailureException;
Expand Down Expand Up @@ -209,7 +210,7 @@ private static KernelTransactions newKernelTransactions( TransactionCommitProces
NeoStores neoStores = mock( NeoStores.class ); NeoStores neoStores = mock( NeoStores.class );


StoreStatement storeStatement = new StoreStatement( neoStores, new ReentrantLockService(), StoreStatement storeStatement = new StoreStatement( neoStores, new ReentrantLockService(),
mock( IndexReaderFactory.class ), null ); mock( Supplier.class ), null );
StoreReadLayer readLayer = mock( StoreReadLayer.class ); StoreReadLayer readLayer = mock( StoreReadLayer.class );
when( readLayer.acquireStatement() ).thenReturn( storeStatement ); when( readLayer.acquireStatement() ).thenReturn( storeStatement );


Expand Down
Expand Up @@ -48,7 +48,7 @@
/** /**
* Base class for disk layer tests, which test read-access to committed data. * Base class for disk layer tests, which test read-access to committed data.
*/ */
public class DiskLayerTest public abstract class DiskLayerTest
{ {
@SuppressWarnings( "deprecation" ) @SuppressWarnings( "deprecation" )
protected GraphDatabaseAPI db; protected GraphDatabaseAPI db;
Expand Down
Expand Up @@ -21,8 +21,8 @@


import org.junit.Test; import org.junit.Test;


import org.neo4j.kernel.api.labelscan.LabelScanStore; import java.util.function.Supplier;
import org.neo4j.kernel.impl.api.IndexReaderFactory;
import org.neo4j.kernel.impl.locking.LockService; import org.neo4j.kernel.impl.locking.LockService;
import org.neo4j.kernel.impl.store.NeoStores; import org.neo4j.kernel.impl.store.NeoStores;
import org.neo4j.storageengine.api.schema.LabelScanReader; import org.neo4j.storageengine.api.schema.LabelScanReader;
Expand All @@ -39,13 +39,13 @@ public class StoreStatementTest
public void shouldCloseOpenedLabelScanReader() throws Exception public void shouldCloseOpenedLabelScanReader() throws Exception
{ {
// given // given
LabelScanStore scanStore = mock( LabelScanStore.class ); Supplier<LabelScanReader> scanStore = mock( Supplier.class );
LabelScanReader scanReader = mock( LabelScanReader.class ); LabelScanReader scanReader = mock( LabelScanReader.class );


when( scanStore.newReader() ).thenReturn( scanReader ); when( scanStore.get() ).thenReturn( scanReader );
StoreStatement statement = new StoreStatement( StoreStatement statement = new StoreStatement(
mock( NeoStores.class ), LockService.NO_LOCK_SERVICE, mock( NeoStores.class ), LockService.NO_LOCK_SERVICE,
mock( IndexReaderFactory.class ), scanStore::newReader ); mock( Supplier.class ), scanStore );


// when // when
LabelScanReader actualReader = statement.getLabelScanReader(); LabelScanReader actualReader = statement.getLabelScanReader();
Expand All @@ -57,7 +57,7 @@ public void shouldCloseOpenedLabelScanReader() throws Exception
statement.close(); statement.close();


// then // then
verify( scanStore ).newReader(); verify( scanStore ).get();
verifyNoMoreInteractions( scanStore ); verifyNoMoreInteractions( scanStore );


verify( scanReader ).close(); verify( scanReader ).close();
Expand Down

0 comments on commit a69d172

Please sign in to comment.