diff options
author | 2016-05-15 18:30:52 +0000 | |
---|---|---|
committer | 2016-05-16 15:17:00 +0000 | |
commit | db89a643180895c0b9ca27911adf1ff692212d1c (patch) | |
tree | 09f928569cb11ef4eb70e3e1133c725a25ab6978 /src/tools/android | |
parent | e37c55eccbd4516b2db7aaf58ef95209dfad3ed4 (diff) |
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
Diffstat (limited to 'src/tools/android')
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 <resource type>[-<qualifier>]/<resource name>[.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 <resource type>[-<qualifier>]/<resource name>[.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 <resources> 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("\"", """); 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); |