From d84082a463bb9c3f2afecd7f81d49a97b4711668 Mon Sep 17 00:00:00 2001 From: Davide Grohmann Date: Tue, 4 Apr 2017 16:36:23 +0200 Subject: [PATCH] Simplify code --- .../kernel/impl/api/KernelTransactions.java | 35 ++++--------------- .../collection/pool/LinkedQueuePool.java | 27 ++++++-------- .../neo4j/collection/pool/MarshlandPool.java | 17 ++------- .../collection/pool/LinkedQueuePoolTest.java | 11 ++++-- .../collection/pool/MarshlandPoolTest.java | 4 +-- .../lock/forseti/ForsetiLockManager.java | 4 +-- 6 files changed, 32 insertions(+), 66 deletions(-) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactions.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactions.java index 8da15226542e8..5849527d92922 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactions.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactions.java @@ -96,10 +96,9 @@ public class KernelTransactions extends LifecycleAdapter implements Supplier allTransactions = newSetFromMap( new ConcurrentHashMap<>() ); - // This is the factory that actually builds brand-new instances. - private final Factory factory = new KernelTransactionImplementationFactory( allTransactions ); // Global pool of transactions, wrapped by the thread-local marshland pool and so is not used directly. - private final LinkedQueuePool globalTxPool = new GlobalKernelTransactionPool( allTransactions, factory ); + private final LinkedQueuePool globalTxPool = + new GlobalKernelTransactionPool( allTransactions ); // Pool of unused transactions. private final MarshlandPool localTxPool = new MarshlandPool<>( globalTxPool ); @@ -288,17 +287,6 @@ KernelTransactionHandle createHandle( KernelTransactionImplementation tx ) return new KernelTransactionImplementationHandle( tx ); } - /** - * Get all transactions - * *

- * Note: this method is package-private for testing only. - * @return set of all kernel transaction - */ - Set getAllTransactions() - { - return allTransactions; - } - private void assertRunning() { if ( availabilityGuard.isShutdown() ) @@ -320,17 +308,18 @@ private void assertCurrentThreadIsNotBlockingNewTransactions() } } - private class KernelTransactionImplementationFactory implements Factory + private class GlobalKernelTransactionPool extends LinkedQueuePool { private Set transactions; - KernelTransactionImplementationFactory( Set transactions ) + GlobalKernelTransactionPool( Set transactions ) { + super( 8 ); this.transactions = transactions; } @Override - public KernelTransactionImplementation newInstance() + protected KernelTransactionImplementation create() { KernelTransactionImplementation tx = new KernelTransactionImplementation( statementOperations, schemaWriteGuard, hooks, @@ -341,18 +330,6 @@ public KernelTransactionImplementation newInstance() this.transactions.add( tx ); return tx; } - } - - private class GlobalKernelTransactionPool extends LinkedQueuePool - { - private Set transactions; - - GlobalKernelTransactionPool( Set transactions, - Factory factory ) - { - super( 8, factory ); - this.transactions = transactions; - } @Override protected void dispose( KernelTransactionImplementation tx ) diff --git a/community/primitive-collections/src/main/java/org/neo4j/collection/pool/LinkedQueuePool.java b/community/primitive-collections/src/main/java/org/neo4j/collection/pool/LinkedQueuePool.java index 6fa902cdcce93..50250e8af5211 100644 --- a/community/primitive-collections/src/main/java/org/neo4j/collection/pool/LinkedQueuePool.java +++ b/community/primitive-collections/src/main/java/org/neo4j/collection/pool/LinkedQueuePool.java @@ -24,9 +24,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.function.LongSupplier; -import org.neo4j.function.Factory; - -public class LinkedQueuePool implements Pool +public abstract class LinkedQueuePool implements Pool { public interface Monitor { @@ -105,39 +103,34 @@ public boolean shouldCheck() } } - public static final int DEFAULT_CHECK_INTERVAL = 60 * 1000; + private static final int DEFAULT_CHECK_INTERVAL = 60 * 1000; private final Queue unused = new ConcurrentLinkedQueue<>(); - private final Monitor monitor; + private final Monitor monitor; private final int minSize; - private final Factory factory; private final CheckStrategy checkStrategy; // Guarded by nothing. Those are estimates, losing some values doesn't matter much - private final AtomicInteger allocated = new AtomicInteger( 0 ); - private final AtomicInteger queueSize = new AtomicInteger( 0 ); + private final AtomicInteger allocated = new AtomicInteger(); + private final AtomicInteger queueSize = new AtomicInteger(); private int currentPeakSize; private int targetSize; - public LinkedQueuePool( int minSize, Factory factory ) + public LinkedQueuePool( int minSize ) { - this( minSize, factory, new CheckStrategy.TimeoutCheckStrategy( DEFAULT_CHECK_INTERVAL ), - new Monitor.Adapter() ); + this( minSize, new CheckStrategy.TimeoutCheckStrategy( DEFAULT_CHECK_INTERVAL ), + new Monitor.Adapter<>() ); } - public LinkedQueuePool( int minSize, Factory factory, CheckStrategy strategy, Monitor monitor ) + LinkedQueuePool( int minSize, CheckStrategy strategy, Monitor monitor ) { this.minSize = minSize; - this.factory = factory; this.currentPeakSize = 0; this.targetSize = minSize; this.checkStrategy = strategy; this.monitor = monitor; } - protected R create() - { - return factory.newInstance(); - } + protected abstract R create(); protected void dispose( R resource ) { diff --git a/community/primitive-collections/src/main/java/org/neo4j/collection/pool/MarshlandPool.java b/community/primitive-collections/src/main/java/org/neo4j/collection/pool/MarshlandPool.java index 9ba8c8c331cc9..8775b8b80fa23 100644 --- a/community/primitive-collections/src/main/java/org/neo4j/collection/pool/MarshlandPool.java +++ b/community/primitive-collections/src/main/java/org/neo4j/collection/pool/MarshlandPool.java @@ -24,8 +24,6 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import org.neo4j.function.Factory; - import static java.util.Collections.newSetFromMap; /** @@ -67,11 +65,6 @@ protected LocalSlot initialValue() newSetFromMap( new ConcurrentHashMap() ); private final ReferenceQueue> objectsFromDeadThreads = new ReferenceQueue<>(); - public MarshlandPool( Factory objectFactory ) - { - this( new LinkedQueuePool<>( 4, objectFactory ) ); - } - public MarshlandPool( Pool delegatePool ) { this.pool = delegatePool; @@ -91,6 +84,7 @@ public T acquire() } // Try the reference queue, containing objects from dead threads + @SuppressWarnings( "unchecked" ) LocalSlotReference slotReference = (LocalSlotReference) objectsFromDeadThreads.poll(); if ( slotReference != null && slotReference.object != null ) { @@ -112,10 +106,9 @@ public void release( T obj ) { localSlot.set( obj ); } - - // Fall back to the delegate pool else { + // Fall back to the delegate pool pool.release( obj ); } } @@ -123,6 +116,7 @@ public void release( T obj ) /** * Dispose of all objects in this pool, releasing them back to the delegate pool */ + @SuppressWarnings( "unchecked" ) public void disposeAll() { for ( LocalSlotReference slotReference : slotReferences ) @@ -151,11 +145,6 @@ public void disposeAll() } } - public void close() - { - disposeAll(); - } - /** * This is used to trigger the GC to notify us whenever the thread local has been garbage collected. */ diff --git a/community/primitive-collections/src/test/java/org/neo4j/collection/pool/LinkedQueuePoolTest.java b/community/primitive-collections/src/test/java/org/neo4j/collection/pool/LinkedQueuePoolTest.java index dd011b6bb3d66..440b5e20a1766 100644 --- a/community/primitive-collections/src/test/java/org/neo4j/collection/pool/LinkedQueuePoolTest.java +++ b/community/primitive-collections/src/test/java/org/neo4j/collection/pool/LinkedQueuePoolTest.java @@ -300,8 +300,15 @@ private void buildAPeakOfAcquiredFlyweightsAndTriggerAlarmWithSideEffects( int M private LinkedQueuePool getLinkedQueuePool( StatefulMonitor stateMonitor, FakeClock clock, int minSize ) { - return new LinkedQueuePool<>( minSize, Object::new, - new LinkedQueuePool.CheckStrategy.TimeoutCheckStrategy( 100, clock ), stateMonitor ); + return new LinkedQueuePool( minSize, + new LinkedQueuePool.CheckStrategy.TimeoutCheckStrategy( 100, clock ), stateMonitor ) + { + @Override + protected Object create() + { + return new Object(); + } + }; } private List> acquireFromPool( final LinkedQueuePool pool, int times ) diff --git a/community/primitive-collections/src/test/java/org/neo4j/collection/pool/MarshlandPoolTest.java b/community/primitive-collections/src/test/java/org/neo4j/collection/pool/MarshlandPoolTest.java index ca3aedc52d46d..a614512fdad73 100644 --- a/community/primitive-collections/src/test/java/org/neo4j/collection/pool/MarshlandPoolTest.java +++ b/community/primitive-collections/src/test/java/org/neo4j/collection/pool/MarshlandPoolTest.java @@ -76,7 +76,7 @@ public void shouldReturnToDelegatePoolIfLocalPoolIsFull() throws Exception } @Test - public void shouldReleaseAllSlotsOnClose() throws Exception + public void shouldReleaseAllSlotsOnDisposeAll() throws Exception { // Given Pool delegatePool = mock( Pool.class ); @@ -88,7 +88,7 @@ public void shouldReleaseAllSlotsOnClose() throws Exception pool.release( first ); // When - pool.close(); + pool.disposeAll(); // Then verify( delegatePool, times( 1 ) ).acquire(); diff --git a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/lock/forseti/ForsetiLockManager.java b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/lock/forseti/ForsetiLockManager.java index 5728d2c41e775..cb15fea56b5ae 100644 --- a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/lock/forseti/ForsetiLockManager.java +++ b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/lock/forseti/ForsetiLockManager.java @@ -254,7 +254,7 @@ public void close() private static class ForsetiClientFlyweightPool extends LinkedQueuePool { /** Client id counter **/ - private final AtomicInteger clientIds = new AtomicInteger( 0 ); + private final AtomicInteger clientIds = new AtomicInteger(); /** Re-use ids, forseti uses these in arrays, so we want to keep them low and not loose them. */ // TODO we could use a synchronised SimpleBitSet instead, since we know that we only care about reusing a @@ -270,7 +270,7 @@ private static class ForsetiClientFlyweightPool extends LinkedQueuePool[] lockMaps, WaitStrategy[] waitStrategies ) { - super( 128, null ); + super( 128 ); this.config = config; this.clock = clock; this.lockMaps = lockMaps;