Memory Optimizations:

* Cache the FullyQualifiedName instances. Due to a very high number of duplicated resource keys, all FullyQualifiedNames should be effectively interned.
* Presume xliff in all resources. Inlining the xmlns is a bit costly.

--
MOS_MIGRATED_REVID=122375955

3 files changed, 117 insertions, 82 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidDataWriter.java b/src/tools/android/java/com/google/devtools/build/android/AndroidDataWriter.java
index 50895ac222..e3e7234f41 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidDataWriter.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidDataWriter.java
@@ -101,8 +101,9 @@ public class AndroidDataWriter implements Flushable, AndroidDataWritingVisitor {
     }
   }
 
-  private static final char[] START_RESOURCES = "<resources>".toCharArray();
-  private static final char[] END_RESOURCES = "</resources>".toCharArray();
+  public static final char[] START_RESOURCES =
+      ("<resources xmlns:xliff=\"" + XmlResourceValues.XLIFF_NAMESPACE + "\">").toCharArray();
+  public static final char[] END_RESOURCES = "</resources>".toCharArray();
   private static final char[] LINE_END = "\n".toCharArray();
   private static final PngCruncher NOOP_CRUNCHER =
       new PngCruncher() {
diff --git a/src/tools/android/java/com/google/devtools/build/android/FullyQualifiedName.java b/src/tools/android/java/com/google/devtools/build/android/FullyQualifiedName.java
index cab92ff202..0f7ee755fc 100644
--- a/src/tools/android/java/com/google/devtools/build/android/FullyQualifiedName.java
+++ b/src/tools/android/java/com/google/devtools/build/android/FullyQualifiedName.java
@@ -18,7 +18,6 @@ import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
-import com.google.common.collect.Ordering;
 import com.google.devtools.build.android.proto.SerializeFormat;
 
 import com.android.resources.ResourceType;
@@ -29,6 +28,10 @@ import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -46,89 +49,38 @@ public class FullyQualifiedName implements DataKey, Comparable<FullyQualifiedNam
   public static final String DEFAULT_PACKAGE = "res-auto";
   private static final Joiner DASH_JOINER = Joiner.on('-');
 
-  private final String pkg;
-  private final ImmutableList<String> qualifiers;
-  private final ResourceType resourceType;
-  private final String resourceName;
-
-  /**
-   * Returns a string path representation of the FullyQualifiedName.
-   *
-   * Non-values Android Resource have a well defined file layout: From the resource directory, they
-   * reside in &lt;resource type&gt;[-&lt;qualifier&gt;]/&lt;resource name&gt;[.extension]
-   *
-   * @param sourceExtension The extension of the resource represented by the FullyQualifiedName
-   * @return A string representation of the FullyQualifiedName with the provided extension.
-   */
-  public String toPathString(String sourceExtension) {
-    // TODO(corysmith): Does the extension belong in the FullyQualifiedName?
-    return Paths.get(
-            DASH_JOINER.join(
-                ImmutableList.<String>builder()
-                    .add(resourceType.getName())
-                    .addAll(qualifiers)
-                    .build()),
-            resourceName + sourceExtension)
-        .toString();
-  }
-
-  @Override
-  public String toPrettyString() {
-    // TODO(corysmith): Add package when we start tracking it.
-    return String.format(
-        "%s/%s",
-        DASH_JOINER.join(
-            ImmutableList.<String>builder().add(resourceType.getName()).addAll(qualifiers).build()),
-        resourceName);
-  }
-
-  /**
-   * Returns the string path representation of the values directory and qualifiers.
-   *
-   * Certain resource types live in the "values" directory. This will calculate the directory and
-   * ensure the qualifiers are represented.
-   */
-  // TODO(corysmith): Combine this with toPathString to clean up the interface of FullyQualifiedName
-  // logically, the FullyQualifiedName should just be able to provide the relative path string for
-  // the resource.
-  public String valuesPath() {
-    return Paths.get(
-            DASH_JOINER.join(
-                ImmutableList.<String>builder().add("values").addAll(qualifiers).build()),
-            "values.xml")
-        .toString();
-  }
-
-  public String name() {
-    return resourceName;
-  }
+  // To save on memory, always return one instance for each FullyQualifiedName.
+  // Using a HashMap to deduplicate the instances -- the key retrieves a single instance.
+  private static final ConcurrentMap<FullyQualifiedName, FullyQualifiedName> instanceCache =
+      new ConcurrentHashMap<>();
+  private static final AtomicInteger cacheHit = new AtomicInteger(0);
 
   /**
    * A factory for parsing an generating FullyQualified names with qualifiers and package.
    */
   public static class Factory {
-
+  
     /** Used to adjust the api number for indvidual qualifiers. */
     private static final class QualifierApiAdjuster {
       private final int minApi;
       private final ImmutableSet<String> values;
       private final Pattern pattern;
-
+  
       static QualifierApiAdjuster fromRegex(int minApi, String regex) {
         return new QualifierApiAdjuster(minApi, ImmutableSet.<String>of(), Pattern.compile(regex));
       }
-
+  
       static QualifierApiAdjuster fromValues(int minApi, String... values) {
         return new QualifierApiAdjuster(minApi, ImmutableSet.copyOf(values), null);
       }
-
+  
       private QualifierApiAdjuster(
           int minApi, @Nullable ImmutableSet<String> values, @Nullable Pattern pattern) {
         this.minApi = minApi;
         this.values = ImmutableSet.copyOf(values);
         this.pattern = pattern;
       }
-
+  
       /** Checks to see if the qualifier string is this type of qualifier. */
       boolean check(String qualifier) {
         if (pattern != null) {
@@ -136,13 +88,13 @@ public class FullyQualifiedName implements DataKey, Comparable<FullyQualifiedNam
         }
         return values.contains(qualifier);
       }
-
+  
       /** Takes the current api and returns a higher one if the qualifier requires it. */
       int maxApi(int current) {
         return current > minApi ? current : minApi;
       }
     }
-
+  
     /**
      * An array used to calculate the api levels.
      *
@@ -200,10 +152,10 @@ public class FullyQualifiedName implements DataKey, Comparable<FullyQualifiedNam
             "Could not find either resource type (%%s) or name (%%s) in %%s. "
                 + "It should be in the pattern [package:]{%s}/resourcename",
             Joiner.on(",").join(ResourceType.values()));
-    private final List<String> qualifiers;
+    private final ImmutableList<String> qualifiers;
     private final String pkg;
 
-    private Factory(List<String> qualifiers, String pkg) {
+    private Factory(ImmutableList<String> qualifiers, String pkg) {
       this.qualifiers = qualifiers;
       this.pkg = pkg;
     }
@@ -260,11 +212,11 @@ public class FullyQualifiedName implements DataKey, Comparable<FullyQualifiedNam
     }
 
     public static Factory from(List<String> qualifiers, String pkg) {
-      return new Factory(qualifiers, pkg);
+      return new Factory(ImmutableList.copyOf(qualifiers), pkg);
     }
 
     public static Factory from(List<String> qualifiers) {
-      return from(qualifiers, DEFAULT_PACKAGE);
+      return from(ImmutableList.copyOf(qualifiers), DEFAULT_PACKAGE);
     }
 
     public FullyQualifiedName create(ResourceType resourceType, String resourceName) {
@@ -291,7 +243,7 @@ public class FullyQualifiedName implements DataKey, Comparable<FullyQualifiedNam
       String parsedPackage = matcher.group("package");
       ResourceType resourceType = ResourceType.getEnum(matcher.group("type"));
       String resourceName = matcher.group("name");
-
+  
       if (resourceType == null || resourceName == null) {
         throw new IllegalArgumentException(
             String.format(
@@ -317,8 +269,20 @@ public class FullyQualifiedName implements DataKey, Comparable<FullyQualifiedNam
    */
   public static FullyQualifiedName of(
       String pkg, List<String> qualifiers, ResourceType resourceType, String resourceName) {
-    return new FullyQualifiedName(
-        pkg, Ordering.natural().immutableSortedCopy(qualifiers), resourceType, resourceName);
+    ImmutableList<String> immutableQualifiers = ImmutableList.copyOf(qualifiers);
+    // TODO(corysmith): Address the GC thrash this creates by managing a simplified, mutable key to
+    // do the instance check.
+    FullyQualifiedName name =
+        new FullyQualifiedName(pkg, immutableQualifiers, resourceType, resourceName);
+    // Use putIfAbsent to get the canonical instance, if there. If it isn't, putIfAbsent will
+    // return null, and we should return the current instance.
+    FullyQualifiedName cached = instanceCache.putIfAbsent(name, name);
+    if (cached == null) {
+      return name;
+    } else {
+      cacheHit.incrementAndGet();
+      return cached;
+    }
   }
 
   public static FullyQualifiedName fromProto(SerializeFormat.DataKey protoKey) {
@@ -329,6 +293,71 @@ public class FullyQualifiedName implements DataKey, Comparable<FullyQualifiedNam
         protoKey.getKeyValue());
   }
 
+  public static void logCacheUsage(Logger logger) {
+    logger.fine(
+        String.format(
+            "Total FullyQualifiedName instance cache hits %s out of %s",
+            cacheHit.intValue(),
+            instanceCache.size()));
+  }
+
+  private final String pkg;
+  private final ImmutableList<String> qualifiers;
+  private final ResourceType resourceType;
+  private final String resourceName;
+
+  /**
+   * Returns a string path representation of the FullyQualifiedName.
+   *
+   * Non-values Android Resource have a well defined file layout: From the resource directory, they
+   * reside in &lt;resource type&gt;[-&lt;qualifier&gt;]/&lt;resource name&gt;[.extension]
+   *
+   * @param sourceExtension The extension of the resource represented by the FullyQualifiedName
+   * @return A string representation of the FullyQualifiedName with the provided extension.
+   */
+  public String toPathString(String sourceExtension) {
+    // TODO(corysmith): Does the extension belong in the FullyQualifiedName?
+    return Paths.get(
+            DASH_JOINER.join(
+                ImmutableList.<String>builder()
+                    .add(resourceType.getName())
+                    .addAll(qualifiers)
+                    .build()),
+            resourceName + sourceExtension)
+        .toString();
+  }
+
+  @Override
+  public String toPrettyString() {
+    // TODO(corysmith): Add package when we start tracking it.
+    return String.format(
+        "%s/%s",
+        DASH_JOINER.join(
+            ImmutableList.<String>builder().add(resourceType.getName()).addAll(qualifiers).build()),
+        resourceName);
+  }
+
+  /**
+   * Returns the string path representation of the values directory and qualifiers.
+   *
+   * Certain resource types live in the "values" directory. This will calculate the directory and
+   * ensure the qualifiers are represented.
+   */
+  // TODO(corysmith): Combine this with toPathString to clean up the interface of FullyQualifiedName
+  // logically, the FullyQualifiedName should just be able to provide the relative path string for
+  // the resource.
+  public String valuesPath() {
+    return Paths.get(
+            DASH_JOINER.join(
+                ImmutableList.<String>builder().add("values").addAll(qualifiers).build()),
+            "values.xml")
+        .toString();
+  }
+
+  public String name() {
+    return resourceName;
+  }
+
   private FullyQualifiedName(
       String pkg,
       ImmutableList<String> qualifiers,
@@ -346,9 +375,7 @@ public class FullyQualifiedName implements DataKey, Comparable<FullyQualifiedNam
     if (pkg.equals(newPackage)) {
       return this;
     }
-    // Don't use "of" because it ensures the qualifiers are sorted -- we already know
-    // they are sorted here.
-    return new FullyQualifiedName(newPackage, qualifiers, resourceType, resourceName);
+    return of(newPackage, qualifiers, resourceType, resourceName);
   }
 
   @Override
@@ -389,12 +416,11 @@ public class FullyQualifiedName implements DataKey, Comparable<FullyQualifiedNam
     if (!resourceName.equals(other.resourceName)) {
       return resourceName.compareTo(other.resourceName);
     }
-    // TODO(corysmith): Figure out a more performant stable way to keep a stable order.
     if (!qualifiers.equals(other.qualifiers)) {
       if (qualifiers.size() != other.qualifiers.size()) {
         return qualifiers.size() - other.qualifiers.size();
       }
-      // This works because the qualifiers are sorted on creation.
+      // This works because the qualifiers are always in an ordered sequence.
       return qualifiers.toString().compareTo(other.qualifiers.toString());
     }
     return 0;
diff --git a/src/tools/android/java/com/google/devtools/build/android/XmlResourceValues.java b/src/tools/android/java/com/google/devtools/build/android/XmlResourceValues.java
index 3a0eefc3c9..9c27f044bd 100644
--- a/src/tools/android/java/com/google/devtools/build/android/XmlResourceValues.java
+++ b/src/tools/android/java/com/google/devtools/build/android/XmlResourceValues.java
@@ -56,6 +56,7 @@ import javax.xml.stream.events.XMLEvent;
  * declared inside the &lt;resources&gt; tag.
  */
 public class XmlResourceValues {
+
   private static final Logger logger = Logger.getLogger(XmlResourceValues.class.getCanonicalName());
 
   private static final QName TAG_EAT_COMMENT = QName.valueOf("eat-comment");
@@ -73,6 +74,9 @@ public class XmlResourceValues {
   private static final QName ATTR_PARENT = QName.valueOf("parent");
   private static final QName ATTR_TYPE = QName.valueOf("type");
 
+  static final String XLIFF_NAMESPACE = "urn:oasis:names:tc:xliff:document:1.2";
+  private static final String XLIFF_PREFIX = "xliff";
+
   static XmlResourceValue parsePlurals(XMLEventReader eventReader) throws XMLStreamException {
     ImmutableMap.Builder<String, String> values = ImmutableMap.builder();
     for (XMLEvent element = nextTag(eventReader);
@@ -170,10 +174,14 @@ public class XmlResourceValues {
       }
       String value = escapeXmlValues(attribute.getValue()).replace("\"", "&quot;");
       if (!name.getNamespaceURI().isEmpty()) {
-        // Declare the xmlns here, so that the written xml will be semantically correct,
-        // if a bit verbose. This allows the resource keys to be written into a generic <resources>
-        // tag.
-        attributeMap.put("xmlns:" + name.getPrefix(), name.getNamespaceURI());
+        // xliff is always provided on resources to help with file size
+        if (!(XLIFF_NAMESPACE.equals(name.getNamespaceURI())
+            && XLIFF_PREFIX.equals(name.getPrefix()))) {
+          // Declare the xmlns here, so that the written xml will be semantically correct,
+          // if a bit verbose. This allows the resource keys to be written into a <resources>
+          // tag without a global namespace.
+          attributeMap.put("xmlns:" + name.getPrefix(), name.getNamespaceURI());
+        }
         attributeMap.put(name.getPrefix() + ":" + attribute.getName().getLocalPart(), value);
       } else {
         attributeMap.put(attribute.getName().getLocalPart(), value);