Skip to content

Commit

Permalink
Page cache cursor unclaimed check made explicit
Browse files Browse the repository at this point in the history
Previously this check was an assertion and thus was not executed in many cases.
This commit makes it explicit so we would catch more incorrect usages or page
cache cursors at runtime.
Check is guarded by a static final boolean flag obtained from system property.
So if `ThreadLocal#get()` appears to be very heavy, this additional check could
be turned off.
  • Loading branch information
lutovich committed Sep 28, 2015
1 parent f1aef32 commit a3cbbce
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 9 deletions.
Expand Up @@ -23,6 +23,8 @@ final class CursorPool
{ {
private static boolean disableCursorPooling = Boolean.getBoolean( private static boolean disableCursorPooling = Boolean.getBoolean(
"org.neo4j.io.pagecache.impl.muninn.CursorPool.disableCursorPooling" ); "org.neo4j.io.pagecache.impl.muninn.CursorPool.disableCursorPooling" );
private static boolean disableClaimedCheck = Boolean.getBoolean(
"org.neo4j.io.pagecache.impl.muninn.CursorPool.disableClaimedCheck" );


private final ThreadLocal<MuninnReadPageCursor> readCursorCache = new MuninnReadPageCursorThreadLocal(); private final ThreadLocal<MuninnReadPageCursor> readCursorCache = new MuninnReadPageCursorThreadLocal();
private final ThreadLocal<MuninnWritePageCursor> writeCursorCache = new MuninnWritePageCursorThreadLocal(); private final ThreadLocal<MuninnWritePageCursor> writeCursorCache = new MuninnWritePageCursorThreadLocal();
Expand All @@ -36,7 +38,7 @@ public MuninnReadPageCursor takeReadCursor()


MuninnReadPageCursor cursor = readCursorCache.get(); MuninnReadPageCursor cursor = readCursorCache.get();


assert unclaimed( cursor, writeCursorCache ); assertUnclaimed( cursor, writeCursorCache );


cursor.markAsClaimed(); cursor.markAsClaimed();
return cursor; return cursor;
Expand All @@ -51,20 +53,19 @@ public MuninnWritePageCursor takeWriteCursor()


MuninnWritePageCursor cursor = writeCursorCache.get(); MuninnWritePageCursor cursor = writeCursorCache.get();


assert unclaimed( cursor, readCursorCache ); assertUnclaimed( cursor, readCursorCache );


cursor.markAsClaimed(); cursor.markAsClaimed();
return cursor; return cursor;
} }


private static boolean unclaimed( MuninnPageCursor first, ThreadLocal<? extends MuninnPageCursor> second ) private static void assertUnclaimed( MuninnPageCursor first, ThreadLocal<? extends MuninnPageCursor> second )
{ {
// This has been pulled out into a static method and guarded behind if ( !disableClaimedCheck )
// `assert`s to reduce the overhead put upon the inlining budget {
// by something that should never happen. first.assertUnclaimed();
first.assertUnclaimed(); second.get().assertUnclaimed();
second.get().assertUnclaimed(); }
return true;
} }


private static class MuninnReadPageCursorThreadLocal extends ThreadLocal<MuninnReadPageCursor> private static class MuninnReadPageCursorThreadLocal extends ThreadLocal<MuninnReadPageCursor>
Expand Down
Expand Up @@ -62,12 +62,14 @@ public boolean next() throws IOException
return true; return true;
} }


@Override
protected void lockPage( MuninnPage page ) protected void lockPage( MuninnPage page )
{ {
lockStamp = page.tryOptimisticRead(); lockStamp = page.tryOptimisticRead();
optimisticLock = true; optimisticLock = true;
} }


@Override
protected void unlockPage( MuninnPage page ) protected void unlockPage( MuninnPage page )
{ {
} }
Expand Down
Expand Up @@ -61,11 +61,13 @@ public boolean next() throws IOException
return true; return true;
} }


@Override
protected void lockPage( MuninnPage page ) protected void lockPage( MuninnPage page )
{ {
lockStamp = page.writeLock(); lockStamp = page.writeLock();
} }


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

0 comments on commit a3cbbce

Please sign in to comment.