From ac6b4bd1a167161cfe3367e6e9612ac42160ef3e Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Sun, 1 Jan 2017 22:54:57 +0100 Subject: [PATCH] Simplify the SwapperSet interface --- .../io/pagecache/impl/muninn/SwapperSet.java | 33 ++++------ .../pagecache/impl/muninn/SwapperSetTest.java | 63 ++++--------------- 2 files changed, 25 insertions(+), 71 deletions(-) diff --git a/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/SwapperSet.java b/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/SwapperSet.java index d33cd4424597d..91081e00da02f 100644 --- a/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/SwapperSet.java +++ b/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/SwapperSet.java @@ -19,7 +19,6 @@ */ package org.neo4j.io.pagecache.impl.muninn; -import java.io.IOException; import java.util.Arrays; import java.util.function.IntConsumer; @@ -36,7 +35,7 @@ final class SwapperSet private final PrimitiveIntSet free = Primitive.intSet(); private final Object vacuumLock = new Object(); - private static final class Allocation + public static final class Allocation { public final int id; public final int filePageSize; @@ -50,28 +49,15 @@ private Allocation( int id, int filePageSize, PageSwapper swapper ) } } - public interface ApplyCall - { - void apply( PageSwapper swapper, int filePageSize ) throws IOException; - } - - public PageSwapper getSwapper( int id ) - { - checkId( id ); - return allocations[id].swapper; - } - - public int getFilePageSize( int id ) - { - checkId( id ); - return allocations[id].filePageSize; - } - - public void apply( int id, ApplyCall call ) throws IOException + public Allocation getAllocation( int id ) { checkId( id ); Allocation allocation = allocations[id]; - call.apply( allocation.swapper, allocation.filePageSize ); + if ( allocation == null ) + { + throw noSuchId( id ); + } + return allocation; } private void checkId( int id ) @@ -82,6 +68,11 @@ private void checkId( int id ) } } + private NullPointerException noSuchId( int id ) + { + return new NullPointerException( "Swapper allocation by id " + id + " does not exist; it might have been freed" ); + } + public synchronized int allocate( PageSwapper swapper, int filePageSize ) { Allocation[] allocations = this.allocations; diff --git a/community/io/src/test/java/org/neo4j/io/pagecache/impl/muninn/SwapperSetTest.java b/community/io/src/test/java/org/neo4j/io/pagecache/impl/muninn/SwapperSetTest.java index 76fead968356d..6bdd1ddffcea3 100644 --- a/community/io/src/test/java/org/neo4j/io/pagecache/impl/muninn/SwapperSetTest.java +++ b/community/io/src/test/java/org/neo4j/io/pagecache/impl/muninn/SwapperSetTest.java @@ -25,9 +25,6 @@ import org.junit.Test; import org.junit.rules.ExpectedException; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; - import org.neo4j.collection.primitive.Primitive; import org.neo4j.collection.primitive.PrimitiveIntSet; import org.neo4j.io.pagecache.PageSwapper; @@ -36,7 +33,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.sameInstance; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; @@ -54,34 +50,27 @@ public void setUp() } @Test - public void mustReturnAllocatedSwapperAndFilePageSize() throws Exception + public void mustReturnAllocationWithSwapperAndFilePageSize() throws Exception { DummyPageSwapper a = new DummyPageSwapper( "a" ); DummyPageSwapper b = new DummyPageSwapper( "b" ); int idA = set.allocate( a, 42 ); int idB = set.allocate( b, 43 ); - assertThat( set.getSwapper( idA ), is( a ) ); - assertThat( set.getFilePageSize( idA ), is( 42 ) ); - assertThat( set.getSwapper( idB ), is( b ) ); - assertThat( set.getFilePageSize( idB ), is( 43 ) ); - } - - @Test - public void accessingPageSwapperOfFreedAllocationMustThrow() throws Exception - { - int id = set.allocate( new DummyPageSwapper( "a" ), 42 ); - set.free( id ); - exception.expect( NullPointerException.class ); - set.getSwapper( id ); + SwapperSet.Allocation allocA = set.getAllocation( idA ); + SwapperSet.Allocation allocB = set.getAllocation( idB ); + assertThat( allocA.swapper, is( a ) ); + assertThat( allocA.filePageSize, is( 42 ) ); + assertThat( allocB.swapper, is( b ) ); + assertThat( allocB.filePageSize, is( 43 ) ); } @Test - public void accessingFilePageSizeOfFreedAllocationMustThrow() throws Exception + public void accessingFreedAllocationMustThrow() throws Exception { int id = set.allocate( new DummyPageSwapper( "a" ), 42 ); set.free( id ); exception.expect( NullPointerException.class ); - set.getSwapper( id ); + set.getAllocation( id ); } @Test @@ -176,42 +165,16 @@ public void mustNotUseZeroAsSwapperId() throws Exception } @Test - public void freeOfIdZeroMustThrow() throws Exception + public void gettingAllocationZeroMustThrow() throws Exception { exception.expect( IllegalArgumentException.class ); - set.free( 0 ); + set.getAllocation( 0 ); } @Test - public void gettingSwapperOfIdZeroMustThrow() throws Exception - { - exception.expect( IllegalArgumentException.class ); - set.getSwapper( 0 ); - } - - @Test - public void gettingFilePageSizeOfIdZeroMustThrow() throws Exception - { - exception.expect( IllegalArgumentException.class ); - set.getFilePageSize( 0 ); - } - - @Test - public void applyMustThrowForIdZero() throws Exception + public void freeOfIdZeroMustThrow() throws Exception { exception.expect( IllegalArgumentException.class ); - set.apply( 0, (swapper,size) -> fail( "should not have been called" ) ); - } - - @Test - public void applyMustReceiveArguments() throws Exception - { - AtomicReference gotSwapper = new AtomicReference<>(); - AtomicInteger gotFilePageSize = new AtomicInteger(); - PageSwapper swapper = new DummyPageSwapper( "a" ); - int id = set.allocate( swapper, 3113 ); - set.apply( id, (s,size) -> { gotSwapper.set( s ); gotFilePageSize.set( size ); } ); - assertThat( gotSwapper.get(), is( sameInstance( swapper ) ) ); - assertThat( gotFilePageSize.get(), is( 3113 ) ); + set.free( 0 ); } }