Reduces incremental output size ~23%. Starts obfuscating just function names. As a result function names are smaller and with sourcemaps are still mostly readable. Change-Id: I3481fe937d26af0ced27f5fc59e2c50f00f3e947 Review-Link: https://gwt-review.googlesource.com/#/c/10452/
diff --git a/dev/core/src/com/google/gwt/dev/MinimalRebuildCache.java b/dev/core/src/com/google/gwt/dev/MinimalRebuildCache.java index 124ce48..8770d7d 100644 --- a/dev/core/src/com/google/gwt/dev/MinimalRebuildCache.java +++ b/dev/core/src/com/google/gwt/dev/MinimalRebuildCache.java
@@ -26,7 +26,7 @@ import com.google.gwt.dev.jjs.ast.JTypeOracle; import com.google.gwt.dev.jjs.ast.JTypeOracle.ImmediateTypeRelations; import com.google.gwt.dev.jjs.impl.ResolveRuntimeTypeReferences.IntTypeMapper; -import com.google.gwt.dev.js.JsPersistentPrettyNamer.PersistentPrettyNamerState; +import com.google.gwt.dev.js.JsIncrementalNamer.JsIncrementalNamerState; import com.google.gwt.dev.resource.Resource; import com.google.gwt.dev.util.Name.InternalName; import com.google.gwt.thirdparty.guava.common.annotations.VisibleForTesting; @@ -174,8 +174,8 @@ private final Set<String> modifiedDiskSourcePaths = Sets.newHashSet(); private final Set<String> modifiedResourcePaths = Sets.newHashSet(); private final Multimap<String, String> nestedTypeNamesByUnitTypeName = HashMultimap.create(); - private final PersistentPrettyNamerState persistentPrettyNamerState = - new PersistentPrettyNamerState(); + private final JsIncrementalNamerState jsIncrementalNamerState = + new JsIncrementalNamerState(); private final Set<String> preambleTypeNames = Sets.newHashSet(); private transient ImmutableSet<String> processedStaleTypeNames = ImmutableSet.<String> of(); private final Multimap<String, String> rebinderTypeNamesByReboundTypeName = HashMultimap.create(); @@ -403,7 +403,7 @@ this.lastLinkedJsBytes = that.lastLinkedJsBytes; this.intTypeMapper.copyFrom(that.intTypeMapper); - this.persistentPrettyNamerState.copyFrom(that.persistentPrettyNamerState); + this.jsIncrementalNamerState.copyFrom(that.jsIncrementalNamerState); this.immediateTypeRelations.copyFrom(that.immediateTypeRelations); copyMap(that.compilationUnitTypeNameByNestedTypeName, @@ -470,7 +470,7 @@ && Objects.equal(this.modifiedDiskSourcePaths, that.modifiedDiskSourcePaths) && Objects.equal(this.modifiedResourcePaths, that.modifiedResourcePaths) && Objects.equal(this.nestedTypeNamesByUnitTypeName, that.nestedTypeNamesByUnitTypeName) - && this.persistentPrettyNamerState.hasSameContent(that.persistentPrettyNamerState) + && this.jsIncrementalNamerState.hasSameContent(that.jsIncrementalNamerState) && Objects.equal(this.preambleTypeNames, that.preambleTypeNames) && Objects.equal( this.rebinderTypeNamesByReboundTypeName, that.rebinderTypeNamesByReboundTypeName) && Objects.equal(this.reboundTypeNamesByGeneratedCompilationUnitNames, @@ -515,8 +515,8 @@ return modifiedCompilationUnitNames; } - public PersistentPrettyNamerState getPersistentPrettyNamerState() { - return persistentPrettyNamerState; + public JsIncrementalNamerState getPersistentPrettyNamerState() { + return jsIncrementalNamerState; } public Set<String> getPreambleTypeNames() {
diff --git a/dev/core/src/com/google/gwt/dev/NullRebuildCache.java b/dev/core/src/com/google/gwt/dev/NullRebuildCache.java index 88c66e3..7accb81 100644 --- a/dev/core/src/com/google/gwt/dev/NullRebuildCache.java +++ b/dev/core/src/com/google/gwt/dev/NullRebuildCache.java
@@ -22,7 +22,7 @@ import com.google.gwt.dev.jjs.JsSourceMap; import com.google.gwt.dev.jjs.ast.JTypeOracle; import com.google.gwt.dev.jjs.impl.ResolveRuntimeTypeReferences.IntTypeMapper; -import com.google.gwt.dev.js.JsPersistentPrettyNamer.PersistentPrettyNamerState; +import com.google.gwt.dev.js.JsIncrementalNamer.JsIncrementalNamerState; import java.util.Collection; import java.util.Map; @@ -116,7 +116,7 @@ } @Override - public PersistentPrettyNamerState getPersistentPrettyNamerState() { + public JsIncrementalNamerState getPersistentPrettyNamerState() { throw new UnsupportedOperationException(failMessage); }
diff --git a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java index 3417fbc..b3092ce 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java +++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
@@ -105,6 +105,7 @@ import com.google.gwt.dev.js.FreshNameGenerator; import com.google.gwt.dev.js.JsBreakUpLargeVarStatements; import com.google.gwt.dev.js.JsDuplicateFunctionRemover; +import com.google.gwt.dev.js.JsIncrementalNamer; import com.google.gwt.dev.js.JsInliner; import com.google.gwt.dev.js.JsLiteralInterner; import com.google.gwt.dev.js.JsNamer.IllegalNameException; @@ -112,7 +113,6 @@ import com.google.gwt.dev.js.JsNamespaceOption; import com.google.gwt.dev.js.JsNormalizer; import com.google.gwt.dev.js.JsObfuscateNamer; -import com.google.gwt.dev.js.JsPersistentPrettyNamer; import com.google.gwt.dev.js.JsPrettyNamer; import com.google.gwt.dev.js.JsReportGenerationVisitor; import com.google.gwt.dev.js.JsStackEmulator; @@ -334,7 +334,7 @@ splitJsIntoFragments(props, permutationId, jjsmap); // TODO(stalcup): move to optimize. - Map<JsName, JsLiteral> internedLiteralByVariableName = renameJsSymbols(props); + Map<JsName, JsLiteral> internedLiteralByVariableName = renameJsSymbols(props, jjsmap); // TODO(stalcup): move to normalization JsBreakUpLargeVarStatements.exec(jsProgram, props.getConfigProps()); @@ -825,7 +825,7 @@ } } - private Map<JsName, JsLiteral> renameJsSymbols(PermProps props) + private Map<JsName, JsLiteral> renameJsSymbols(PermProps props, JavaToJavaScriptMap jjsmap) throws UnableToCompleteException { Map<JsName, JsLiteral> internedLiteralByVariableName; try { @@ -834,7 +834,7 @@ internedLiteralByVariableName = runObfuscateNamer(props); break; case PRETTY: - internedLiteralByVariableName = runPrettyNamer(props.getConfigProps()); + internedLiteralByVariableName = runPrettyNamer(props.getConfigProps(), jjsmap); break; case DETAILED: internedLiteralByVariableName = runDetailedNamer(props.getConfigProps()); @@ -864,10 +864,11 @@ return internedLiteralByVariableName; } - private Map<JsName, JsLiteral> runPrettyNamer(ConfigProps config) throws IllegalNameException { + private Map<JsName, JsLiteral> runPrettyNamer(ConfigProps config, JavaToJavaScriptMap jjsmap) + throws IllegalNameException { if (compilerContext.getOptions().isIncrementalCompileEnabled()) { - JsPersistentPrettyNamer.exec(jsProgram, config, - compilerContext.getMinimalRebuildCache().getPersistentPrettyNamerState()); + JsIncrementalNamer.exec(jsProgram, config, + compilerContext.getMinimalRebuildCache().getPersistentPrettyNamerState(), jjsmap); return null; }
diff --git a/dev/core/src/com/google/gwt/dev/js/JsIncrementalNamer.java b/dev/core/src/com/google/gwt/dev/js/JsIncrementalNamer.java new file mode 100644 index 0000000..950ce38 --- /dev/null +++ b/dev/core/src/com/google/gwt/dev/js/JsIncrementalNamer.java
@@ -0,0 +1,164 @@ +/* + * Copyright 2014 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package com.google.gwt.dev.js; + +import com.google.gwt.dev.cfg.ConfigProps; +import com.google.gwt.dev.jjs.impl.JavaToJavaScriptMap; +import com.google.gwt.dev.js.ast.JsName; +import com.google.gwt.dev.js.ast.JsProgram; +import com.google.gwt.dev.js.ast.JsScope; +import com.google.gwt.thirdparty.guava.common.annotations.VisibleForTesting; +import com.google.gwt.thirdparty.guava.common.base.Objects; +import com.google.gwt.thirdparty.guava.common.collect.HashMultiset; +import com.google.gwt.thirdparty.guava.common.collect.Maps; +import com.google.gwt.thirdparty.guava.common.collect.Multiset; +import com.google.gwt.thirdparty.guava.common.collect.Sets; + +import java.io.Serializable; +import java.util.Map; +import java.util.Set; + +/** + * A namer that creates short but readable identifiers wherever possible. The old ident -> + * new ident mappings are recorded and can be persisted across multiple compiles. + */ +public class JsIncrementalNamer extends JsNamer { + + /** + * Encapsulates the complete state of this namer so that state can be persisted and reused. + */ + public static class JsIncrementalNamerState implements Serializable { + + private int nextObfuscatedId = -1; + private Map<String, String> renamedIdentByOriginalIdent = Maps.newHashMap(); + private Multiset<String> shortIdentCollisionCounts = HashMultiset.create(); + private Set<String> usedIdents = Sets.newHashSet(); + + public void copyFrom(JsIncrementalNamerState that) { + this.shortIdentCollisionCounts.clear(); + this.renamedIdentByOriginalIdent.clear(); + this.usedIdents.clear(); + + this.shortIdentCollisionCounts.addAll(that.shortIdentCollisionCounts); + this.renamedIdentByOriginalIdent.putAll(that.renamedIdentByOriginalIdent); + this.usedIdents.addAll(that.usedIdents); + this.nextObfuscatedId = that.nextObfuscatedId; + } + + @VisibleForTesting + public boolean hasSameContent(JsIncrementalNamerState that) { + return Objects.equal(this.shortIdentCollisionCounts, that.shortIdentCollisionCounts) + && Objects.equal(this.renamedIdentByOriginalIdent, that.renamedIdentByOriginalIdent) + && Objects.equal(this.usedIdents, that.usedIdents) + && Objects.equal(this.nextObfuscatedId, that.nextObfuscatedId); + } + } + + @VisibleForTesting + public static final String RESERVED_IDENT_SUFFIX = "_g$"; + + public static void exec(JsProgram program, ConfigProps config, JsIncrementalNamerState state, + JavaToJavaScriptMap jjsmap) throws IllegalNameException { + new JsIncrementalNamer(program, config, state, jjsmap).execImpl(); + } + + private final JavaToJavaScriptMap jjsmap; + + private final JsIncrementalNamerState state; + + public JsIncrementalNamer(JsProgram program, ConfigProps config, + JsIncrementalNamerState state, JavaToJavaScriptMap jjsmap) { + super(program, config); + this.state = state; + this.jjsmap = jjsmap; + } + + @Override + protected void reset() { + // Nothing to do. + } + + @Override + protected void visit(JsScope scope) throws IllegalNameException { + // Visit children. + for (JsScope child : scope.getChildren()) { + visit(child); + } + + // Visit all my idents. + for (JsName name : scope.getAllNames()) { + if (!name.isObfuscatable()) { + // Unobfuscatable names become themselves. + String ident = name.getIdent(); + if (ident.endsWith(RESERVED_IDENT_SUFFIX)) { + throw new IllegalNameException("Identifier " + ident + " ends with " + + RESERVED_IDENT_SUFFIX + + ". This is not allowed since that suffix is used to separate obfuscatable and " + + "nonobfuscatable names in per-file compiles."); + } + name.setShortIdent(ident); + continue; + } + + name.setShortIdent(getOrCreateIdent(name)); + } + } + + private String getOrCreateIdent(JsName name) { + String originalIdent = name.getIdent(); + String shortIdent = name.getShortIdent(); + + // Reuse previous names. + if (state.renamedIdentByOriginalIdent.containsKey(originalIdent)) { + return state.renamedIdentByOriginalIdent.get(originalIdent); + } + + // If the name is for a method. + if (jjsmap != null && jjsmap.nameToMethod(name) != null) { + // Come up with an obfuscated name (since OptionDisplayName will show it properly) and cache + // it for reuse. + String obfuscatedIdent = makeObfuscatedIdent(); + state.usedIdents.add(obfuscatedIdent); + state.renamedIdentByOriginalIdent.put(originalIdent, obfuscatedIdent); + return obfuscatedIdent; + } + + // Otherwise come up with a new pretty name and cache it for reuse. + String prettyIdent = makePrettyName(shortIdent); + state.usedIdents.add(prettyIdent); + state.renamedIdentByOriginalIdent.put(originalIdent, prettyIdent); + return prettyIdent; + } + + private String makeObfuscatedIdent() { + while (true) { + String obfuscatedIdent = + JsObfuscateNamer.makeObfuscatedIdent(++state.nextObfuscatedId) + RESERVED_IDENT_SUFFIX; + if (reserved.isAvailable(obfuscatedIdent) && !state.usedIdents.contains(obfuscatedIdent)) { + return obfuscatedIdent; + } + } + } + + private String makePrettyName(String shortIdent) { + while (true) { + String prettyIdent = shortIdent + "_" + state.shortIdentCollisionCounts.count(shortIdent) + + RESERVED_IDENT_SUFFIX; + state.shortIdentCollisionCounts.add(shortIdent); + if (reserved.isAvailable(prettyIdent) && !state.usedIdents.contains(prettyIdent)) { + return prettyIdent; + } + } + } +}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsObfuscateNamer.java b/dev/core/src/com/google/gwt/dev/js/JsObfuscateNamer.java index bbe26ca..a84dc4e 100644 --- a/dev/core/src/com/google/gwt/dev/js/JsObfuscateNamer.java +++ b/dev/core/src/com/google/gwt/dev/js/JsObfuscateNamer.java
@@ -74,10 +74,6 @@ * running the global renaming again. */ private int maxId = -1; - /** - * A temp buffer big enough to hold at least 32 bits worth of base-64 chars. - */ - private final char[] sIdentBuf = new char[6]; public JsObfuscateNamer(JsProgram program, ConfigProps config) { super(program, config); @@ -146,7 +142,9 @@ return (scope.findExistingUnobfuscatableName(newIdent) == null); } - private String makeObfuscatedIdent(int id) { + public static String makeObfuscatedIdent(int id) { + char[] sIdentBuf = new char[6]; + // Use base-54 for the first character of the identifier, // so that we don't use any numbers (which are illegal at // the beginning of an identifier).
diff --git a/dev/core/src/com/google/gwt/dev/js/JsPersistentPrettyNamer.java b/dev/core/src/com/google/gwt/dev/js/JsPersistentPrettyNamer.java deleted file mode 100644 index fb27f6b..0000000 --- a/dev/core/src/com/google/gwt/dev/js/JsPersistentPrettyNamer.java +++ /dev/null
@@ -1,137 +0,0 @@ -/* - * Copyright 2014 Google Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except - * in compliance with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License - * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express - * or implied. See the License for the specific language governing permissions and limitations under - * the License. - */ -package com.google.gwt.dev.js; - -import com.google.gwt.dev.cfg.ConfigProps; -import com.google.gwt.dev.js.ast.JsName; -import com.google.gwt.dev.js.ast.JsProgram; -import com.google.gwt.dev.js.ast.JsScope; -import com.google.gwt.thirdparty.guava.common.annotations.VisibleForTesting; -import com.google.gwt.thirdparty.guava.common.base.Objects; -import com.google.gwt.thirdparty.guava.common.collect.HashMultiset; -import com.google.gwt.thirdparty.guava.common.collect.Maps; -import com.google.gwt.thirdparty.guava.common.collect.Multiset; -import com.google.gwt.thirdparty.guava.common.collect.Sets; - -import java.io.Serializable; -import java.util.Map; -import java.util.Set; - -/** - * A namer that keeps the short ("pretty") identifier wherever possible. The original ident -> - * pretty ident mappings are recorded and can be persisted across multiple compiles. - */ -public class JsPersistentPrettyNamer extends JsNamer { - - /** - * Encapsulates the complete state of this namer so that state can be persisted and reused. - */ - public static class PersistentPrettyNamerState implements Serializable { - - private Multiset<String> shortIdentCollisionCounts = HashMultiset.create(); - private Map<String, String> prettyIdentByOriginalIdent = Maps.newHashMap(); - private Set<String> usedPrettyIdents = Sets.newHashSet(); - - public void copyFrom(PersistentPrettyNamerState that) { - this.shortIdentCollisionCounts.clear(); - this.prettyIdentByOriginalIdent.clear(); - this.usedPrettyIdents.clear(); - - this.shortIdentCollisionCounts.addAll(that.shortIdentCollisionCounts); - this.prettyIdentByOriginalIdent.putAll(that.prettyIdentByOriginalIdent); - this.usedPrettyIdents.addAll(that.usedPrettyIdents); - } - - @VisibleForTesting - public boolean hasSameContent(PersistentPrettyNamerState that) { - return Objects.equal(this.shortIdentCollisionCounts, that.shortIdentCollisionCounts) - && Objects.equal(this.prettyIdentByOriginalIdent, that.prettyIdentByOriginalIdent) - && Objects.equal(this.usedPrettyIdents, that.usedPrettyIdents); - } - } - - @VisibleForTesting - public static final String RESERVED_IDENT_SUFFIX = "_g$"; - - public static void exec(JsProgram program, ConfigProps config, PersistentPrettyNamerState state) - throws IllegalNameException { - new JsPersistentPrettyNamer(program, config, state).execImpl(); - } - - private final PersistentPrettyNamerState state; - - public JsPersistentPrettyNamer(JsProgram program, ConfigProps config, - PersistentPrettyNamerState state) { - super(program, config); - this.state = state; - } - - @Override - protected void reset() { - // Nothing to do. - } - - @Override - protected void visit(JsScope scope) throws IllegalNameException { - // Visit children. - for (JsScope child : scope.getChildren()) { - visit(child); - } - - // Visit all my idents. - for (JsName name : scope.getAllNames()) { - if (!name.isObfuscatable()) { - // Unobfuscatable names become themselves. - String prettyIdent = name.getIdent(); - if (prettyIdent.endsWith(RESERVED_IDENT_SUFFIX)) { - throw new IllegalNameException("Identifier " + prettyIdent + " ends with " - + RESERVED_IDENT_SUFFIX - + ". This is not allowed since that suffix is used to separate obfuscatable and " - + "nonobfuscatable names in per-file compiles."); - } - name.setShortIdent(prettyIdent); - continue; - } - - name.setShortIdent(getOrCreatePrettyIdent(name)); - } - } - - private String getOrCreatePrettyIdent(JsName name) { - String originalIdent = name.getIdent(); - String shortIdent = name.getShortIdent(); - - // Reuse previous names. - if (state.prettyIdentByOriginalIdent.containsKey(originalIdent)) { - return state.prettyIdentByOriginalIdent.get(originalIdent); - } - - // Otherwise come up with a new pretty name and cache it for reuse. - String prettyIdent = makePrettyName(shortIdent); - state.usedPrettyIdents.add(prettyIdent); - state.prettyIdentByOriginalIdent.put(originalIdent, prettyIdent); - return prettyIdent; - } - - private String makePrettyName(String shortIdent) { - while (true) { - String prettyIdent = shortIdent + "_" + state.shortIdentCollisionCounts.count(shortIdent) - + RESERVED_IDENT_SUFFIX; - state.shortIdentCollisionCounts.add(shortIdent); - if (reserved.isAvailable(prettyIdent) && !state.usedPrettyIdents.contains(prettyIdent)) { - return prettyIdent; - } - } - } -}
diff --git a/dev/core/test/com/google/gwt/dev/js/JsNamerTest.java b/dev/core/test/com/google/gwt/dev/js/JsNamerTest.java index 167dd7d..0ef7a30 100644 --- a/dev/core/test/com/google/gwt/dev/js/JsNamerTest.java +++ b/dev/core/test/com/google/gwt/dev/js/JsNamerTest.java
@@ -18,8 +18,8 @@ import com.google.gwt.dev.cfg.ConfigProps; import com.google.gwt.dev.jjs.JsOutputOption; import com.google.gwt.dev.jjs.SourceOrigin; +import com.google.gwt.dev.js.JsIncrementalNamer.JsIncrementalNamerState; import com.google.gwt.dev.js.JsNamer.IllegalNameException; -import com.google.gwt.dev.js.JsPersistentPrettyNamer.PersistentPrettyNamerState; import com.google.gwt.dev.js.ast.JsBlock; import com.google.gwt.dev.js.ast.JsExprStmt; import com.google.gwt.dev.js.ast.JsFunction; @@ -48,7 +48,7 @@ public class JsNamerTest extends TestCase { private BlacklistProps props; - private PersistentPrettyNamerState persistentPrettyNamerState; + private JsIncrementalNamerState jsIncrementalNamerState; @Override protected void setUp() throws Exception { @@ -56,7 +56,7 @@ props = new BlacklistProps(); props.blacklist = Arrays.asList("foo, bar", "baz"); props.blacklistSuffixes = Arrays.asList("logger"); - persistentPrettyNamerState = new PersistentPrettyNamerState(); + jsIncrementalNamerState = new JsIncrementalNamerState(); } public void testBannedIdent() throws Exception { @@ -143,13 +143,13 @@ // Renaming with the reserved suffix is rejected. try { - String functionName = "foo" + JsPersistentPrettyNamer.RESERVED_IDENT_SUFFIX; + String functionName = "foo" + JsIncrementalNamer.RESERVED_IDENT_SUFFIX; JsProgram jsProgram = parseJs( "function " + functionName + "() { return 42; }"); jsProgram.getScope().findExistingName(functionName).setObfuscatable(false); rename(jsProgram, JsOutputOption.PRETTY, true); fail("Naming an unobfuscatable identifier containing the reserved suffix should have " - + "thrown an exception in JsPersistentPrettyNamer."); + + "thrown an exception in JsIncrementalNamer."); } catch (IllegalNameException e) { // Expected path. } @@ -174,7 +174,7 @@ switch (outputOption) { case PRETTY: if (persistent) { - JsPersistentPrettyNamer.exec(program, config, persistentPrettyNamerState); + JsIncrementalNamer.exec(program, config, jsIncrementalNamerState, null); } else { JsPrettyNamer.exec(program, config); }