You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by we...@apache.org on 2017/06/30 17:25:51 UTC

hadoop git commit: HDFS-12042. Lazy initialize AbstractINodeDiffList#diffs for snapshots to reduce memory consumption. Contributed by Misha Dmitriev.

Repository: hadoop
Updated Branches:
  refs/heads/trunk 6a9dc5f44 -> bcba844d1


HDFS-12042. Lazy initialize AbstractINodeDiffList#diffs for snapshots to reduce memory consumption. Contributed by Misha Dmitriev.


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/bcba844d
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/bcba844d
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/bcba844d

Branch: refs/heads/trunk
Commit: bcba844d1144cc334e2babbc34c9d42eac1c203a
Parents: 6a9dc5f
Author: Wei-Chiu Chuang <we...@apache.org>
Authored: Fri Jun 30 10:28:01 2017 -0700
Committer: Wei-Chiu Chuang <we...@apache.org>
Committed: Fri Jun 30 10:28:01 2017 -0700

----------------------------------------------------------------------
 .../hdfs/server/namenode/INodeDirectory.java    |  7 ++-
 .../snapshot/AbstractINodeDiffList.java         | 53 +++++++++++++++-----
 .../namenode/TestTruncateQuotaUpdate.java       |  1 +
 3 files changed, 46 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/bcba844d/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
index a29a118..4012783 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
@@ -65,8 +65,11 @@ public class INodeDirectory extends INodeWithAdditionalFields
     return inode.asDirectory(); 
   }
 
-  protected static final int DEFAULT_FILES_PER_DIRECTORY = 5;
-  final static byte[] ROOT_NAME = DFSUtil.string2Bytes("");
+  // Profiling shows that most of the file lists are between 1 and 4 elements.
+  // Thus allocate the corresponding ArrayLists with a small initial capacity.
+  public static final int DEFAULT_FILES_PER_DIRECTORY = 2;
+
+  static final byte[] ROOT_NAME = DFSUtil.string2Bytes("");
 
   private List<INode> children = null;
   

http://git-wip-us.apache.org/repos/asf/hadoop/blob/bcba844d/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
index 64825f1..98d8c53 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
@@ -24,6 +24,7 @@ import java.util.List;
 
 import org.apache.hadoop.hdfs.server.namenode.INode;
 import org.apache.hadoop.hdfs.server.namenode.INodeAttributes;
+import org.apache.hadoop.hdfs.server.namenode.INodeDirectory;
 
 /**
  * A list of snapshot diffs for storing snapshot data.
@@ -35,17 +36,19 @@ abstract class AbstractINodeDiffList<N extends INode,
                                      A extends INodeAttributes,
                                      D extends AbstractINodeDiff<N, A, D>> 
     implements Iterable<D> {
-  /** Diff list sorted by snapshot IDs, i.e. in chronological order. */
-  private final List<D> diffs = new ArrayList<D>();
+  /** Diff list sorted by snapshot IDs, i.e. in chronological order.
+    * Created lazily to avoid wasting memory by empty lists. */
+  private List<D> diffs;
 
   /** @return this list as a unmodifiable {@link List}. */
   public final List<D> asList() {
-    return Collections.unmodifiableList(diffs);
+    return diffs != null ?
+        Collections.unmodifiableList(diffs) : Collections.emptyList();
   }
   
-  /** Get the size of the list and then clear it. */
+  /** Clear the list. */
   public void clear() {
-    diffs.clear();
+    diffs = null;
   }
 
   /** @return an {@link AbstractINodeDiff}. */
@@ -66,6 +69,9 @@ abstract class AbstractINodeDiffList<N extends INode,
    */
   public final void deleteSnapshotDiff(INode.ReclaimContext reclaimContext,
       final int snapshot, final int prior, final N currentINode) {
+    if (diffs == null) {
+      return;
+    }
     int snapshotIndex = Collections.binarySearch(diffs, snapshot);
 
     D removed;
@@ -75,6 +81,9 @@ abstract class AbstractINodeDiffList<N extends INode,
         diffs.get(snapshotIndex).setSnapshotId(prior);
       } else { // there is no snapshot before
         removed = diffs.remove(0);
+        if (diffs.isEmpty()) {
+          diffs = null;
+        }
         removed.destroyDiffAndCollectBlocks(reclaimContext, currentINode);
       }
     } else if (snapshotIndex > 0) {
@@ -103,6 +112,7 @@ abstract class AbstractINodeDiffList<N extends INode,
 
   /** Append the diff at the end of the list. */
   private D addLast(D diff) {
+    createDiffsIfNeeded();
     final D last = getLast();
     diffs.add(diff);
     if (last != null) {
@@ -113,15 +123,25 @@ abstract class AbstractINodeDiffList<N extends INode,
   
   /** Add the diff to the beginning of the list. */
   final void addFirst(D diff) {
-    final D first = diffs.isEmpty()? null: diffs.get(0);
+    createDiffsIfNeeded();
+    final D first = diffs.isEmpty()? null : diffs.get(0);
     diffs.add(0, diff);
     diff.setPosterior(first);
   }
 
   /** @return the last diff. */
   public final D getLast() {
-    final int n = diffs.size();
-    return n == 0? null: diffs.get(n - 1);
+    if (diffs == null) {
+      return null;
+    }
+    int n = diffs.size();
+    return n == 0 ? null : diffs.get(n - 1);
+  }
+
+  private void createDiffsIfNeeded() {
+    if (diffs == null) {
+      diffs = new ArrayList<>(INodeDirectory.DEFAULT_FILES_PER_DIRECTORY);
+    }
   }
 
   /** @return the id of the last snapshot. */
@@ -139,10 +159,14 @@ abstract class AbstractINodeDiffList<N extends INode,
    * @return The id of the latest snapshot before the given snapshot.
    */
   public final int getPrior(int anchorId, boolean exclusive) {
+    if (diffs == null) {
+      return Snapshot.NO_SNAPSHOT_ID;
+    }
     if (anchorId == Snapshot.CURRENT_STATE_ID) {
       int last = getLastSnapshotId();
-      if(exclusive && last == anchorId)
+      if (exclusive && last == anchorId) {
         return Snapshot.NO_SNAPSHOT_ID;
+      }
       return last;
     }
     final int i = Collections.binarySearch(diffs, anchorId);
@@ -181,7 +205,7 @@ abstract class AbstractINodeDiffList<N extends INode,
   }
   
   public final D getDiffById(final int snapshotId) {
-    if (snapshotId == Snapshot.CURRENT_STATE_ID) {
+    if (snapshotId == Snapshot.CURRENT_STATE_ID || diffs == null) {
       return null;
     }
     final int i = Collections.binarySearch(diffs, snapshotId);
@@ -193,7 +217,7 @@ abstract class AbstractINodeDiffList<N extends INode,
       // given snapshot and the next state so that the diff for the given
       // snapshot was not recorded. Thus, return the next state.
       final int j = -i - 1;
-      return j < diffs.size()? diffs.get(j): null;
+      return j < diffs.size() ? diffs.get(j) : null;
     }
   }
   
@@ -207,6 +231,9 @@ abstract class AbstractINodeDiffList<N extends INode,
   }
 
   final int[] changedBetweenSnapshots(Snapshot from, Snapshot to) {
+    if (diffs == null) {
+      return null;
+    }
     Snapshot earlier = from;
     Snapshot later = to;
     if (Snapshot.ID_COMPARATOR.compare(from, to) > 0) {
@@ -275,11 +302,11 @@ abstract class AbstractINodeDiffList<N extends INode,
   
   @Override
   public Iterator<D> iterator() {
-    return diffs.iterator();
+    return diffs != null ? diffs.iterator() : Collections.emptyIterator();
   }
 
   @Override
   public String toString() {
-    return getClass().getSimpleName() + ": " + diffs;
+    return getClass().getSimpleName() + ": " + (diffs != null ? diffs : "[]");
   }
 }

http://git-wip-us.apache.org/repos/asf/hadoop/blob/bcba844d/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestTruncateQuotaUpdate.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestTruncateQuotaUpdate.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestTruncateQuotaUpdate.java
index 106edad..fcdd650 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestTruncateQuotaUpdate.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestTruncateQuotaUpdate.java
@@ -156,6 +156,7 @@ public class TestTruncateQuotaUpdate {
     FileDiff diff = mock(FileDiff.class);
     when(diff.getBlocks()).thenReturn(blocks);
     FileDiffList diffList = new FileDiffList();
+    Whitebox.setInternalState(diffList, "diffs", new ArrayList<FileDiff>());
     @SuppressWarnings("unchecked")
     ArrayList<FileDiff> diffs = ((ArrayList<FileDiff>)Whitebox.getInternalState
         (diffList, "diffs"));


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org