Skip to content

Commit

Permalink
Make MuninnPage extend OptiLock and make things *compile*
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisvest committed Jan 20, 2016
1 parent b2060b4 commit fe4bb8f
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 67 deletions.
@@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2002-2015 "Neo Technology," * Copyright (c) 2002-2016 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com] * Network Engine for Objects in Lund AB [http://neotechnology.com]
* *
* This file is part of Neo4j. * This file is part of Neo4j.
Expand Down
Expand Up @@ -21,7 +21,6 @@


import java.io.IOException; import java.io.IOException;


import org.neo4j.concurrent.jsr166e.StampedLock;
import org.neo4j.io.pagecache.Page; 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;
Expand All @@ -34,7 +33,7 @@


import static java.lang.String.format; import static java.lang.String.format;


final class MuninnPage extends StampedLock implements Page final class MuninnPage extends OptiLock implements Page
{ {
private static final long usageStampOffset = UnsafeUtil.getFieldOffset( MuninnPage.class, "usageStamp" ); private static final long usageStampOffset = UnsafeUtil.getFieldOffset( MuninnPage.class, "usageStamp" );


Expand Down Expand Up @@ -144,7 +143,6 @@ private void doFlush(
long filePageId, long filePageId,
FlushEventOpportunity flushOpportunity ) throws IOException FlushEventOpportunity flushOpportunity ) throws IOException
{ {
assert isReadLocked() || isWriteLocked() : "doFlush requires lock";
FlushEvent event = flushOpportunity.beginFlush( filePageId, getCachePageId(), swapper ); FlushEvent event = flushOpportunity.beginFlush( filePageId, getCachePageId(), swapper );
try try
{ {
Expand All @@ -168,7 +166,6 @@ public void fault(
long filePageId, long filePageId,
PageFaultEvent faultEvent ) throws IOException PageFaultEvent faultEvent ) throws IOException
{ {
assert isWriteLocked(): "Cannot fault page without write-lock";
if ( this.swapper != null || this.filePageId != PageCursor.UNBOUND_PAGE_ID ) if ( this.swapper != null || this.filePageId != PageCursor.UNBOUND_PAGE_ID )
{ {
String msg = format( String msg = format(
Expand Down Expand Up @@ -198,7 +195,6 @@ public void fault(
*/ */
public void evict( EvictionEvent evictionEvent ) throws IOException public void evict( EvictionEvent evictionEvent ) throws IOException
{ {
assert isWriteLocked(): "Cannot evict page without write-lock";
long filePageId = this.filePageId; long filePageId = this.filePageId;
evictionEvent.setCachePageId( getCachePageId() ); evictionEvent.setCachePageId( getCachePageId() );
evictionEvent.setFilePageId( filePageId ); evictionEvent.setFilePageId( filePageId );
Expand Down Expand Up @@ -232,7 +228,6 @@ public boolean isBoundTo( PageSwapper swapper, long filePageId )
*/ */
public void initBuffer() public void initBuffer()
{ {
assert isWriteLocked(): "Cannot initBuffer without write-lock";
if ( pointer == 0 ) if ( pointer == 0 )
{ {
pointer = memoryManager.allocateAligned( size() ); pointer = memoryManager.allocateAligned( size() );
Expand All @@ -249,6 +244,6 @@ public String toString()
{ {
return format( "MuninnPage@%x[%s -> %x, filePageId = %s%s, swapper = %s]%s", return format( "MuninnPage@%x[%s -> %x, filePageId = %s%s, swapper = %s]%s",
hashCode(), getCachePageId(), pointer, filePageId, (isDirty() ? ", dirty" : ""), hashCode(), getCachePageId(), pointer, filePageId, (isDirty() ? ", dirty" : ""),
swapper, getLockStateString() ); swapper, super.toString() );
} }
} }
Expand Up @@ -663,8 +663,7 @@ private MuninnPage cooperativelyEvict( PageFaultEvent faultEvent ) throws IOExce


if ( page.isLoaded() && page.decrementUsage() ) if ( page.isLoaded() && page.decrementUsage() )
{ {
long stamp = page.tryWriteLock(); if ( page.tryExclusiveLock() )
if ( stamp != 0 )
{ {
// We got the write lock. Time to evict the page! // We got the write lock. Time to evict the page!
try ( EvictionEvent evictionEvent = faultEvent.beginEviction() ) try ( EvictionEvent evictionEvent = faultEvent.beginEviction() )
Expand All @@ -673,7 +672,7 @@ private MuninnPage cooperativelyEvict( PageFaultEvent faultEvent ) throws IOExce
} }
finally finally
{ {
page.unlockWrite( stamp ); page.unlockExclusive();
} }
} }
} }
Expand Down Expand Up @@ -790,8 +789,7 @@ int evictPages( int pageCountToEvict, int clockArm, EvictionRunEvent evictionRun


if ( page.isLoaded() && page.decrementUsage() ) if ( page.isLoaded() && page.decrementUsage() )
{ {
long stamp = page.tryWriteLock(); if ( page.tryExclusiveLock() )
if ( stamp != 0 )
{ {
// We got the lock. // We got the lock.
// Assume that the eviction is going to succeed, so that we // Assume that the eviction is going to succeed, so that we
Expand All @@ -812,7 +810,7 @@ int evictPages( int pageCountToEvict, int clockArm, EvictionRunEvent evictionRun
} }
finally finally
{ {
page.unlockWrite( stamp ); page.unlockExclusive();
} }


if ( pageEvicted ) if ( pageEvicted )
Expand Down Expand Up @@ -951,14 +949,13 @@ private long flushAtIORatio( double ratio )


// Skip the page if it is already write locked, or not dirty, or too popular. // Skip the page if it is already write locked, or not dirty, or too popular.
boolean thisPageIsDirty = false; boolean thisPageIsDirty = false;
if ( page.isWriteLocked() || !(thisPageIsDirty = page.isDirty()) || !page.decrementUsage() ) if ( !(thisPageIsDirty = page.isDirty()) || !page.decrementUsage() )
{ {
seenDirtyPages |= thisPageIsDirty; seenDirtyPages |= thisPageIsDirty;
continue; // Continue looping to the next page. continue; // Continue looping to the next page.
} }


long stamp = page.tryReadLock(); if ( page.tryExclusiveLock() ) // TODO somehow avoid taking these exclusive locks, that we currently need to avoid racing with other flushes
if ( stamp != 0 )
{ {
try try
{ {
Expand All @@ -984,7 +981,7 @@ private long flushAtIORatio( double ratio )
} }
finally finally
{ {
page.unlockRead( stamp ); page.unlockExclusive();
} }
} }


Expand Down
Expand Up @@ -47,7 +47,6 @@ abstract class MuninnPageCursor implements PageCursor
protected int pf_flags; protected int pf_flags;
protected long currentPageId; protected long currentPageId;
protected long nextPageId; protected long nextPageId;
protected long lockStamp;
private long pointer; private long pointer;
private int size; private int size;


Expand Down Expand Up @@ -115,7 +114,6 @@ public final void close()
protected void clearPageState() protected void clearPageState()
{ {
size = 0; // make all future bound checks fail size = 0; // make all future bound checks fail
lockStamp = 0; // make sure not to accidentally keep a lock state around
page = null; // make all future page navigation fail page = null; // make all future page navigation fail
} }


Expand Down Expand Up @@ -236,7 +234,6 @@ private MuninnPage pageFault(
// we must make sure to release that write lock as well. // we must make sure to release that write lock as well.
PageFaultEvent faultEvent = pinEvent.beginPageFault(); PageFaultEvent faultEvent = pinEvent.beginPageFault();
MuninnPage page; MuninnPage page;
long stamp;
try try
{ {
// The grabFreePage method might throw. // The grabFreePage method might throw.
Expand All @@ -247,7 +244,10 @@ private MuninnPage pageFault(
// their translation tables might race with eviction) and try to pin it. // their translation tables might race with eviction) and try to pin it.
// However, they will all fail because when they try to pin, the page will either be 1) free, 2) bound to // However, they will all fail because when they try to pin, the page will either be 1) free, 2) bound to
// our file, or 3) the page is write locked. // our file, or 3) the page is write locked.
stamp = page.writeLock(); if ( !page.tryExclusiveLock() )
{
throw new AssertionError( "Unable to take exclusive lock on free page" );
}
} }
catch ( Throwable throwable ) catch ( Throwable throwable )
{ {
Expand All @@ -273,15 +273,15 @@ private MuninnPage pageFault(
catch ( Throwable throwable ) catch ( Throwable throwable )
{ {
// Make sure to unlock the page, so the eviction thread can pick up our trash. // Make sure to unlock the page, so the eviction thread can pick up our trash.
page.unlockWrite( stamp ); page.unlockExclusive();
// Make sure to unstuck the page fault latch. // Make sure to unstuck the page fault latch.
UnsafeUtil.putObjectVolatile( chunk, chunkOffset, null ); UnsafeUtil.putObjectVolatile( chunk, chunkOffset, null );
latch.release(); latch.release();
faultEvent.done( throwable ); faultEvent.done( throwable );
pinEvent.done(); pinEvent.done();
throw throwable; throw throwable;
} }
convertPageFaultLock( page, stamp ); convertPageFaultLock( page );
UnsafeUtil.putObjectVolatile( chunk, chunkOffset, page ); UnsafeUtil.putObjectVolatile( chunk, chunkOffset, page );
latch.release(); latch.release();
faultEvent.done(); faultEvent.done();
Expand All @@ -295,7 +295,7 @@ protected long assertPagedFileStillMappedAndGetIdOfLastPage()


protected abstract void unpinCurrentPage(); protected abstract void unpinCurrentPage();


protected abstract void convertPageFaultLock( MuninnPage page, long stamp ); protected abstract void convertPageFaultLock( MuninnPage page );


protected abstract void pinCursorToPage( MuninnPage page, long filePageId, PageSwapper swapper ); protected abstract void pinCursorToPage( MuninnPage page, long filePageId, PageSwapper swapper );


Expand Down
Expand Up @@ -193,7 +193,6 @@ public void flushAndForce() throws IOException
void flushAndForceInternal( FlushEventOpportunity flushOpportunity ) throws IOException void flushAndForceInternal( FlushEventOpportunity flushOpportunity ) throws IOException
{ {
pageCache.pauseBackgroundFlushTask(); pageCache.pauseBackgroundFlushTask();
long[] stamps = new long[translationTableChunkSize];
MuninnPage[] pages = new MuninnPage[translationTableChunkSize]; MuninnPage[] pages = new MuninnPage[translationTableChunkSize];
long filePageId = -1; // Start at -1 because we increment at the *start* of the chunk-loop iteration. long filePageId = -1; // Start at -1 because we increment at the *start* of the chunk-loop iteration.
try try
Expand All @@ -210,7 +209,7 @@ void flushAndForceInternal( FlushEventOpportunity flushOpportunity ) throws IOEx
if ( element instanceof MuninnPage ) if ( element instanceof MuninnPage )
{ {
MuninnPage page = (MuninnPage) element; MuninnPage page = (MuninnPage) element;
stamps[pagesGrabbed] = page.readLock(); page.writeLock();
if ( page.isBoundTo( swapper, filePageId ) && page.isDirty() ) if ( page.isBoundTo( swapper, filePageId ) && page.isDirty() )
{ {
// The page is still bound to the expected file and file page id after we locked it, // The page is still bound to the expected file and file page id after we locked it,
Expand All @@ -222,17 +221,17 @@ void flushAndForceInternal( FlushEventOpportunity flushOpportunity ) throws IOEx
} }
else else
{ {
page.unlockRead( stamps[pagesGrabbed] ); page.unlockWrite();
} }
} }
if ( pagesGrabbed > 0 ) if ( pagesGrabbed > 0 )
{ {
pagesGrabbed = vectoredFlush( stamps, pages, pagesGrabbed, flushOpportunity ); pagesGrabbed = vectoredFlush( pages, pagesGrabbed, flushOpportunity );
} }
} }
if ( pagesGrabbed > 0 ) if ( pagesGrabbed > 0 )
{ {
vectoredFlush( stamps, pages, pagesGrabbed, flushOpportunity ); vectoredFlush( pages, pagesGrabbed, flushOpportunity );
} }
} }


Expand All @@ -244,8 +243,7 @@ void flushAndForceInternal( FlushEventOpportunity flushOpportunity ) throws IOEx
} }
} }


private int vectoredFlush( private int vectoredFlush( MuninnPage[] pages, int pagesGrabbed, FlushEventOpportunity flushOpportunity )
long[] stamps, MuninnPage[] pages, int pagesGrabbed, FlushEventOpportunity flushOpportunity )
throws IOException throws IOException
{ {
FlushEvent flush = null; FlushEvent flush = null;
Expand Down Expand Up @@ -284,7 +282,7 @@ private int vectoredFlush(
// Always unlock all the pages in the vector // Always unlock all the pages in the vector
for ( int j = 0; j < pagesGrabbed; j++ ) for ( int j = 0; j < pagesGrabbed; j++ )
{ {
pages[j].unlockRead( stamps[j] ); pages[j].unlockWrite();
} }
} }
} }
Expand Down
Expand Up @@ -25,7 +25,7 @@


final class MuninnReadPageCursor extends MuninnPageCursor final class MuninnReadPageCursor extends MuninnPageCursor
{ {
private boolean optimisticLock; protected long lockStamp;


@Override @Override
protected void unpinCurrentPage() protected void unpinCurrentPage()
Expand All @@ -35,14 +35,8 @@ protected void unpinCurrentPage()
if ( p != null ) if ( p != null )
{ {
pinEvent.done(); pinEvent.done();
assert optimisticLock || p.isReadLocked() :
"pinned page wasn't really locked; not even optimistically: " + p;

if ( !optimisticLock )
{
p.unlockRead( lockStamp );
}
} }
lockStamp = 0; // make sure not to accidentally keep a lock state around
clearPageState(); clearPageState();
} }


Expand All @@ -64,8 +58,7 @@ public boolean next() throws IOException
@Override @Override
protected void lockPage( MuninnPage page ) protected void lockPage( MuninnPage page )
{ {
lockStamp = page.tryOptimisticRead(); lockStamp = page.tryOptimisticReadLock();
optimisticLock = true;
} }


@Override @Override
Expand All @@ -81,43 +74,37 @@ protected void pinCursorToPage( MuninnPage page, long filePageId, PageSwapper sw
} }


@Override @Override
protected void convertPageFaultLock( MuninnPage page, long stamp ) protected void convertPageFaultLock( MuninnPage page )
{ {
stamp = page.tryConvertToReadLock( stamp ); lockStamp = page.unlockExclusive();
assert stamp != 0: "Converting a write lock to a read lock should always succeed";
lockStamp = stamp;
optimisticLock = false; // We're using a pessimistic read lock
} }


@Override @Override
public boolean shouldRetry() throws IOException public boolean shouldRetry() throws IOException
{ {
boolean needsRetry = optimisticLock && !page.validate( lockStamp ); boolean needsRetry = !page.validateReadLock( lockStamp );
if ( needsRetry ) if ( needsRetry )
{ {
setOffset( 0 ); setOffset( 0 );
optimisticLock = false; lockStamp = page.tryOptimisticReadLock();
lockStamp = page.readLock(); // The page might have been evicted while we held the optimistic
// We have a pessimistic read lock on the page now. This prevents
// writes to the page, and it prevents the page from being evicted.
// However, it might have been evicted while we held the optimistic
// read lock, so we need to check with page.pin that this is still // read lock, so we need to check with page.pin that this is still
// the page we're actually interested in: // the page we're actually interested in:
if ( !page.isBoundTo( pagedFile.swapper, currentPageId ) ) if ( !page.isBoundTo( pagedFile.swapper, currentPageId ) )
{ {
// This is no longer the page we're interested in, so we have // This is no longer the page we're interested in, so we have
// to release our lock and redo the pinning. // to redo the pinning.
// This might in turn lead to a new optimistic lock on a // This might in turn lead to a new optimistic lock on a
// different page if someone else has taken the page fault for // different page if someone else has taken the page fault for
// us. If nobody has done that, we'll take the page fault // us. If nobody has done that, we'll take the page fault
// ourselves, and in that case we'll end up with first a write // ourselves, and in that case we'll end up with first an exclusive
// lock during the faulting, and then a read lock once the // lock during the faulting, and then an optimistic read lock once the
// fault itself is over. // fault itself is over.
page.unlockRead( lockStamp ); // First, forget about this page in case pin() throws and the cursor
// Forget about this page in case pin() throws and the cursor
// is closed; we don't want unpinCurrentPage() to try unlocking // is closed; we don't want unpinCurrentPage() to try unlocking
// this page. // this page.
page = null; page = null;
// Then try pin again.
pin( currentPageId, false ); pin( currentPageId, false );
} }
} }
Expand Down
Expand Up @@ -32,7 +32,6 @@ protected void unpinCurrentPage()
if ( page != null ) if ( page != null )
{ {
pinEvent.done(); pinEvent.done();
assert page.isWriteLocked(): "page pinned for writing was not write locked: " + page;
unlockPage( page ); unlockPage( page );
} }
clearPageState(); clearPageState();
Expand Down Expand Up @@ -63,13 +62,13 @@ public boolean next() throws IOException
@Override @Override
protected void lockPage( MuninnPage page ) protected void lockPage( MuninnPage page )
{ {
lockStamp = page.writeLock(); page.writeLock();
} }


@Override @Override
protected void unlockPage( MuninnPage page ) protected void unlockPage( MuninnPage page )
{ {
page.unlockWrite( lockStamp ); page.unlockWrite();
} }


@Override @Override
Expand All @@ -87,9 +86,10 @@ protected void pinCursorToPage( MuninnPage page, long filePageId, PageSwapper sw
} }


@Override @Override
protected void convertPageFaultLock( MuninnPage page, long stamp ) protected void convertPageFaultLock( MuninnPage page )
{ {
lockStamp = stamp; page.unlockExclusive(); // TODO page.unlockExclusiveAndTakeWriteLock
page.writeLock();
} }


@Override @Override
Expand Down
@@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2002-2015 "Neo Technology," * Copyright (c) 2002-2016 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com] * Network Engine for Objects in Lund AB [http://neotechnology.com]
* *
* This file is part of Neo4j. * This file is part of Neo4j.
Expand Down Expand Up @@ -222,6 +222,11 @@ public long unlockExclusive()
return n; return n;
} }


public void unlockExclusiveAndTakeWriteLock()
{
// TODO
}

private void throwUnmatchedUnlockExclusive( long s ) private void throwUnmatchedUnlockExclusive( long s )
{ {
throw new IllegalMonitorStateException( "Unmatched unlockExclusive: " + describeState( s ) ); throw new IllegalMonitorStateException( "Unmatched unlockExclusive: " + describeState( s ) );
Expand Down
@@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2002-2015 "Neo Technology," * Copyright (c) 2002-2016 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com] * Network Engine for Objects in Lund AB [http://neotechnology.com]
* *
* This file is part of Neo4j. * This file is part of Neo4j.
Expand Down

0 comments on commit fe4bb8f

Please sign in to comment.