From 2a4863f870fa4941181ee1063213a756bfddf56e Mon Sep 17 00:00:00 2001 From: Mattias Persson Date: Thu, 12 Jan 2017 08:22:32 +0100 Subject: [PATCH] Reverts previous addition of batching argument to LSS#writer 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. --- .../kernel/api/labelscan/LabelScanStore.java | 14 +----------- .../impl/api/scan/LabelScanStoreProvider.java | 2 +- .../index/labelscan/NativeLabelScanStore.java | 11 +++++----- .../labelscan/NativeLabelScanWriter.java | 22 +++++++++++++------ .../impl/api/scan/InMemoryLabelScanStore.java | 2 +- .../labelscan/NativeLabelScanWriterTest.java | 8 +++++-- .../batchinsert/internal/BatchInsertTest.java | 2 +- .../impl/labelscan/LuceneLabelScanStore.java | 2 +- 8 files changed, 31 insertions(+), 32 deletions(-) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/labelscan/LabelScanStore.java b/community/kernel/src/main/java/org/neo4j/kernel/api/labelscan/LabelScanStore.java index f4fbf86205423..058cbeda13e40 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/labelscan/LabelScanStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/labelscan/LabelScanStore.java @@ -95,19 +95,7 @@ public void rebuilt( long roughNodeCount ) * * @return {@link LabelScanWriter} which can modify the {@link LabelScanStore}. */ - default 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 ); + LabelScanWriter newWriter(); /** * Forces all changes to disk. Called at certain points from within Neo4j for example when diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/scan/LabelScanStoreProvider.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/scan/LabelScanStoreProvider.java index a45fb79e9fa92..c42092ec3cbde 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/scan/LabelScanStoreProvider.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/scan/LabelScanStoreProvider.java @@ -102,7 +102,7 @@ public static FullStoreChangeStream fullStoreLabelUpdateStream( Supplier inserter = index.writer(); - return new NativeLabelScanWriter( inserter, rangeSize, batching ? 10_000 : 1_000 ); + return singleWriter.initialize( index.writer() ); } catch ( IOException e ) { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/labelscan/NativeLabelScanWriter.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/labelscan/NativeLabelScanWriter.java index d36726fa86f26..ba301327a9b9c 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/labelscan/NativeLabelScanWriter.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/labelscan/NativeLabelScanWriter.java @@ -69,9 +69,9 @@ class NativeLabelScanWriter implements LabelScanWriter /** * {@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 inserter; + private IndexWriter writer; /** * Instance of {@link LabelScanKey} acting as place to read keys into and also to set for each applied update. @@ -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 * in the next round. This variable keeps track of that next labelId. */ - private long lowestLabelId = Long.MAX_VALUE; + private long lowestLabelId; - NativeLabelScanWriter( IndexWriter inserter, int rangeSize, int batchSize ) + NativeLabelScanWriter( int rangeSize, int batchSize ) { - this.inserter = inserter; this.rangeSize = rangeSize; this.pendingUpdates = new NodeLabelUpdate[batchSize]; } + NativeLabelScanWriter initialize( IndexWriter 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, * or when {@link #close() closing}. @@ -241,7 +249,7 @@ private void flushPendingRange() throws IOException if ( value.bits != 0 ) { // 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 // hmm, or perhaps that could be a feature of ValueAmender? value.clear(); @@ -266,7 +274,7 @@ public void close() throws IOException } finally { - inserter.close(); + writer.close(); } } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/scan/InMemoryLabelScanStore.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/scan/InMemoryLabelScanStore.java index a7e6ed334f2b3..33fef082546c8 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/scan/InMemoryLabelScanStore.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/scan/InMemoryLabelScanStore.java @@ -189,7 +189,7 @@ public void shutdown() } @Override - public LabelScanWriter newWriter( boolean batching ) + public LabelScanWriter newWriter() { return new InMemoryLabelScanWriter(); } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/labelscan/NativeLabelScanWriterTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/labelscan/NativeLabelScanWriterTest.java index ae2a9eff5098a..ee44751e8f176 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/labelscan/NativeLabelScanWriterTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/labelscan/NativeLabelScanWriterTest.java @@ -65,8 +65,10 @@ public void shouldAddLabels() throws Exception // GIVEN ControlledInserter inserter = new ControlledInserter(); 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 for ( int i = 0; i < NODE_COUNT * 3; i++ ) { @@ -90,8 +92,10 @@ public void shouldNotAcceptUnsortedLabels() throws Exception // GIVEN ControlledInserter inserter = new ControlledInserter(); boolean failed = false; - try ( NativeLabelScanWriter writer = new NativeLabelScanWriter( inserter, RANGE_SIZE, 1 ) ) + try ( NativeLabelScanWriter writer = new NativeLabelScanWriter( RANGE_SIZE, 1 ) ) { + writer.initialize( inserter ); + // WHEN 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 diff --git a/community/kernel/src/test/java/org/neo4j/unsafe/batchinsert/internal/BatchInsertTest.java b/community/kernel/src/test/java/org/neo4j/unsafe/batchinsert/internal/BatchInsertTest.java index 699efa64fc534..945d0fadec2e1 100644 --- a/community/kernel/src/test/java/org/neo4j/unsafe/batchinsert/internal/BatchInsertTest.java +++ b/community/kernel/src/test/java/org/neo4j/unsafe/batchinsert/internal/BatchInsertTest.java @@ -1503,7 +1503,7 @@ public void shutdown() throws IOException } @Override - public LabelScanWriter newWriter( boolean batching ) + public LabelScanWriter newWriter() { writersCreated++; return new LabelScanWriter() diff --git a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/labelscan/LuceneLabelScanStore.java b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/labelscan/LuceneLabelScanStore.java index 9e3a1a12a27eb..35da7255e0f1f 100644 --- a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/labelscan/LuceneLabelScanStore.java +++ b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/labelscan/LuceneLabelScanStore.java @@ -148,7 +148,7 @@ public void shutdown() throws IOException } @Override - public LabelScanWriter newWriter( boolean batching ) + public LabelScanWriter newWriter() { return luceneIndex.getLabelScanWriter(); }