From 09dbc905d361316b1475eb1e4508322b7735b5a2 Mon Sep 17 00:00:00 2001 From: Anton Persson Date: Fri, 2 Dec 2016 10:44:09 +0100 Subject: [PATCH] Address PR comments --- .../neo4j/index/gbptree/ByteArrayPageCursor.java | 3 +-- .../index/gbptree/PageAwareByteArrayCursor.java | 4 ++-- .../java/org/neo4j/io/pagecache/PageCursor.java | 3 +-- .../io/pagecache/impl/CompositePageCursor.java | 6 +++--- .../io/pagecache/impl/DelegatingPageCursor.java | 4 ++-- .../pagecache/impl/muninn/MuninnPageCursor.java | 3 +-- .../impl/muninn/MuninnReadPageCursor.java | 2 +- .../pagecache/AdversarialReadPageCursor.java | 2 +- .../pagecache/AdversarialWritePageCursor.java | 4 ++-- .../org/neo4j/io/pagecache/PageCacheTest.java | 15 +++++++++------ .../org/neo4j/io/pagecache/StubPageCursor.java | 5 ++--- .../neo4j/io/pagecache/impl/ByteBufferPage.java | 15 +++------------ 12 files changed, 28 insertions(+), 38 deletions(-) diff --git a/community/index/src/main/java/org/neo4j/index/gbptree/ByteArrayPageCursor.java b/community/index/src/main/java/org/neo4j/index/gbptree/ByteArrayPageCursor.java index 7719b1225d1bd..fbfd312e91614 100644 --- a/community/index/src/main/java/org/neo4j/index/gbptree/ByteArrayPageCursor.java +++ b/community/index/src/main/java/org/neo4j/index/gbptree/ByteArrayPageCursor.java @@ -289,9 +289,8 @@ public PageCursor openLinkedCursor( long pageId ) } @Override - public void clear() + public void zapPage() { Arrays.fill( buffer.array(), (byte) 0 ); - buffer.position( 0 ); } } diff --git a/community/index/src/test/java/org/neo4j/index/gbptree/PageAwareByteArrayCursor.java b/community/index/src/test/java/org/neo4j/index/gbptree/PageAwareByteArrayCursor.java index fdd6f1ef6e77a..774e027b3358a 100644 --- a/community/index/src/test/java/org/neo4j/index/gbptree/PageAwareByteArrayCursor.java +++ b/community/index/src/test/java/org/neo4j/index/gbptree/PageAwareByteArrayCursor.java @@ -325,8 +325,8 @@ public PageCursor openLinkedCursor( long pageId ) } @Override - public void clear() + public void zapPage() { - current.clear(); + current.zapPage(); } } diff --git a/community/io/src/main/java/org/neo4j/io/pagecache/PageCursor.java b/community/io/src/main/java/org/neo4j/io/pagecache/PageCursor.java index 033c3fb853199..351242a6b13e1 100644 --- a/community/io/src/main/java/org/neo4j/io/pagecache/PageCursor.java +++ b/community/io/src/main/java/org/neo4j/io/pagecache/PageCursor.java @@ -344,7 +344,6 @@ public abstract class PageCursor implements AutoCloseable /** * Sets all bytes in this page to zero, as if this page was newly allocated at the end of the file. - * Offset is also set to 0. */ - public abstract void clear(); + public abstract void zapPage(); } diff --git a/community/io/src/main/java/org/neo4j/io/pagecache/impl/CompositePageCursor.java b/community/io/src/main/java/org/neo4j/io/pagecache/impl/CompositePageCursor.java index 6435369f2a55d..d612d05f96804 100644 --- a/community/io/src/main/java/org/neo4j/io/pagecache/impl/CompositePageCursor.java +++ b/community/io/src/main/java/org/neo4j/io/pagecache/impl/CompositePageCursor.java @@ -483,10 +483,10 @@ public PageCursor openLinkedCursor( long pageId ) } @Override - public void clear() + public void zapPage() { - first.clear(); - second.clear(); + first.zapPage(); + second.zapPage(); } /** diff --git a/community/io/src/main/java/org/neo4j/io/pagecache/impl/DelegatingPageCursor.java b/community/io/src/main/java/org/neo4j/io/pagecache/impl/DelegatingPageCursor.java index 5c897e9d6c9ca..8946ca57e94e1 100644 --- a/community/io/src/main/java/org/neo4j/io/pagecache/impl/DelegatingPageCursor.java +++ b/community/io/src/main/java/org/neo4j/io/pagecache/impl/DelegatingPageCursor.java @@ -255,9 +255,9 @@ public boolean shouldRetry() throws IOException } @Override - public void clear() + public void zapPage() { - delegate.clear(); + delegate.zapPage(); } public DelegatingPageCursor( PageCursor delegate ) diff --git a/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnPageCursor.java b/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnPageCursor.java index fa3acf80a04ae..2b59f4f9a87e5 100644 --- a/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnPageCursor.java +++ b/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnPageCursor.java @@ -778,7 +778,7 @@ public void setCursorException( String message ) } @Override - public void clear() + public void zapPage() { if ( pageSize == 0 ) { @@ -789,7 +789,6 @@ public void clear() else { UnsafeUtil.setMemory( pointer, pageSize, (byte) 0 ); - offset = 0; } } } diff --git a/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnReadPageCursor.java b/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnReadPageCursor.java index 82e0024bf4dc6..0e22bfc8fedbb 100644 --- a/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnReadPageCursor.java +++ b/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnReadPageCursor.java @@ -167,7 +167,7 @@ public void putShort( short value ) } @Override - public void clear() + public void zapPage() { throw new IllegalStateException( "Cannot write to read-locked page" ); } diff --git a/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialReadPageCursor.java b/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialReadPageCursor.java index 316b49af67cce..9c29bee296608 100644 --- a/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialReadPageCursor.java +++ b/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialReadPageCursor.java @@ -457,7 +457,7 @@ public PageCursor openLinkedCursor( long pageId ) } @Override - public void clear() + public void zapPage() { throw new IllegalStateException( "Cannot write using read cursor" ); } diff --git a/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialWritePageCursor.java b/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialWritePageCursor.java index 24fc46ae0414e..742fda66d080b 100644 --- a/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialWritePageCursor.java +++ b/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialWritePageCursor.java @@ -304,8 +304,8 @@ public PageCursor openLinkedCursor( long pageId ) } @Override - public void clear() + public void zapPage() { - delegate.clear(); + delegate.zapPage(); } } diff --git a/community/io/src/test/java/org/neo4j/io/pagecache/PageCacheTest.java b/community/io/src/test/java/org/neo4j/io/pagecache/PageCacheTest.java index 2d0a2074f8529..2b28972a99c24 100644 --- a/community/io/src/test/java/org/neo4j/io/pagecache/PageCacheTest.java +++ b/community/io/src/test/java/org/neo4j/io/pagecache/PageCacheTest.java @@ -1184,7 +1184,7 @@ private void verifyOnWriteCursor( testTemplate.accept( ( cursor ) -> cursor.putInt( 0, 1 ) ); testTemplate.accept( ( cursor ) -> cursor.putLong( 0, 1 ) ); testTemplate.accept( ( cursor ) -> cursor.putShort( 0, (short) 1 ) ); - testTemplate.accept( ( cursor ) -> cursor.clear() ); + testTemplate.accept( ( cursor ) -> cursor.zapPage() ); } private void checkUnboundReadCursorAccess( PageCursorAction action ) throws IOException @@ -2491,10 +2491,10 @@ public void cursorOffsetMustBeUpdatedReadAndWrite() throws IOException private void verifyWriteOffsets( PageCursor cursor ) { - assertThat( cursor.getOffset(), is( 0 ) ); cursor.setOffset( filePageSize / 2 ); - cursor.clear(); - assertThat( cursor.getOffset(), is( 0 ) ); + cursor.zapPage(); + assertThat( cursor.getOffset(), is( filePageSize / 2 ) ); + cursor.setOffset( 0 ); cursor.putLong( 1 ); assertThat( cursor.getOffset(), is( 8 ) ); cursor.putInt( 1 ); @@ -5398,7 +5398,7 @@ public void shouldZeroAllBytesOnClear() throws Exception try ( PageCursor cursor = pagedFile.io( pageId, PF_SHARED_WRITE_LOCK ) ) { assertTrue( cursor.next() ); - cursor.clear(); + cursor.zapPage(); byte[] read = new byte[filePageSize]; cursor.getBytes( read ); @@ -5411,7 +5411,10 @@ public void shouldZeroAllBytesOnClear() throws Exception assertTrue( cursor.next() ); byte[] read = new byte[filePageSize]; - cursor.getBytes( read ); + do + { + cursor.getBytes( read ); + } while ( cursor.shouldRetry() ); assertFalse( cursor.checkAndClearBoundsFlag() ); assertArrayEquals( allZeros, read ); } diff --git a/community/io/src/test/java/org/neo4j/io/pagecache/StubPageCursor.java b/community/io/src/test/java/org/neo4j/io/pagecache/StubPageCursor.java index 009717543a940..5dda4e72a58cd 100644 --- a/community/io/src/test/java/org/neo4j/io/pagecache/StubPageCursor.java +++ b/community/io/src/test/java/org/neo4j/io/pagecache/StubPageCursor.java @@ -399,10 +399,9 @@ public void setOffset( int offset ) } @Override - public void clear() + public void zapPage() { - page.clear(); - currentOffset = 0; + page.zapPage(); } public Page getPage() diff --git a/community/io/src/test/java/org/neo4j/io/pagecache/impl/ByteBufferPage.java b/community/io/src/test/java/org/neo4j/io/pagecache/impl/ByteBufferPage.java index 25379658120e8..579be6a2fe1b2 100644 --- a/community/io/src/test/java/org/neo4j/io/pagecache/impl/ByteBufferPage.java +++ b/community/io/src/test/java/org/neo4j/io/pagecache/impl/ByteBufferPage.java @@ -25,14 +25,13 @@ import java.nio.Buffer; import java.nio.ByteBuffer; -import org.neo4j.io.ByteUnit; import org.neo4j.io.pagecache.Page; +import org.neo4j.unsafe.impl.internal.dragons.UnsafeUtil; /** A page backed by a simple byte buffer. */ public class ByteBufferPage implements Page { private static final MethodHandle addressOfMH = addressOfMH(); - private static final byte[] ZEROS = new byte[(int) ByteUnit.kibiBytes( 1 )]; private static MethodHandle addressOfMH() { @@ -138,16 +137,8 @@ public long address() return addressOf( buffer ); } - public void clear() + public void zapPage() { - int remaining = buffer.capacity(); - buffer.position( 0 ); - while ( remaining >= ZEROS.length ) - { - buffer.put( ZEROS ); - remaining -= ZEROS.length; - } - buffer.put( ZEROS, 0, remaining ); - buffer.position( 0 ); + UnsafeUtil.setMemory( address(), size(), (byte) 0 ); } }