HADOOP-14938. Configuration.updatingResource map should be initialized lazily 
(mi...@cloudera.com via rkanter)


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

Branch: refs/heads/YARN-1011
Commit: e163f41850bd09a17d3102a3af0af2e3cd831ab0
Parents: 7a27c2c
Author: Robert Kanter <rkan...@apache.org>
Authored: Fri Oct 13 13:52:58 2017 -0700
Committer: Robert Kanter <rkan...@apache.org>
Committed: Fri Oct 13 13:52:58 2017 -0700

----------------------------------------------------------------------
 .../org/apache/hadoop/conf/Configuration.java   | 97 ++++++++++++--------
 1 file changed, 57 insertions(+), 40 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/e163f418/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
----------------------------------------------------------------------
diff --git 
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
 
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
index 9d5bb1b..f94eba6 100644
--- 
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
+++ 
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
@@ -290,9 +290,9 @@ public class Configuration implements 
Iterable<Map.Entry<String,String>>,
 
   /**
    * Stores the mapping of key to the resource which modifies or loads 
-   * the key most recently
+   * the key most recently. Created lazily to avoid wasting memory.
    */
-  private Map<String, String[]> updatingResource;
+  private volatile Map<String, String[]> updatingResource;
 
   /**
    * Specify exact input factory to avoid time finding correct one.
@@ -749,7 +749,6 @@ public class Configuration implements 
Iterable<Map.Entry<String,String>>,
    */
   public Configuration(boolean loadDefaults) {
     this.loadDefaults = loadDefaults;
-    updatingResource = new ConcurrentHashMap<String, String[]>();
 
     // Register all classes holding property tags with
     REGISTERED_TAG_CLASS.put("core", CorePropertyTag.class);
@@ -768,25 +767,27 @@ public class Configuration implements 
Iterable<Map.Entry<String,String>>,
    */
   @SuppressWarnings("unchecked")
   public Configuration(Configuration other) {
-   this.resources = (ArrayList<Resource>) other.resources.clone();
-   synchronized(other) {
-     if (other.properties != null) {
-       this.properties = (Properties)other.properties.clone();
-     }
-
-     if (other.overlay!=null) {
-       this.overlay = (Properties)other.overlay.clone();
-     }
-
-     this.updatingResource = new ConcurrentHashMap<String, String[]>(
-         other.updatingResource);
-     this.finalParameters = Collections.newSetFromMap(
-         new ConcurrentHashMap<String, Boolean>());
-     this.finalParameters.addAll(other.finalParameters);
-     this.REGISTERED_TAG_CLASS.putAll(other.REGISTERED_TAG_CLASS);
-     this.propertyTagsMap.putAll(other.propertyTagsMap);
-   }
-   
+    this.resources = (ArrayList<Resource>) other.resources.clone();
+    synchronized(other) {
+      if (other.properties != null) {
+        this.properties = (Properties)other.properties.clone();
+      }
+
+      if (other.overlay!=null) {
+        this.overlay = (Properties)other.overlay.clone();
+      }
+
+      if (other.updatingResource != null) {
+        this.updatingResource = new ConcurrentHashMap<String, String[]>(
+           other.updatingResource);
+      }
+      this.finalParameters = Collections.newSetFromMap(
+          new ConcurrentHashMap<String, Boolean>());
+      this.finalParameters.addAll(other.finalParameters);
+      this.REGISTERED_TAG_CLASS.putAll(other.REGISTERED_TAG_CLASS);
+      this.propertyTagsMap.putAll(other.propertyTagsMap);
+    }
+
     synchronized(Configuration.class) {
       REGISTRY.put(this, null);
     }
@@ -1277,14 +1278,14 @@ public class Configuration implements 
Iterable<Map.Entry<String,String>>,
     String newSource = (source == null ? "programmatically" : source);
 
     if (!isDeprecated(name)) {
-      updatingResource.put(name, new String[] {newSource});
+      putIntoUpdatingResource(name, new String[] {newSource});
       String[] altNames = getAlternativeNames(name);
       if(altNames != null) {
         for(String n: altNames) {
           if(!n.equals(name)) {
             getOverlay().setProperty(n, value);
             getProps().setProperty(n, value);
-            updatingResource.put(n, new String[] {newSource});
+            putIntoUpdatingResource(n, new String[] {newSource});
           }
         }
       }
@@ -1295,7 +1296,7 @@ public class Configuration implements 
Iterable<Map.Entry<String,String>>,
       for(String n : names) {
         getOverlay().setProperty(n, value);
         getProps().setProperty(n, value);
-        updatingResource.put(n, new String[] {altSource});
+        putIntoUpdatingResource(n, new String[] {altSource});
       }
     }
   }
@@ -2634,17 +2635,19 @@ public class Configuration implements 
Iterable<Map.Entry<String,String>>,
   protected synchronized Properties getProps() {
     if (properties == null) {
       properties = new Properties();
-      Map<String, String[]> backup =
-          new ConcurrentHashMap<String, String[]>(updatingResource);
+      Map<String, String[]> backup = updatingResource != null ?
+          new ConcurrentHashMap<String, String[]>(updatingResource) : null;
       loadResources(properties, resources, quietmode);
 
       if (overlay != null) {
         properties.putAll(overlay);
-        for (Map.Entry<Object,Object> item: overlay.entrySet()) {
-          String key = (String)item.getKey();
-          String[] source = backup.get(key);
-          if(source != null) {
-            updatingResource.put(key, source);
+        if (backup != null) {
+          for (Map.Entry<Object, Object> item : overlay.entrySet()) {
+            String key = (String) item.getKey();
+            String[] source = backup.get(key);
+            if (source != null) {
+              updatingResource.put(key, source);
+            }
           }
         }
       }
@@ -3060,8 +3063,8 @@ public class Configuration implements 
Iterable<Map.Entry<String,String>>,
       }
       if (!finalParameters.contains(attr)) {
         properties.setProperty(attr, value);
-        if(source != null) {
-          updatingResource.put(attr, source);
+        if (source != null) {
+          putIntoUpdatingResource(attr, source);
         }
       } else {
         // This is a final parameter so check for overrides.
@@ -3366,9 +3369,10 @@ public class Configuration implements 
Iterable<Map.Entry<String,String>>,
           redactor.redact(name, config.get(name)));
       jsonGen.writeBooleanField("isFinal",
           config.finalParameters.contains(name));
-      String[] resources = config.updatingResource.get(name);
+      String[] resources = config.updatingResource != null ?
+          config.updatingResource.get(name) : null;
       String resource = UNKNOWN_RESOURCE;
-      if(resources != null && resources.length > 0) {
+      if (resources != null && resources.length > 0) {
         resource = resources[0];
       }
       jsonGen.writeStringField("resource", resource);
@@ -3448,8 +3452,8 @@ public class Configuration implements 
Iterable<Map.Entry<String,String>>,
       String value = org.apache.hadoop.io.Text.readString(in);
       set(key, value); 
       String sources[] = WritableUtils.readCompressedStringArray(in);
-      if(sources != null) {
-        updatingResource.put(key, sources);
+      if (sources != null) {
+        putIntoUpdatingResource(key, sources);
       }
     }
   }
@@ -3462,8 +3466,8 @@ public class Configuration implements 
Iterable<Map.Entry<String,String>>,
     for(Map.Entry<Object, Object> item: props.entrySet()) {
       org.apache.hadoop.io.Text.writeString(out, (String) item.getKey());
       org.apache.hadoop.io.Text.writeString(out, (String) item.getValue());
-      WritableUtils.writeCompressedStringArray(out, 
-          updatingResource.get(item.getKey()));
+      WritableUtils.writeCompressedStringArray(out, updatingResource != null ?
+          updatingResource.get(item.getKey()) : null);
     }
   }
   
@@ -3563,4 +3567,17 @@ public class Configuration implements 
Iterable<Map.Entry<String,String>>,
     }
     return tag;
   }
+
+  private void putIntoUpdatingResource(String key, String[] value) {
+    Map<String, String[]> localUR = updatingResource;
+    if (localUR == null) {
+      synchronized (this) {
+        localUR = updatingResource;
+        if (localUR == null) {
+          updatingResource = localUR = new ConcurrentHashMap<>(8);
+        }
+      }
+    }
+    localUR.put(key, value);
+  }
 }


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

Reply via email to