Skip to content

Commit

Permalink
Update PageList to make use of the new SwapperSet, and starton implem…
Browse files Browse the repository at this point in the history 
…enting eviction
  • Loading branch information
@chrisvest
chrisvest committed May 26, 2017
1 parent ac6b4bd commit be415f1
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 47 deletions.
26 changes: 16 additions & 10 deletions community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/PageList.java
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -62,14 +62,16 @@ class PageList
private final long pageCount; private final long pageCount;
private final int cachePageSize; private final int cachePageSize;
private final MemoryManager memoryManager; private final MemoryManager memoryManager;
private final SwapperSet swappers;
private final long victimPageAddress; private final long victimPageAddress;
private final long baseAddress; private final long baseAddress;


PageList( long pageCount, int cachePageSize, MemoryManager memoryManager, long victimPageAddress ) PageList( long pageCount, int cachePageSize, MemoryManager memoryManager, SwapperSet swappers, long victimPageAddress )
{ {
this.pageCount = pageCount; this.pageCount = pageCount;
this.cachePageSize = cachePageSize; this.cachePageSize = cachePageSize;
this.memoryManager = memoryManager; this.memoryManager = memoryManager;
this.swappers = swappers;
this.victimPageAddress = victimPageAddress; this.victimPageAddress = victimPageAddress;
long bytes = pageCount * META_DATA_BYTES_PER_PAGE; long bytes = pageCount * META_DATA_BYTES_PER_PAGE;
this.baseAddress = memoryManager.allocateAligned( bytes ); this.baseAddress = memoryManager.allocateAligned( bytes );
Expand All @@ -88,6 +90,7 @@ class PageList
this.pageCount = pageList.pageCount; this.pageCount = pageList.pageCount;
this.cachePageSize = pageList.cachePageSize; this.cachePageSize = pageList.cachePageSize;
this.memoryManager = pageList.memoryManager; this.memoryManager = pageList.memoryManager;
this.swappers = pageList.swappers;
this.victimPageAddress = pageList.victimPageAddress; this.victimPageAddress = pageList.victimPageAddress;
this.baseAddress = pageList.baseAddress; this.baseAddress = pageList.baseAddress;
} }
Expand Down Expand Up @@ -207,14 +210,14 @@ public int getCachePageSize()
return cachePageSize; return cachePageSize;
} }


public long address( long pageRef ) public long getAddress( long pageRef )
{ {
return UnsafeUtil.getLong( offAddress( pageRef ) ); return UnsafeUtil.getLong( offAddress( pageRef ) );
} }


public void initBuffer( long pageRef ) public void initBuffer( long pageRef )
{ {
if ( address( pageRef ) == 0L ) if ( getAddress( pageRef ) == 0L )
{ {
long addr = memoryManager.allocateAligned( getCachePageSize() ); long addr = memoryManager.allocateAligned( getCachePageSize() );
UnsafeUtil.putLong( offAddress( pageRef ), addr ); UnsafeUtil.putLong( offAddress( pageRef ), addr );
Expand Down Expand Up @@ -312,7 +315,7 @@ public void fault( long pageRef, PageSwapper swapper, int swapperId, long filePa
// the file page, so any subsequent thread that finds the page in their // the file page, so any subsequent thread that finds the page in their
// translation table will re-do the page fault. // translation table will re-do the page fault.
setFilePageId( pageRef, filePageId ); // Page now considered isLoaded() setFilePageId( pageRef, filePageId ); // Page now considered isLoaded()
long bytesRead = swapper.read( filePageId, address( pageRef ), cachePageSize ); long bytesRead = swapper.read( filePageId, getAddress( pageRef ), cachePageSize );
event.addBytesRead( bytesRead ); event.addBytesRead( bytesRead );
event.setCachePageId( (int) pageRef ); event.setCachePageId( (int) pageRef );
setSwapperId( pageRef, swapperId ); // Page now considered isBoundTo( swapper, filePageId ) setSwapperId( pageRef, swapperId ); // Page now considered isBoundTo( swapper, filePageId )
Expand All @@ -334,21 +337,24 @@ private static IllegalStateException cannotFaultException( long pageRef, PageSwa
return new IllegalStateException( msg ); return new IllegalStateException( msg );
} }


public boolean tryEvict( long pageRef, PageSwapper swapper ) public boolean tryEvict( long pageRef ) throws IOException
{ {
if ( swapper == null )
{
throw swapperCannotBeNull();
}
if ( tryExclusiveLock( pageRef ) ) if ( tryExclusiveLock( pageRef ) )
{ {
if ( isLoaded( pageRef ) ) if ( isLoaded( pageRef ) )
{ {
clearBinding( pageRef ); int swapperId = getSwapperId( pageRef );
SwapperSet.Allocation allocation = swappers.getAllocation( swapperId );
PageSwapper swapper = allocation.swapper;
long filePageId = getFilePageId( pageRef );
if ( isModified( pageRef ) ) if ( isModified( pageRef ) )
{ {
long address = getAddress( pageRef );
swapper.write( filePageId, address, allocation.filePageSize );
explicitlyMarkPageUnmodifiedUnderExclusiveLock( pageRef ); explicitlyMarkPageUnmodifiedUnderExclusiveLock( pageRef );
} }
swapper.evicted( filePageId, null );
clearBinding( pageRef );
return true; return true;
} }
else else
Expand Down
140 changes: 103 additions & 37 deletions community/io/src/test/java/org/neo4j/io/pagecache/impl/muninn/PageListTest.java
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import java.util.function.LongFunction; import java.util.function.LongFunction;


import org.neo4j.io.ByteUnit; import org.neo4j.io.ByteUnit;
import org.neo4j.io.pagecache.Page;
import org.neo4j.io.pagecache.PageCursor; import org.neo4j.io.pagecache.PageCursor;
import org.neo4j.io.pagecache.PageSwapper; import org.neo4j.io.pagecache.PageSwapper;
import org.neo4j.io.pagecache.tracing.DummyPageSwapper; import org.neo4j.io.pagecache.tracing.DummyPageSwapper;
Expand Down Expand Up @@ -104,6 +105,7 @@ public static void tearDownStatics()
private long prevPageRef; private long prevPageRef;
private long nextPageRef; private long nextPageRef;
private final int pageSize; private final int pageSize;
private SwapperSet swappers;
private PageList pageList; private PageList pageList;


public PageListTest( long pageId ) public PageListTest( long pageId )
Expand All @@ -117,7 +119,8 @@ public PageListTest( long pageId )
@Before @Before
public void setUp() public void setUp()
{ {
pageList = new PageList( pageIds.length, pageSize, mman, VictimPageReference.getVictimPage( pageSize ) ); swappers = new SwapperSet();
pageList = new PageList( pageIds.length, pageSize, mman, swappers, VictimPageReference.getVictimPage( pageSize ) );
pageRef = pageList.deref( pageId ); pageRef = pageList.deref( pageId );
prevPageRef = pageList.deref( prevPageId ); prevPageRef = pageList.deref( prevPageId );
nextPageRef = pageList.deref( nextPageId ); nextPageRef = pageList.deref( nextPageId );
Expand Down Expand Up @@ -864,14 +867,14 @@ public void allowExclusiveLockedPageToExplicitlyLowerModifiedFlag() throws Excep
@Test @Test
public void mustExposeCachePageSize() throws Exception public void mustExposeCachePageSize() throws Exception
{ {
PageList list = new PageList( 0, 42, mman, VictimPageReference.getVictimPage( 42 ) ); PageList list = new PageList( 0, 42, mman, swappers, VictimPageReference.getVictimPage( 42 ) );
assertThat( list.getCachePageSize(), is( 42 ) ); assertThat( list.getCachePageSize(), is( 42 ) );
} }


@Test @Test
public void addressesMustBeZeroBeforeInitialisation() throws Exception public void addressesMustBeZeroBeforeInitialisation() throws Exception
{ {
assertThat( pageList.address( pageRef ), is( 0L ) ); assertThat( pageList.getAddress( pageRef ), is( 0L ) );
} }


@Test @Test
Expand All @@ -889,19 +892,19 @@ public void initialisingBufferMustConsumeMemoryFromMemoryManager() throws Except
public void addressMustNotBeZeroAfterInitialisation() throws Exception public void addressMustNotBeZeroAfterInitialisation() throws Exception
{ {
pageList.initBuffer( pageRef ); pageList.initBuffer( pageRef );
assertThat( pageList.address( pageRef ), is( not( equalTo( 0L ) ) ) ); assertThat( pageList.getAddress( pageRef ), is( not( equalTo( 0L ) ) ) );
} }


@Test @Test
public void pageListMustBeCopyableViaConstructor() throws Exception public void pageListMustBeCopyableViaConstructor() throws Exception
{ {
assertThat( pageList.address( pageRef ), is( equalTo( 0L ) ) ); assertThat( pageList.getAddress( pageRef ), is( equalTo( 0L ) ) );
PageList pl = new PageList( pageList ); PageList pl = new PageList( pageList );
assertThat( pl.address( pageRef ), is( equalTo( 0L ) ) ); assertThat( pl.getAddress( pageRef ), is( equalTo( 0L ) ) );


pageList.initBuffer( pageRef ); pageList.initBuffer( pageRef );
assertThat( pageList.address( pageRef ), is( not( equalTo( 0L ) ) ) ); assertThat( pageList.getAddress( pageRef ), is( not( equalTo( 0L ) ) ) );
assertThat( pl.address( pageRef ), is( not( equalTo( 0L ) ) ) ); assertThat( pl.getAddress( pageRef ), is( not( equalTo( 0L ) ) ) );
} }


@Test @Test
Expand Down Expand Up @@ -1021,7 +1024,7 @@ public long read( long fpId, long bufferAddress, int bufferSize ) throws IOExcep
pageList.initBuffer( pageRef ); pageList.initBuffer( pageRef );
pageList.fault( pageRef, swapper, swapperId, filePageId, PageFaultEvent.NULL ); pageList.fault( pageRef, swapper, swapperId, filePageId, PageFaultEvent.NULL );


long address = pageList.address( pageRef ); long address = pageList.getAddress( pageRef );
assertThat( address, is( not( 0L ) ) ); assertThat( address, is( not( 0L ) ) );
for ( int i = 0; i < pageSize; i++ ) for ( int i = 0; i < pageSize; i++ )
{ {
Expand Down Expand Up @@ -1227,67 +1230,66 @@ public void exclusiveLockMustStillBeHeldAfterFault() throws Exception
@Test @Test
public void tryEvictMustFailIfPageIsAlreadyExclusivelyLocked() throws Exception public void tryEvictMustFailIfPageIsAlreadyExclusivelyLocked() throws Exception
{ {
doFault( 1, 42 ); // page is now loaded int swapperId = swappers.allocate( DUMMY_SWAPPER, pageSize );
doFault( swapperId, 42 ); // page is now loaded
// pages are delivered from the fault routine with the exclusive lock already held! // pages are delivered from the fault routine with the exclusive lock already held!
assertFalse( pageList.tryEvict( pageRef, DUMMY_SWAPPER ) ); assertFalse( pageList.tryEvict( pageRef ) );
} }


@Test @Test
public void tryEvictThatFailsOnExclusiveLockMustNotUndoSaidLock() throws Exception public void tryEvictThatFailsOnExclusiveLockMustNotUndoSaidLock() throws Exception
{ {
doFault( 1, 42 ); // page is now loaded int swapperId = swappers.allocate( DUMMY_SWAPPER, pageSize );
doFault( swapperId, 42 ); // page is now loaded
// pages are delivered from the fault routine with the exclusive lock already held! // pages are delivered from the fault routine with the exclusive lock already held!
pageList.tryEvict( pageRef, DUMMY_SWAPPER ); // This attempt will fail pageList.tryEvict( pageRef ); // This attempt will fail
assertTrue( pageList.isExclusivelyLocked( pageRef ) ); // page should still have its lock assertTrue( pageList.isExclusivelyLocked( pageRef ) ); // page should still have its lock
} }


@Test @Test
public void tryEvictMustFailIfPageIsNotLoaded() throws Exception public void tryEvictMustFailIfPageIsNotLoaded() throws Exception
{ {
assertFalse( pageList.tryEvict( pageRef, DUMMY_SWAPPER ) ); assertFalse( pageList.tryEvict( pageRef ) );
} }


@Test @Test
public void tryEvictMustWhenPageIsNotLoadedMustNotLeavePageLocked() throws Exception public void tryEvictMustWhenPageIsNotLoadedMustNotLeavePageLocked() throws Exception
{ {
pageList.tryEvict( pageRef, DUMMY_SWAPPER ); // This attempt fails pageList.tryEvict( pageRef ); // This attempt fails
assertFalse( pageList.isExclusivelyLocked( pageRef ) ); // Page should not be left in locked state assertFalse( pageList.isExclusivelyLocked( pageRef ) ); // Page should not be left in locked state
} }


@Test( expected = IllegalArgumentException.class )
public void tryEvictMustThrowIfSwapperIsNull() throws Exception
{
pageList.tryEvict( pageRef, null );
}

@Test @Test
public void tryEvictMustLeavePageExclusivelyLockedOnSuccess() throws Exception public void tryEvictMustLeavePageExclusivelyLockedOnSuccess() throws Exception
{ {
doFault( 1, 42 ); // page now bound & exclusively locked int swapperId = swappers.allocate( DUMMY_SWAPPER, pageSize );
doFault( swapperId, 42 ); // page now bound & exclusively locked
pageList.unlockExclusive( pageRef ); // no longer exclusively locked; can now be evicted pageList.unlockExclusive( pageRef ); // no longer exclusively locked; can now be evicted
assertTrue( pageList.tryEvict( pageRef, DUMMY_SWAPPER ) ); assertTrue( pageList.tryEvict( pageRef ) );
pageList.unlockExclusive( pageRef ); // will throw if lock is not held pageList.unlockExclusive( pageRef ); // will throw if lock is not held
} }


@Test @Test
public void pageMustNotBeLoadedAfterSuccessfulEviction() throws Exception public void pageMustNotBeLoadedAfterSuccessfulEviction() throws Exception
{ {
doFault( 1, 42 ); // page now bound & exclusively locked int swapperId = swappers.allocate( DUMMY_SWAPPER, pageSize );
doFault( swapperId, 42 ); // page now bound & exclusively locked
pageList.unlockExclusive( pageRef ); // no longer exclusively locked; can now be evicted pageList.unlockExclusive( pageRef ); // no longer exclusively locked; can now be evicted
assertTrue( pageList.isLoaded( pageRef ) ); assertTrue( pageList.isLoaded( pageRef ) );
pageList.tryEvict( pageRef, DUMMY_SWAPPER ); pageList.tryEvict( pageRef );
assertFalse( pageList.isLoaded( pageRef ) ); assertFalse( pageList.isLoaded( pageRef ) );
} }


@Test @Test
public void pageMustNotBeBoundAfterSuccessfulEviction() throws Exception public void pageMustNotBeBoundAfterSuccessfulEviction() throws Exception
{ {
doFault( 1, 42 ); // page now bound & exclusively locked int swapperId = swappers.allocate( DUMMY_SWAPPER, pageSize );
doFault( swapperId, 42 ); // page now bound & exclusively locked
pageList.unlockExclusive( pageRef ); // no longer exclusively locked; can now be evicted pageList.unlockExclusive( pageRef ); // no longer exclusively locked; can now be evicted
assertTrue( pageList.isBoundTo( pageRef, 1, 42 ) ); assertTrue( pageList.isBoundTo( pageRef, 1, 42 ) );
assertTrue( pageList.isLoaded( pageRef ) ); assertTrue( pageList.isLoaded( pageRef ) );
assertThat( pageList.getSwapperId( pageRef ), is( 1 ) ); assertThat( pageList.getSwapperId( pageRef ), is( 1 ) );
pageList.tryEvict( pageRef, DUMMY_SWAPPER ); pageList.tryEvict( pageRef );
assertFalse( pageList.isBoundTo( pageRef, 1, 42 ) ); assertFalse( pageList.isBoundTo( pageRef, 1, 42 ) );
assertFalse( pageList.isLoaded( pageRef ) ); assertFalse( pageList.isLoaded( pageRef ) );
assertThat( pageList.getSwapperId( pageRef ), is( 0 ) ); assertThat( pageList.getSwapperId( pageRef ), is( 0 ) );
Expand All @@ -1296,11 +1298,12 @@ public void pageMustNotBeBoundAfterSuccessfulEviction() throws Exception
@Test @Test
public void pageMustNotBeModifiedAfterSuccessfulEviction() throws Exception public void pageMustNotBeModifiedAfterSuccessfulEviction() throws Exception
{ {
doFault( 1, 42 ); int swapperId = swappers.allocate( DUMMY_SWAPPER, pageSize );
doFault( swapperId, 42 );
pageList.unlockExclusiveAndTakeWriteLock( pageRef ); pageList.unlockExclusiveAndTakeWriteLock( pageRef );
pageList.unlockWrite( pageRef ); // page is now modified pageList.unlockWrite( pageRef ); // page is now modified
assertTrue( pageList.isModified( pageRef ) ); assertTrue( pageList.isModified( pageRef ) );
assertTrue( pageList.tryEvict( pageRef, DUMMY_SWAPPER ) ); assertTrue( pageList.tryEvict( pageRef ) );
assertFalse( pageList.isModified( pageRef ) ); assertFalse( pageList.isModified( pageRef ) );
} }


Expand All @@ -1321,26 +1324,89 @@ public long write( long filePageId, long bufferAddress, int bufferSize ) throws
return super.write( filePageId, bufferAddress, bufferSize ); return super.write( filePageId, bufferAddress, bufferSize );
} }
}; };
doFault( 1, 42 ); int swapperId = swappers.allocate( swapper, 313 );
doFault( swapperId, 42 );
pageList.unlockExclusiveAndTakeWriteLock( pageRef ); pageList.unlockExclusiveAndTakeWriteLock( pageRef );
pageList.unlockWrite( pageRef ); // page is now modified pageList.unlockWrite( pageRef ); // page is now modified
assertTrue( pageList.isModified( pageRef ) ); assertTrue( pageList.isModified( pageRef ) );
assertTrue( pageList.tryEvict( pageRef, swapper ) ); assertTrue( pageList.tryEvict( pageRef ) );
assertThat( writtenFilePageId.get(), is( 42L ) ); assertThat( writtenFilePageId.get(), is( 42L ) );
assertThat( writtenBufferAddress.get(), is( pageList.address( pageRef ) ) ); assertThat( writtenBufferAddress.get(), is( pageList.getAddress( pageRef ) ) );
// assertThat( writtenBufferSize.get(), is( /* ... */ ) ); // todo assertThat( writtenBufferSize.get(), is( 313 ) );
}

@Test
public void tryEvictMustNotFlushPageIfNotModified() throws Exception
{
AtomicInteger writes = new AtomicInteger();
PageSwapper swapper = new DummyPageSwapper( "a" )
{
@Override
public long write( long filePageId, long bufferAddress, int bufferSize ) throws IOException
{
writes.getAndIncrement();
return super.write( filePageId, bufferAddress, bufferSize );
}
};
int swapperId = swappers.allocate( swapper, 313 );
doFault( swapperId, 42 );
pageList.unlockExclusive( pageRef ); // we take no write lock, so page is not modified
assertFalse( pageList.isModified( pageRef ) );
assertTrue( pageList.tryEvict( pageRef ) );
assertThat( writes.get(), is( 0 ) );
}

@Test
public void tryEvictMustNotifySwapperOnSuccess() throws Exception
{
AtomicBoolean evictionNotified = new AtomicBoolean();
PageSwapper swapper = new DummyPageSwapper( "a" )
{
@Override
public void evicted( long pageId, Page page )
{
evictionNotified.set( true );
assertThat( pageId, is( 42L ) );
}
};
int swapperId = swappers.allocate( swapper, 313 );
doFault( swapperId, 42 );
pageList.unlockExclusive( pageRef );
assertTrue( pageList.tryEvict( pageRef ) );
assertTrue( evictionNotified.get() );
}

@Test
public void tryEvictMustNotifySwapperOnSuccessEvenWhenFlushing() throws Exception
{
AtomicBoolean evictionNotified = new AtomicBoolean();
PageSwapper swapper = new DummyPageSwapper( "a" )
{
@Override
public void evicted( long pageId, Page page )
{
evictionNotified.set( true );
assertThat( pageId, is( 42L ) );
}
};
int swapperId = swappers.allocate( swapper, 313 );
doFault( swapperId, 42 );
pageList.unlockExclusiveAndTakeWriteLock( pageRef );
pageList.unlockWrite( pageRef ); // page is now modified
assertTrue( pageList.isModified( pageRef ) );
assertTrue( pageList.tryEvict( pageRef ) );
assertTrue( evictionNotified.get() );
assertFalse( pageList.isModified( pageRef ) );
} }
// todo try evict must flush page if modified
// todo try evict must not flush page if not modified
// todo try evict must notify swapper on success
// todo try evict must leave page unlocked if flush throws // todo try evict must leave page unlocked if flush throws
// todo try evict must leave page loaded if flush throws // todo try evict must leave page loaded if flush throws
// todo try evict must leave page bound if flush throws // todo try evict must leave page bound if flush throws
// todo try evict must leave page modified if flush throws // todo try evict must leave page modified if flush throws
// todo try evict must not notify swapper of eviction if flush throws
// todo try evict must report to eviction event // todo try evict must report to eviction event
// todo try evict that flushes must report to flush event // todo try evict that flushes must report to flush event
// todo try evict that fails must not interfere with adjacent pages
// todo try evict that succeeds must not interfere with adjacent pages // todo try evict that succeeds must not interfere with adjacent pages
// todo try evict that fails must not interfere with adjacent pages


// todo evict // todo evict
// todo flush // todo flush
Expand Down

0 comments on commit be415f1

Please sign in to comment.