From 607705c488fa5263d851cee578a2d319e6e52ecd Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Mon, 3 Oct 2016 10:53:21 -0700 Subject: [PATCH] HDFS-10690. Optimize insertion/removal of replica in ShortCircuitCache. Contributed by Fenghua Hu. --- .../hdfs/shortcircuit/ShortCircuitCache.java | 88 ++++++++++++------- .../fs/TestEnhancedByteBufferAccess.java | 17 ++-- .../shortcircuit/TestShortCircuitCache.java | 9 +- 3 files changed, 69 insertions(+), 45 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/shortcircuit/ShortCircuitCache.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/shortcircuit/ShortCircuitCache.java index 62ade70442..bd02a9775c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/shortcircuit/ShortCircuitCache.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/shortcircuit/ShortCircuitCache.java @@ -26,13 +26,14 @@ import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; -import java.util.TreeMap; +import java.util.NoSuchElementException; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.ReentrantLock; +import org.apache.commons.collections.map.LinkedMap; import org.apache.commons.lang.mutable.MutableBoolean; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.hdfs.ExtendedBlockId; @@ -107,16 +108,20 @@ public void run() { int numDemoted = demoteOldEvictableMmaped(curMs); int numPurged = 0; - Long evictionTimeNs = (long) 0; + Long evictionTimeNs; while (true) { - Entry entry = - evictable.ceilingEntry(evictionTimeNs); - if (entry == null) break; - evictionTimeNs = entry.getKey(); + Object eldestKey; + try { + eldestKey = evictable.firstKey(); + } catch (NoSuchElementException e) { + break; + } + evictionTimeNs = (Long)eldestKey; long evictionTimeMs = TimeUnit.MILLISECONDS.convert(evictionTimeNs, TimeUnit.NANOSECONDS); if (evictionTimeMs + maxNonMmappedEvictableLifespanMs >= curMs) break; - ShortCircuitReplica replica = entry.getValue(); + ShortCircuitReplica replica = (ShortCircuitReplica)evictable.get( + eldestKey); if (LOG.isTraceEnabled()) { LOG.trace("CacheCleaner: purging " + replica + ": " + StringUtils.getStackTrace(Thread.currentThread())); @@ -263,11 +268,11 @@ public interface ShortCircuitReplicaCreator { private CacheCleaner cacheCleaner; /** - * Tree of evictable elements. + * LinkedMap of evictable elements. * * Maps (unique) insertion time in nanoseconds to the element. */ - private final TreeMap evictable = new TreeMap<>(); + private final LinkedMap evictable = new LinkedMap(); /** * Maximum total size of the cache, including both mmapped and @@ -281,12 +286,11 @@ public interface ShortCircuitReplicaCreator { private long maxNonMmappedEvictableLifespanMs; /** - * Tree of mmaped evictable elements. + * LinkedMap of mmaped evictable elements. * * Maps (unique) insertion time in nanoseconds to the element. */ - private final TreeMap evictableMmapped = - new TreeMap<>(); + private final LinkedMap evictableMmapped = new LinkedMap(); /** * Maximum number of mmaped evictable elements. @@ -482,13 +486,16 @@ void unref(ShortCircuitReplica replica) { private int demoteOldEvictableMmaped(long now) { int numDemoted = 0; boolean needMoreSpace = false; - Long evictionTimeNs = (long) 0; + Long evictionTimeNs; while (true) { - Entry entry = - evictableMmapped.ceilingEntry(evictionTimeNs); - if (entry == null) break; - evictionTimeNs = entry.getKey(); + Object eldestKey; + try { + eldestKey = evictableMmapped.firstKey(); + } catch (NoSuchElementException e) { + break; + } + evictionTimeNs = (Long)eldestKey; long evictionTimeMs = TimeUnit.MILLISECONDS.convert(evictionTimeNs, TimeUnit.NANOSECONDS); if (evictionTimeMs + maxEvictableMmapedLifespanMs >= now) { @@ -497,7 +504,8 @@ private int demoteOldEvictableMmaped(long now) { } needMoreSpace = true; } - ShortCircuitReplica replica = entry.getValue(); + ShortCircuitReplica replica = (ShortCircuitReplica)evictableMmapped.get( + eldestKey); if (LOG.isTraceEnabled()) { String rationale = needMoreSpace ? "because we need more space" : "because it's too old"; @@ -527,10 +535,15 @@ private void trimEvictionMaps() { return; } ShortCircuitReplica replica; - if (evictableSize == 0) { - replica = evictableMmapped.firstEntry().getValue(); - } else { - replica = evictable.firstEntry().getValue(); + try { + if (evictableSize == 0) { + replica = (ShortCircuitReplica)evictableMmapped.get(evictableMmapped + .firstKey()); + } else { + replica = (ShortCircuitReplica)evictable.get(evictable.firstKey()); + } + } catch (NoSuchElementException e) { + break; } if (LOG.isTraceEnabled()) { LOG.trace(this + ": trimEvictionMaps is purging " + replica + @@ -573,10 +586,11 @@ private String removeEvictable(ShortCircuitReplica replica) { * @param map The map to remove it from. */ private void removeEvictable(ShortCircuitReplica replica, - TreeMap map) { + LinkedMap map) { Long evictableTimeNs = replica.getEvictableTimeNs(); Preconditions.checkNotNull(evictableTimeNs); - ShortCircuitReplica removed = map.remove(evictableTimeNs); + ShortCircuitReplica removed = (ShortCircuitReplica)map.remove( + evictableTimeNs); Preconditions.checkState(removed == replica, "failed to make %s unevictable", replica); replica.setEvictableTimeNs(null); @@ -593,7 +607,7 @@ private void removeEvictable(ShortCircuitReplica replica, * @param map The map to insert it into. */ private void insertEvictable(Long evictionTimeNs, - ShortCircuitReplica replica, TreeMap map) { + ShortCircuitReplica replica, LinkedMap map) { while (map.containsKey(evictionTimeNs)) { evictionTimeNs++; } @@ -861,14 +875,22 @@ public void close() { IOUtilsClient.cleanup(LOG, cacheCleaner); // Purge all replicas. while (true) { - Entry entry = evictable.firstEntry(); - if (entry == null) break; - purge(entry.getValue()); + Object eldestKey; + try { + eldestKey = evictable.firstKey(); + } catch (NoSuchElementException e) { + break; + } + purge((ShortCircuitReplica)evictable.get(eldestKey)); } while (true) { - Entry entry = evictableMmapped.firstEntry(); - if (entry == null) break; - purge(entry.getValue()); + Object eldestKey; + try { + eldestKey = evictableMmapped.firstKey(); + } catch (NoSuchElementException e) { + break; + } + purge((ShortCircuitReplica)evictableMmapped.get(eldestKey)); } } finally { lock.unlock(); @@ -909,8 +931,8 @@ public interface CacheVisitor { void visit(int numOutstandingMmaps, Map replicas, Map failedLoads, - Map evictable, - Map evictableMmapped); + LinkedMap evictable, + LinkedMap evictableMmapped); } @VisibleForTesting // ONLY for testing diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestEnhancedByteBufferAccess.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestEnhancedByteBufferAccess.java index 0ccc07a833..9cd46c191d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestEnhancedByteBufferAccess.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestEnhancedByteBufferAccess.java @@ -34,6 +34,7 @@ import java.util.Random; import java.util.concurrent.TimeoutException; +import org.apache.commons.collections.map.LinkedMap; import org.apache.commons.lang.SystemUtils; import org.apache.commons.lang.mutable.MutableBoolean; import org.apache.commons.logging.Log; @@ -307,8 +308,8 @@ private static class CountingVisitor implements CacheVisitor { public void visit(int numOutstandingMmaps, Map replicas, Map failedLoads, - Map evictable, - Map evictableMmapped) { + LinkedMap evictable, + LinkedMap evictableMmapped) { if (expectedNumOutstandingMmaps >= 0) { Assert.assertEquals(expectedNumOutstandingMmaps, numOutstandingMmaps); } @@ -373,8 +374,8 @@ public void testZeroCopyMmapCache() throws Exception { public void visit(int numOutstandingMmaps, Map replicas, Map failedLoads, - Map evictable, - Map evictableMmapped) { + LinkedMap evictable, + LinkedMap evictableMmapped) { ShortCircuitReplica replica = replicas.get( new ExtendedBlockId(firstBlock.getBlockId(), firstBlock.getBlockPoolId())); Assert.assertNotNull(replica); @@ -410,8 +411,8 @@ public Boolean get() { public void visit(int numOutstandingMmaps, Map replicas, Map failedLoads, - Map evictable, - Map evictableMmapped) { + LinkedMap evictable, + LinkedMap evictableMmapped) { finished.setValue(evictableMmapped.isEmpty()); } }); @@ -685,8 +686,8 @@ public Boolean get() { public void visit(int numOutstandingMmaps, Map replicas, Map failedLoads, - Map evictable, - Map evictableMmapped) { + LinkedMap evictable, + LinkedMap evictableMmapped) { Assert.assertEquals(expectedOutstandingMmaps, numOutstandingMmaps); ShortCircuitReplica replica = replicas.get(ExtendedBlockId.fromExtendedBlock(block)); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/shortcircuit/TestShortCircuitCache.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/shortcircuit/TestShortCircuitCache.java index ac14438c63..8e217c2219 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/shortcircuit/TestShortCircuitCache.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/shortcircuit/TestShortCircuitCache.java @@ -34,6 +34,7 @@ import java.util.Map; import java.util.concurrent.TimeoutException; +import org.apache.commons.collections.map.LinkedMap; import org.apache.commons.lang.mutable.MutableBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -502,8 +503,8 @@ public void testShmBasedStaleness() throws Exception { public void visit(int numOutstandingMmaps, Map replicas, Map failedLoads, - Map evictable, - Map evictableMmapped) { + LinkedMap evictable, + LinkedMap evictableMmapped) { ShortCircuitReplica replica = replicas.get( ExtendedBlockId.fromExtendedBlock(block)); Assert.assertNotNull(replica); @@ -518,8 +519,8 @@ public void visit(int numOutstandingMmaps, public void visit(int numOutstandingMmaps, Map replicas, Map failedLoads, - Map evictable, - Map evictableMmapped) { + LinkedMap evictable, + LinkedMap evictableMmapped) { ShortCircuitReplica replica = replicas.get( ExtendedBlockId.fromExtendedBlock(block)); Assert.assertNotNull(replica);