From 7f1defafe4155a4e14a9805cb6c14fa406b390f3 Mon Sep 17 00:00:00 2001 From: rosch Date: Tue, 6 Mar 2018 17:58:43 -0800 Subject: [PATCH] Change all instances of encoded.subSequence() to use a simple start index offset, which reduces the Java memory footprint for TrieParser by at at least 130 MBs. This is because the original call to subSequence() would generate a new String on the heap for each call, where as just using an offset avoids the need to generate a new String all together. This memory enhancement was found while investigating flakiness issues for some tests which had the same root cause of: java.lang.OutOfMemoryError: GC overhead limit exceeded, mostly in the TrieParser class. 100-test runs at original code: Memory usage - maximum: 503MiB (527,769,600 bytes) - average: 388MiB (407,225,077 bytes) 100-test runs with my changes: Memory usage - maximum: 316MiB (325,271,552 bytes) - average: 221MiB (239,355,320 bytes) At maximum: 187 MB saved At average: 167 MB saved RELNOTES=Reduced heap memory usage when parsing domain names ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=188111053 --- .../thirdparty/publicsuffix/TrieParser.java | 15 +++++++-------- .../thirdparty/publicsuffix/TrieParser.java | 15 +++++++-------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/android/guava/src/com/google/thirdparty/publicsuffix/TrieParser.java b/android/guava/src/com/google/thirdparty/publicsuffix/TrieParser.java index 28fd61a28d98..9c387ebda117 100755 --- a/android/guava/src/com/google/thirdparty/publicsuffix/TrieParser.java +++ b/android/guava/src/com/google/thirdparty/publicsuffix/TrieParser.java @@ -23,7 +23,6 @@ /** Parser for a map of reversed domain names stored as a serialized radix tree. */ @GwtCompatible final class TrieParser { - private static final Joiner PREFIX_JOINER = Joiner.on(""); /** @@ -35,9 +34,7 @@ static ImmutableMap parseTrie(CharSequence encoded) { int encodedLen = encoded.length(); int idx = 0; while (idx < encodedLen) { - idx += - doParseTrieToBuilder( - Lists.newLinkedList(), encoded.subSequence(idx, encodedLen), builder); + idx += doParseTrieToBuilder(Lists.newLinkedList(), encoded, idx, builder); } return builder.build(); } @@ -48,16 +45,18 @@ static ImmutableMap parseTrie(CharSequence encoded) { * @param stack The prefixes that precede the characters represented by this node. Each entry of * the stack is in reverse order. * @param encoded The serialized trie. + * @param start An index in the encoded serialized trie to begin reading characters from. * @param builder A map builder to which all entries will be added. * @return The number of characters consumed from {@code encoded}. */ private static int doParseTrieToBuilder( List stack, CharSequence encoded, + int start, ImmutableMap.Builder builder) { int encodedLen = encoded.length(); - int idx = 0; + int idx = start; char c = '\0'; // Read all of the characters for this node. @@ -68,7 +67,7 @@ private static int doParseTrieToBuilder( } } - stack.add(0, reverse(encoded.subSequence(0, idx))); + stack.add(0, reverse(encoded.subSequence(start, idx))); if (c == '!' || c == '?' || c == ':' || c == ',') { // '!' represents an interior node that represents a REGISTRY entry in the map. @@ -85,7 +84,7 @@ private static int doParseTrieToBuilder( if (c != '?' && c != ',') { while (idx < encodedLen) { // Read all the children - idx += doParseTrieToBuilder(stack, encoded.subSequence(idx, encodedLen), builder); + idx += doParseTrieToBuilder(stack, encoded, idx, builder); if (encoded.charAt(idx) == '?' || encoded.charAt(idx) == ',') { // An extra '?' or ',' after a child node indicates the end of all children of this node. idx++; @@ -94,7 +93,7 @@ private static int doParseTrieToBuilder( } } stack.remove(0); - return idx; + return idx - start; } private static CharSequence reverse(CharSequence s) { diff --git a/guava/src/com/google/thirdparty/publicsuffix/TrieParser.java b/guava/src/com/google/thirdparty/publicsuffix/TrieParser.java index 28fd61a28d98..9c387ebda117 100644 --- a/guava/src/com/google/thirdparty/publicsuffix/TrieParser.java +++ b/guava/src/com/google/thirdparty/publicsuffix/TrieParser.java @@ -23,7 +23,6 @@ /** Parser for a map of reversed domain names stored as a serialized radix tree. */ @GwtCompatible final class TrieParser { - private static final Joiner PREFIX_JOINER = Joiner.on(""); /** @@ -35,9 +34,7 @@ static ImmutableMap parseTrie(CharSequence encoded) { int encodedLen = encoded.length(); int idx = 0; while (idx < encodedLen) { - idx += - doParseTrieToBuilder( - Lists.newLinkedList(), encoded.subSequence(idx, encodedLen), builder); + idx += doParseTrieToBuilder(Lists.newLinkedList(), encoded, idx, builder); } return builder.build(); } @@ -48,16 +45,18 @@ static ImmutableMap parseTrie(CharSequence encoded) { * @param stack The prefixes that precede the characters represented by this node. Each entry of * the stack is in reverse order. * @param encoded The serialized trie. + * @param start An index in the encoded serialized trie to begin reading characters from. * @param builder A map builder to which all entries will be added. * @return The number of characters consumed from {@code encoded}. */ private static int doParseTrieToBuilder( List stack, CharSequence encoded, + int start, ImmutableMap.Builder builder) { int encodedLen = encoded.length(); - int idx = 0; + int idx = start; char c = '\0'; // Read all of the characters for this node. @@ -68,7 +67,7 @@ private static int doParseTrieToBuilder( } } - stack.add(0, reverse(encoded.subSequence(0, idx))); + stack.add(0, reverse(encoded.subSequence(start, idx))); if (c == '!' || c == '?' || c == ':' || c == ',') { // '!' represents an interior node that represents a REGISTRY entry in the map. @@ -85,7 +84,7 @@ private static int doParseTrieToBuilder( if (c != '?' && c != ',') { while (idx < encodedLen) { // Read all the children - idx += doParseTrieToBuilder(stack, encoded.subSequence(idx, encodedLen), builder); + idx += doParseTrieToBuilder(stack, encoded, idx, builder); if (encoded.charAt(idx) == '?' || encoded.charAt(idx) == ',') { // An extra '?' or ',' after a child node indicates the end of all children of this node. idx++; @@ -94,7 +93,7 @@ private static int doParseTrieToBuilder( } } stack.remove(0); - return idx; + return idx - start; } private static CharSequence reverse(CharSequence s) {