From 5fb2076e2fe2f212b4f01631fbd478064be518c9 Mon Sep 17 00:00:00 2001 From: chrisn Date: Wed, 7 Jun 2017 11:24:43 -0700 Subject: [PATCH] Change RandomNameGenerator to use Hasher API correctly. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=158293027 --- .../jscomp/RandomNameGenerator.java | 116 +++++++----------- .../jscomp/RandomNameGeneratorTest.java | 20 +-- 2 files changed, 53 insertions(+), 83 deletions(-) diff --git a/src/com/google/javascript/jscomp/RandomNameGenerator.java b/src/com/google/javascript/jscomp/RandomNameGenerator.java index c611f3fd86d..7607596aaca 100644 --- a/src/com/google/javascript/jscomp/RandomNameGenerator.java +++ b/src/com/google/javascript/jscomp/RandomNameGenerator.java @@ -18,15 +18,15 @@ import com.google.common.annotations.GwtIncompatible; import com.google.common.base.Joiner; -import com.google.common.collect.Lists; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.common.hash.Hasher; import com.google.common.hash.Hashing; import com.google.common.primitives.Chars; import com.google.javascript.rhino.TokenStream; +import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.List; import java.util.Random; import java.util.Set; @@ -69,22 +69,22 @@ public final class RandomNameGenerator implements NameGenerator { /** Generate random names with this first character. */ - static final char[] FIRST_CHAR = DefaultNameGenerator.FIRST_CHAR; + static final ImmutableSet FIRST_CHAR = asSet(DefaultNameGenerator.FIRST_CHAR); /** These appear after after the first character */ - static final char[] NONFIRST_CHAR = DefaultNameGenerator.NONFIRST_CHAR; + static final ImmutableSet NONFIRST_CHAR = asSet(DefaultNameGenerator.NONFIRST_CHAR); /** The possible first characters, after reserved characters are removed */ - private LinkedHashSet firstChars; + private ImmutableSet firstChars; /** Possible non-first characters, after reserved characters are removed */ - private LinkedHashSet nonFirstChars; + private ImmutableSet nonFirstChars; /** Source of randomness */ private final Random random; /** List of reserved names; these are not returned by generateNextName */ - private Set reservedNames; + private ImmutableSet reservedNames; /** Prefix added to all generated names */ private String prefix; @@ -98,14 +98,13 @@ public final class RandomNameGenerator implements NameGenerator { private static final int NUM_SHUFFLES = 16; /** Randomly-shuffled version of firstChars */ - private List shuffledFirst; - /** Randomly-shuffled versions of nonFirstChard (there are NUM_SUFFLES of - * them) */ - private List> shuffledNonFirst; + private String shuffledFirst; + /** Randomly-shuffled versions of nonFirstChars (there are NUM_SHUFFLES of them) */ + private ImmutableList shuffledNonFirst; public RandomNameGenerator(Random random) { this.random = random; - reset(new HashSet(), "", null); + reset(ImmutableSet.of(), "", null); } RandomNameGenerator( @@ -155,16 +154,17 @@ public void reset( String prefix, @Nullable char[] reservedFirstCharacters, @Nullable char[] reservedNonFirstCharacters) { - this.reservedNames = reservedNames; + this.reservedNames = ImmutableSet.copyOf(reservedNames); this.prefix = prefix; nameCount = 0; // Build the character arrays to use - this.firstChars = reserveCharacters(FIRST_CHAR, reservedFirstCharacters); - this.nonFirstChars = reserveCharacters(NONFIRST_CHAR, reservedNonFirstCharacters); + firstChars = Sets.difference(FIRST_CHAR, asSet(reservedFirstCharacters)).immutableCopy(); + nonFirstChars = + Sets.difference(NONFIRST_CHAR, asSet(reservedNonFirstCharacters)).immutableCopy(); checkPrefix(prefix); - shuffleAlphabets(random); + shuffleAlphabets(); } @Override @@ -176,27 +176,8 @@ public NameGenerator clone( reservedNames, prefix, reservedCharacters, random); } - /** - * Provides the array of available characters based on the specified arrays. - * Also nicely converts to LinkedHashSet which is useful later. - * - * @param chars The list of characters that are legal - * @param reservedCharacters The characters that should not be used - * @return An array of characters to use; order from {@code chars} is - * preserved - */ - private LinkedHashSet reserveCharacters( - char[] chars, char[] reservedCharacters) { - if (reservedCharacters == null) { - reservedCharacters = new char[0]; - } - - // A LinkedHashSet has a defined iteration ordering, which is that of - // insertion. - LinkedHashSet result = Sets.newLinkedHashSet( - Chars.asList(chars)); - result.removeAll(Chars.asList(reservedCharacters)); - return result; + private static ImmutableSet asSet(@Nullable char[] chars) { + return chars == null ? ImmutableSet.of() : ImmutableSet.copyOf(Chars.asList(chars)); } /** @@ -220,35 +201,20 @@ private void checkPrefix(String prefix) { } } - private List shuffleAndCopyAlphabet( - Iterable input, Random random) { - List shuffled = Lists.newArrayList(input); + private static String shuffleAndCopyAlphabet(Set input, Random random) { + List shuffled = new ArrayList<>(input); Collections.shuffle(shuffled, random); - return shuffled; + return new String(Chars.toArray(shuffled)); } - /** - * Generates random shuffles of the alphabets. - */ - private void shuffleAlphabets(Random random) { + /** Generates random shuffles of the alphabets. */ + private void shuffleAlphabets() { shuffledFirst = shuffleAndCopyAlphabet(firstChars, random); - shuffledNonFirst = Lists.newArrayList(); + ImmutableList.Builder builder = ImmutableList.builder(); for (int i = 0; i < NUM_SHUFFLES; ++i) { - shuffledNonFirst.add(shuffleAndCopyAlphabet(nonFirstChars, random)); - } - } - - /** - * Gets the alphabet to use for a character at position {@code position} - * (0-based) given a previous history for that name stored in {@code past} - */ - private List getAlphabet(int position, Hasher past) { - if (position == 0) { - return shuffledFirst; - } else { - int alphabetIdx = (past.hash().asInt() & 0x7fffffff) % NUM_SHUFFLES; - return shuffledNonFirst.get(alphabetIdx); + builder.add(shuffleAndCopyAlphabet(nonFirstChars, random)); } + shuffledNonFirst = builder.build(); } /** @@ -274,24 +240,29 @@ private int getNameLength(int position, int nameIdx) { * is supposed to go at position {@code position} in the final name */ private String generateSuffix(int position, int nameIdx) { - String name = ""; + StringBuilder name = new StringBuilder(); int length = getNameLength(position, nameIdx); - Hasher hasher = Hashing.murmur3_128().newHasher(); - hasher.putInt(length); nameIdx++; do { nameIdx--; - List alphabet = getAlphabet(position, hasher); - int alphabetSize = alphabet.size(); - - Character character = alphabet.get(nameIdx % alphabetSize); - name += character; - hasher.putChar(character); + String alphabet; + if (position == 0) { + alphabet = shuffledFirst; + } else { + Hasher hasher = Hashing.murmur3_128().newHasher(); + hasher.putInt(length); + hasher.putUnencodedChars(name); + int alphabetIdx = (hasher.hash().asInt() & 0x7fffffff) % NUM_SHUFFLES; + alphabet = shuffledNonFirst.get(alphabetIdx); + } + int alphabetSize = alphabet.length(); + char character = alphabet.charAt(nameIdx % alphabetSize); + name.append(character); nameIdx /= alphabetSize; position++; } while (nameIdx > 0); - return name; + return name.toString(); } /** @@ -303,8 +274,7 @@ private String generateSuffix(int position, int nameIdx) { @Override public String generateNextName() { while (true) { - String name = prefix; - name += generateSuffix(prefix.length(), nameCount++); + String name = prefix + generateSuffix(prefix.length(), nameCount++); // Make sure it's not a JS keyword or reserved name. if (TokenStream.isKeyword(name) || reservedNames.contains(name)) { diff --git a/test/com/google/javascript/jscomp/RandomNameGeneratorTest.java b/test/com/google/javascript/jscomp/RandomNameGeneratorTest.java index 86d54de453d..d390f77b30f 100644 --- a/test/com/google/javascript/jscomp/RandomNameGeneratorTest.java +++ b/test/com/google/javascript/jscomp/RandomNameGeneratorTest.java @@ -72,9 +72,9 @@ public static void testGenerate() throws Exception { reservedNames, prefix, null, random); // Generate all 1- and 2-character names. // alphabet length, 1st digit - int len1 = RandomNameGenerator.NONFIRST_CHAR.length; + int len1 = RandomNameGenerator.NONFIRST_CHAR.size(); // alphabet length, 2nd digit - int len2 = RandomNameGenerator.NONFIRST_CHAR.length; + int len2 = RandomNameGenerator.NONFIRST_CHAR.size(); // len1 == len2 because we have a prefix int count = len1 * (1 + len2); String[] result = generate(ng, count); @@ -128,8 +128,8 @@ public static void testFirstCharAlphabet() throws Exception { RandomNameGenerator ng = new RandomNameGenerator( reservedNames, "", null, random); // Generate all 1- and 2-character names. - int len1 = RandomNameGenerator.FIRST_CHAR.length; - int len2 = RandomNameGenerator.NONFIRST_CHAR.length; + int len1 = RandomNameGenerator.FIRST_CHAR.size(); + int len2 = RandomNameGenerator.NONFIRST_CHAR.size(); int count = len1 * (1 + len2); String[] result = generate(ng, count); Set resultSet = Sets.newHashSet(result); @@ -160,8 +160,8 @@ public static void testPrefix() throws Exception { RandomNameGenerator ng = new RandomNameGenerator( reservedNames, prefix, null, random); // Generate all 1- and 2-character names. - int len1 = RandomNameGenerator.FIRST_CHAR.length; - int len2 = RandomNameGenerator.NONFIRST_CHAR.length; + int len1 = RandomNameGenerator.FIRST_CHAR.size(); + int len2 = RandomNameGenerator.NONFIRST_CHAR.size(); int count = len1 * (1 + len2); String[] result = generate(ng, count); @@ -202,8 +202,8 @@ public static void testReservedNames() throws Exception { reservedNames, "", null, random); // Generate all 1- and 2-character names (and a couple 3-character names, // because "x" and "ba", and keywords, shouldn't be used). - int count = RandomNameGenerator.FIRST_CHAR.length - * (RandomNameGenerator.NONFIRST_CHAR.length + 1); + int count = + RandomNameGenerator.FIRST_CHAR.size() * (RandomNameGenerator.NONFIRST_CHAR.size() + 1); Set result = Sets.newHashSet(generate(ng, count)); assertThat(result).doesNotContain("x"); @@ -221,8 +221,8 @@ public static void testReservedCharacters() throws Exception { reservedNames, "", new char[]{'a', 'b'}, random); // Generate all 1- and 2-character names (and also many 3-character names, // because "a" and "b" shouldn't be used). - int count = RandomNameGenerator.FIRST_CHAR.length - * (RandomNameGenerator.NONFIRST_CHAR.length + 1); + int count = + RandomNameGenerator.FIRST_CHAR.size() * (RandomNameGenerator.NONFIRST_CHAR.size() + 1); Set result = Sets.newHashSet(generate(ng, count)); assertThat(result).doesNotContain("a");