From 8e1c1f4d9dc93f1d4ca324092462690e27ca13e1 Mon Sep 17 00:00:00 2001 From: Tobias Thierer Date: Tue, 8 Oct 2019 16:27:35 +0100 Subject: Minimize default MIME map and optimize loading. This CL topic speeds up DefaultMimeMapFactory.create() from 15.5msec to 5.7msec (measured by bundling a copy of the logic and data into a test app) and saves 5.2 KByte of space (16 KBytes uncompressed) in framework.jar. This CL topic does not change the amount of heap memory consumed by the loaded MimeMap. This is achieved by moving the following parsing steps from DefaultMimeMapFactory into a build-time optimization step: * strip off comments (starting with '#') * normalize whitespace (trim leading and trailing whitespace; replace remaining whitespace with a single ' '). * drop lines that do not contain a ' ' after this step (those only contained comments or only a MIME type without any extension). * use String.indexOf(char) in favor of String.contains(String) or Pattern matching. Noteworthy extra step specific to this CL: * specifically for vendor.mime.types (but not android.mime.types), add a prefix '?' to MIME types and extensions at build time, rather than at runtime. Note that after this CL, DefaultMimeMapFactory can now *only* parse minimized *mime.types files, not the original (non-minimized) file format. Test: Checked that the mapping didn't change by checking that a device flashed after this CL passes CtsMimeMapTestCases built before this CL. Test: Ran "make mimemap-res.jar" and manually verified that files in the resulting jar look as expected. Test: Locally added some extra mappings to vendor.mime.types, some of them with a leading '?', and verified that they all show up with exactly one leading '?' for the MIME type and each extension within mimemap-res.jar. Bug: 142267887 Change-Id: Idf1ef945a4ac225476f2036fbc8bb35eb9c090af --- mime/Android.bp | 33 +++++++----- mime/java-res/vendor.mime.types | 3 ++ .../content/type/DefaultMimeMapFactory.java | 63 ++++++++++------------ 3 files changed, 52 insertions(+), 47 deletions(-) (limited to 'mime') diff --git a/mime/Android.bp b/mime/Android.bp index 8b2b05958b6f..23a8fbf5059c 100644 --- a/mime/Android.bp +++ b/mime/Android.bp @@ -60,7 +60,7 @@ java_genrule { tools: [ "soong_zip", ], - srcs: [":mime.types"], + srcs: [":mime.types.minimized"], out: ["mimemap-res.jar"], cmd: "mkdir $(genDir)/res/ && cp $(in) $(genDir)/res/ && $(location soong_zip) -C $(genDir) -o $(out) -D $(genDir)/res/", } @@ -73,42 +73,49 @@ java_genrule { tools: [ "soong_zip", ], - srcs: [":mime.types"], + srcs: [":mime.types.minimized"], out: ["mimemap-testing-res.jar"], cmd: "mkdir $(genDir)/testres/ && cp $(in) $(genDir)/testres/ && $(location soong_zip) -C $(genDir) -o $(out) -D $(genDir)/testres/", } -// Combination of all *mime.types resources. +// Combination of all *mime.types.minimized resources. filegroup { - name: "mime.types", + name: "mime.types.minimized", visibility: [ "//visibility:private", ], srcs: [ - ":debian.mime.types", - ":android.mime.types", - ":vendor.mime.types", + ":debian.mime.types.minimized", + ":android.mime.types.minimized", + ":vendor.mime.types.minimized", ], } -filegroup { - name: "android.mime.types", +java_genrule { + name: "android.mime.types.minimized", visibility: [ "//visibility:private", ], - path: "java-res/", + out: ["android.mime.types"], srcs: [ "java-res/android.mime.types", ], + // strip comments normalize whitepace drop empty lines + cmd: "awk '{gsub(/#.*$$/,\"\"); $$1=$$1; print;}' $(in) | grep ' ' > $(out)", } -filegroup { - name: "vendor.mime.types", +// Unlike the other *mime.types files, vendor.mime.types gets '?' prepended to +// every field so that its mappings will never overwrite earlier mappings by +// the other resource files. http://b/141842825 +java_genrule { + name: "vendor.mime.types.minimized", visibility: [ "//visibility:private", ], - path: "java-res/", + out: ["vendor.mime.types"], srcs: [ "java-res/vendor.mime.types", ], + // strip comments normalize whitepace drop empty lines prepend ? to fields that are missing it + cmd: "awk '{gsub(/#.*$$/,\"\"); $$1=$$1; print;}' $(in) | grep ' ' | awk '{for(i=1;i<=NF;i++) { sub(/^\\??/, \"?\", $$i); }; print}' > $(out)", } diff --git a/mime/java-res/vendor.mime.types b/mime/java-res/vendor.mime.types index afb8f9e4ef39..06939c525a85 100644 --- a/mime/java-res/vendor.mime.types +++ b/mime/java-res/vendor.mime.types @@ -39,3 +39,6 @@ # # Add your custom mappings below this line (with no "#" at the start of the line): +test/mimeA extA extB extX +?test/mimeC ?extX ?extY ?extZ +test/mimeB extC ?extD extF diff --git a/mime/java/android/content/type/DefaultMimeMapFactory.java b/mime/java/android/content/type/DefaultMimeMapFactory.java index 03b685df644e..11d20d4d6c80 100644 --- a/mime/java/android/content/type/DefaultMimeMapFactory.java +++ b/mime/java/android/content/type/DefaultMimeMapFactory.java @@ -23,11 +23,9 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Objects; import java.util.function.Function; -import java.util.regex.Pattern; /** * Creates the framework default {@link MimeMap}, a bidirectional mapping @@ -53,8 +51,6 @@ public class DefaultMimeMapFactory { return create(resourceName -> c.getResourceAsStream("/res/" + resourceName)); } - private static final Pattern SPLIT_PATTERN = Pattern.compile("\\s+"); - /** * Creates a {@link MimeMap} instance whose resources are loaded from the * InputStreams looked up in {@code resourceSupplier}. @@ -63,33 +59,43 @@ public class DefaultMimeMapFactory { */ public static MimeMap create(Function resourceSupplier) { MimeMap.Builder builder = MimeMap.builder(); - parseTypes(builder, true, resourceSupplier, "mime.types"); - parseTypes(builder, true, resourceSupplier, "android.mime.types"); - parseTypes(builder, false, resourceSupplier, "vendor.mime.types"); + // The files loaded here must be in minimized format with lines of the + // form "mime/type ext1 ext2 ext3", i.e. no comments, no empty lines, no + // leading/trailing whitespace and with a single space between entries on + // each line. See http://b/142267887 + // + // Note: the order here matters - later entries can overwrite earlier ones + // (except that vendor.mime.types entries are prefixed with '?' which makes + // them never overwrite). + parseTypes(builder, resourceSupplier, "debian.mime.types"); + parseTypes(builder, resourceSupplier, "android.mime.types"); + parseTypes(builder, resourceSupplier, "vendor.mime.types"); return builder.build(); } - private static void parseTypes(MimeMap.Builder builder, boolean allowOverwrite, + private static void parseTypes(MimeMap.Builder builder, Function resourceSupplier, String resourceName) { try (InputStream inputStream = Objects.requireNonNull(resourceSupplier.apply(resourceName)); BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream))) { String line; + List specs = new ArrayList<>(10); // re-use for each line while ((line = reader.readLine()) != null) { - int commentPos = line.indexOf('#'); - if (commentPos >= 0) { - line = line.substring(0, commentPos); - } - line = line.trim(); - if (line.isEmpty()) { - continue; - } - List specs = Arrays.asList(SPLIT_PATTERN.split(line)); - if (!allowOverwrite) { - // Pretend that the mimeType and each file extension listed in the line - // carries a "?" prefix, which means that it can add new mappings but - // not modify existing mappings (putIfAbsent() semantics). - specs = ensurePrefix("?", specs); - } + specs.clear(); + // Lines are of the form "mimeSpec extSpec extSpec[...]" with a single space + // separating them and no leading/trailing spaces and no empty lines. + int startIdx = 0; + do { + int endIdx = line.indexOf(' ', startIdx); + if (endIdx < 0) { + endIdx = line.length(); + } + String spec = line.substring(startIdx, endIdx); + if (spec.isEmpty()) { + throw new IllegalArgumentException("Malformed line: " + line); + } + specs.add(spec); + startIdx = endIdx + 1; // skip over the space + } while (startIdx < line.length()); builder.put(specs.get(0), specs.subList(1, specs.size())); } } catch (IOException | RuntimeException e) { @@ -97,15 +103,4 @@ public class DefaultMimeMapFactory { } } - private static List ensurePrefix(String prefix, List strings) { - List result = new ArrayList<>(strings.size()); - for (String s : strings) { - if (!s.startsWith(prefix)) { - s = prefix + s; - } - result.add(s); - } - return result; - } - } -- cgit v1.2.3