Skip to content

Commit

Permalink
More work on eviction via PageList
Browse files Browse the repository at this point in the history
  • Loading branch information
@chrisvest
chrisvest committed May 26, 2017
1 parent d7189e7 commit f61ca22
Show file tree
Hide file tree
Showing 3 changed files with 217 additions and 15 deletions.
35 changes: 27 additions & 8 deletions community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/OffHeapPageLock.java
View file
Expand Up @@ -122,17 +122,17 @@ private static void unconditionallySetState( long address, long update )
* Start an optimistic critical section, and return a stamp that can be used to validate if the read lock was
* consistent. That is, if no write or exclusive lock was overlapping with the optimistic read lock.
*
* @return A stamp that must be passed to {@link #validateReadLock(long,long)} to validate the critical section.
* @return A stamp that must be passed to {@link #validateReadLock(long, long)} to validate the critical section.
*/
public static long tryOptimisticReadLock( long address )
{
return getState( address ) & SEQ_MASK;
}

/**
* Validate a stamp from {@link #tryOptimisticReadLock(long)} or {@link #unlockExclusive(long)}, and return {@code true}
* if no write or exclusive lock overlapped with the critical section of the optimistic read lock represented by
* the stamp.
* Validate a stamp from {@link #tryOptimisticReadLock(long)} or {@link #unlockExclusive(long)}, and return
* {@code true} if no write or exclusive lock overlapped with the critical section of the optimistic read lock
* represented by the stamp.
*
* @param stamp The stamp of the optimistic read lock.
* @return {@code true} if the optimistic read lock was valid, {@code false} otherwise.
Expand Down Expand Up @@ -165,7 +165,7 @@ public static boolean isExclusivelyLocked( long address )
public static boolean tryWriteLock( long address )
{
long s, n;
for (; ; )
for ( ; ; )
{
s = getState( address );
boolean unwritablyLocked = (s & EXL_MASK) != 0;
Expand Down Expand Up @@ -267,7 +267,7 @@ public static long unlockExclusive( long address )
public static void unlockExclusiveAndTakeWriteLock( long address )
{
long s = initiateExclusiveLockRelease( address );
long n = nextSeq( s ) - EXL_MASK + CNT_UNIT;
long n = (nextSeq( s ) - EXL_MASK + CNT_UNIT) | MOD_MASK;
unconditionallySetState( address, n );
}

Expand All @@ -286,17 +286,36 @@ private static void throwUnmatchedUnlockExclusive( long s )
throw new IllegalMonitorStateException( "Unmatched unlockExclusive: " + describeState( s ) );
}

/**
* If the given lock is exclusively held, then the <em>modified</em> flag will be explicitly lowered (marked as
* unmodified) if the <em>modified</em> is currently raised.
* <p>
* If the <em>modified</em> flag is currently not raised, then this method does nothing.
*
* @throws IllegalStateException if the lock at the given address is not in the exclusively locked state.
*/
public static void explicitlyMarkPageUnmodifiedUnderExclusiveLock( long address )
{
long s = getState( address );
if ( (s & EXL_MASK) != EXL_MASK )
{
throw new IllegalStateException( "Page must be exclusively locked to explicitly lower modified bit" );
}
s = s & (~MOD_MASK);
unconditionallySetState( address, s );
}

/**
* Grab the flush lock if it is immediately available. Flush locks prevent overlapping exclusive locks,
* but do not invalidate optimistic read locks, nor do they prevent overlapping write locks. Only one flush lock
* can be held at a time. If any flush or exclusive lock is already held, the attempt to take the flush lock will
* fail.
* <p>
* Successfully grabbed flush locks must always be paired with a corresponding
* {@link #unlockFlush(long,long,boolean)}.
* {@link #unlockFlush(long, long, boolean)}.
*
* @return If the lock is successfully grabbed, the method will return a stamp value that must be passed to the
* {@link #unlockFlush(long,long,boolean)}, and which is used for detecting any overlapping write locks. If the
* {@link #unlockFlush(long, long, boolean)}, and which is used for detecting any overlapping write locks. If the
* flush lock could not be taken, {@code 0} will be returned.
*/
public static long tryFlushLock( long address )
Expand Down
40 changes: 38 additions & 2 deletions community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/PageList.java
View file
Expand Up @@ -197,6 +197,11 @@ public void unlockFlush( long pageRef, long stamp, boolean success )
OffHeapPageLock.unlockFlush( offLock( pageRef ), stamp, success );
}

public void explicitlyMarkPageUnmodifiedUnderExclusiveLock( long pageRef )
{
OffHeapPageLock.explicitlyMarkPageUnmodifiedUnderExclusiveLock( offLock( pageRef ) );
}

public int getCachePageSize()
{
return cachePageSize;
Expand Down Expand Up @@ -290,7 +295,7 @@ public void fault( long pageRef, PageSwapper swapper, int swapperId, long filePa
{
if ( swapper == null )
{
throw new IllegalArgumentException( "swapper cannot be null" );
throw swapperCannotBeNull();
}
int currentSwapper = getSwapperId( pageRef );
long currentFilePageId = getFilePageId( pageRef );
Expand All @@ -313,7 +318,12 @@ public void fault( long pageRef, PageSwapper swapper, int swapperId, long filePa
setSwapperId( pageRef, swapperId ); // Page now considered isBoundTo( swapper, filePageId )
}

private IllegalStateException cannotFaultException( long pageRef, PageSwapper swapper, int swapperId,
private static IllegalArgumentException swapperCannotBeNull()
{
return new IllegalArgumentException( "swapper cannot be null" );
}

private static IllegalStateException cannotFaultException( long pageRef, PageSwapper swapper, int swapperId,
long filePageId, int currentSwapper, long currentFilePageId )
{
String msg = format(
Expand All @@ -326,6 +336,32 @@ private IllegalStateException cannotFaultException( long pageRef, PageSwapper sw

public boolean tryEvict( long pageRef, PageSwapper swapper )
{
if ( swapper == null )
{
throw swapperCannotBeNull();
}
if ( tryExclusiveLock( pageRef ) )
{
if ( isLoaded( pageRef ) )
{
clearBinding( pageRef );
if ( isModified( pageRef ) )
{
explicitlyMarkPageUnmodifiedUnderExclusiveLock( pageRef );
}
return true;
}
else
{
unlockExclusive( pageRef );
}
}
return false;
}

private void clearBinding( long pageRef )
{
setFilePageId( pageRef, PageCursor.UNBOUND_PAGE_ID );
setSwapperId( pageRef, 0 );
}
}
157 changes: 152 additions & 5 deletions community/io/src/test/java/org/neo4j/io/pagecache/impl/muninn/PageListTest.java
View file
Expand Up @@ -37,6 +37,8 @@
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.LongFunction;

import org.neo4j.io.ByteUnit;
Expand Down Expand Up @@ -70,7 +72,8 @@ public class PageListTest
public static Iterable<Object[]> parameters()
{
LongFunction<Object[]> toArray = x -> new Object[]{x};
return () -> Arrays.stream( pageIds ).mapToObj( toArray ).iterator();
// return () -> Arrays.stream( pageIds ).mapToObj( toArray ).iterator();
return () -> Arrays.stream( new long[]{0} ).mapToObj( toArray ).iterator();
}

private static ExecutorService executor;
Expand Down Expand Up @@ -711,6 +714,17 @@ public void takingWriteLockMustRaiseModifiedFlag() throws Exception
pageList.unlockWrite( pageRef );
}

@Test
public void turningExclusiveLockIntoWriteLockMustRaiseModifiedFlag() throws Exception
{
assertFalse( pageList.isModified( pageRef ) );
assertTrue( pageList.tryExclusiveLock( pageRef ) );
assertFalse( pageList.isModified( pageRef ) );
pageList.unlockExclusiveAndTakeWriteLock( pageRef );
assertTrue( pageList.isModified( pageRef ) );
pageList.unlockWrite( pageRef );
}

@Test
public void releasingFlushLockMustLowerModifiedFlagIfSuccessful() throws Exception
{
Expand Down Expand Up @@ -804,6 +818,47 @@ public void writeLockMustNotInterfereWithAdjacentModifiedFlags() throws Exceptio
assertFalse( pageList.isModified( nextPageRef ) );
}

@Test( expected = IllegalStateException.class )
public void disallowUnlockedPageToExplicitlyLowerModifiedFlag() throws Exception
{
pageList.explicitlyMarkPageUnmodifiedUnderExclusiveLock( pageRef );
}

@Test( expected = IllegalStateException.class )
public void disallowReadLockedPageToExplicitlyLowerModifiedFlag() throws Exception
{
pageList.tryOptimisticReadLock( pageRef );
pageList.explicitlyMarkPageUnmodifiedUnderExclusiveLock( pageRef );
}

@Test( expected = IllegalStateException.class )
public void disallowFlushLockedPageToExplicitlyLowerModifiedFlag() throws Exception
{
assertThat( pageList.tryFlushLock( pageRef ), is( not( 0L ) ) );
pageList.explicitlyMarkPageUnmodifiedUnderExclusiveLock( pageRef );
}

@Test( expected = IllegalStateException.class )
public void disallowWriteLockedPageToExplicitlyLowerModifiedFlag() throws Exception
{
assertTrue( pageList.tryWriteLock( pageRef ) );
pageList.explicitlyMarkPageUnmodifiedUnderExclusiveLock( pageRef );
}

@Test
public void allowExclusiveLockedPageToExplicitlyLowerModifiedFlag() throws Exception
{
assertFalse( pageList.isModified( pageRef ) );
assertTrue( pageList.tryWriteLock( pageRef ) );
pageList.unlockWrite( pageRef );
assertTrue( pageList.isModified( pageRef ) );
assertTrue( pageList.tryExclusiveLock( pageRef ) );
assertTrue( pageList.isModified( pageRef ) );
pageList.explicitlyMarkPageUnmodifiedUnderExclusiveLock( pageRef );
assertFalse( pageList.isModified( pageRef ) );
pageList.unlockExclusive( pageRef );
}

// xxx ---[ Page state tests ]---

@Test
Expand Down Expand Up @@ -1177,19 +1232,111 @@ public void tryEvictMustFailIfPageIsAlreadyExclusivelyLocked() throws Exception
assertFalse( pageList.tryEvict( pageRef, DUMMY_SWAPPER ) );
}

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

@Test
public void tryEvictMustFailIfPageIsNotLoaded() throws Exception
{
assertFalse( pageList.tryEvict( pageRef, DUMMY_SWAPPER ) );
}
// todo try evict must leave page exclusively locked on success

@Test
public void tryEvictMustWhenPageIsNotLoadedMustNotLeavePageLocked() throws Exception
{
pageList.tryEvict( pageRef, DUMMY_SWAPPER ); // This attempt fails
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
public void tryEvictMustLeavePageExclusivelyLockedOnSuccess() throws Exception
{
doFault( 1, 42 ); // page now bound & exclusively locked
pageList.unlockExclusive( pageRef ); // no longer exclusively locked; can now be evicted
assertTrue( pageList.tryEvict( pageRef, DUMMY_SWAPPER ) );
pageList.unlockExclusive( pageRef ); // will throw if lock is not held
}

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

@Test
public void pageMustNotBeBoundAfterSuccessfulEviction() throws Exception
{
doFault( 1, 42 ); // page now bound & exclusively locked
pageList.unlockExclusive( pageRef ); // no longer exclusively locked; can now be evicted
assertTrue( pageList.isBoundTo( pageRef, 1, 42 ) );
assertTrue( pageList.isLoaded( pageRef ) );
assertThat( pageList.getSwapperId( pageRef ), is( 1 ) );
pageList.tryEvict( pageRef, DUMMY_SWAPPER );
assertFalse( pageList.isBoundTo( pageRef, 1, 42 ) );
assertFalse( pageList.isLoaded( pageRef ) );
assertThat( pageList.getSwapperId( pageRef ), is( 0 ) );
}

@Test
public void pageMustNotBeModifiedAfterSuccessfulEviction() throws Exception
{
doFault( 1, 42 );
pageList.unlockExclusiveAndTakeWriteLock( pageRef );
pageList.unlockWrite( pageRef ); // page is now modified
assertTrue( pageList.isModified( pageRef ) );
assertTrue( pageList.tryEvict( pageRef, DUMMY_SWAPPER ) );
assertFalse( pageList.isModified( pageRef ) );
}

@Test
public void tryEvictMustFlushPageIfModified() throws Exception
{
AtomicLong writtenFilePageId = new AtomicLong( -1 );
AtomicLong writtenBufferAddress = new AtomicLong( -1 );
AtomicInteger writtenBufferSize = new AtomicInteger( -1 );
PageSwapper swapper = new DummyPageSwapper( "file" )
{
@Override
public long write( long filePageId, long bufferAddress, int bufferSize ) throws IOException
{
assertTrue( writtenFilePageId.compareAndSet( -1, filePageId ) );
assertTrue( writtenBufferAddress.compareAndSet( -1, bufferAddress ) );
assertTrue( writtenBufferSize.compareAndSet( -1, bufferSize ) );
return super.write( filePageId, bufferAddress, bufferSize );
}
};
doFault( 1, 42 );
pageList.unlockExclusiveAndTakeWriteLock( pageRef );
pageList.unlockWrite( pageRef ); // page is now modified
assertTrue( pageList.isModified( pageRef ) );
assertTrue( pageList.tryEvict( pageRef, swapper ) );
assertThat( writtenFilePageId.get(), is( 42L ) );
assertThat( writtenBufferAddress.get(), is( pageList.address( pageRef ) ) );
// assertThat( writtenBufferSize.get(), is( /* ... */ ) ); // todo
}
// todo try evict must flush page if modified
// todo page must not be loaded after successful eviction
// todo page must not be bound after successful eviction
// todo page must not be modified after successful eviction
// 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 loaded 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 report to eviction event
// todo try evict that flushes must report to flush event
// todo try evict that fails must not interfere with adjacent pages
Expand Down

0 comments on commit f61ca22

Please sign in to comment.