Skip to content

Commit

Permalink
Fix codesize regression with -XjsInteropMode JS.
Browse files Browse the repository at this point in the history
Erroneusly -XjsInteropMode JS inhibited string interning, now
it correctly inhibited when -XclosureFormattedOutput is selected.

This patch also makes it consistent not to intern anything when
not optimizing.

Change-Id: Ia18d0f496be8439df28f9f8bce90240a0fa3129a
  • Loading branch information
rluble authored and Gerrit Code Review committed Apr 22, 2015
1 parent 6953700 commit c017098
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 25 deletions.
31 changes: 17 additions & 14 deletions dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
Expand Up @@ -526,18 +526,24 @@ private void postNormalizationOptimizeJava() {


private Map<JsName, JsLiteral> runDetailedNamer(ConfigurationProperties config) private Map<JsName, JsLiteral> runDetailedNamer(ConfigurationProperties config)
throws IllegalNameException { throws IllegalNameException {
Map<JsName, JsLiteral> internedTextByVariableName = null; Map<JsName, JsLiteral> internedTextByVariableName =
if (shouldOptimize()) { maybeInternLiterals(JsLiteralInterner.INTERN_ALL);
// Only perform the interning optimization when optimizations are enabled.
internedTextByVariableName =
JsLiteralInterner.exec(jprogram, jsProgram, (byte) (JsLiteralInterner.INTERN_ALL
& (byte) (jprogram.typeOracle.isJsInteropEnabled()
? ~JsLiteralInterner.INTERN_STRINGS : ~0)));
}
JsVerboseNamer.exec(jsProgram, config); JsVerboseNamer.exec(jsProgram, config);
return internedTextByVariableName; return internedTextByVariableName;
} }


private Map<JsName, JsLiteral> maybeInternLiterals(int interningMask) {
if (!shouldOptimize()) {
return null;
}
// Only perform the interning optimization when optimizations are enabled.
if (options.isClosureCompilerFormatEnabled()) {
// Do no intern strings in closure format as it breaks goog.provides, etc.
interningMask &= ~JsLiteralInterner.INTERN_STRINGS;
}
return JsLiteralInterner.exec(jprogram, jsProgram, interningMask);
}

private Pair<SyntheticArtifact, MultipleDependencyGraphRecorder> splitJsIntoFragments( private Pair<SyntheticArtifact, MultipleDependencyGraphRecorder> splitJsIntoFragments(
PermutationProperties properties, int permutationId, JavaToJavaScriptMap jjsmap) { PermutationProperties properties, int permutationId, JavaToJavaScriptMap jjsmap) {
Pair<SyntheticArtifact, MultipleDependencyGraphRecorder> dependenciesAndRecorder; Pair<SyntheticArtifact, MultipleDependencyGraphRecorder> dependenciesAndRecorder;
Expand Down Expand Up @@ -1057,9 +1063,7 @@ private Map<JsName, JsLiteral> renameJsSymbols(PermutationProperties properties,
private Map<JsName, JsLiteral> runObfuscateNamer(PermutationProperties properties) private Map<JsName, JsLiteral> runObfuscateNamer(PermutationProperties properties)
throws IllegalNameException { throws IllegalNameException {
Map<JsName, JsLiteral> internedLiteralByVariableName = Map<JsName, JsLiteral> internedLiteralByVariableName =
JsLiteralInterner.exec(jprogram, jsProgram, (byte) (JsLiteralInterner.INTERN_ALL maybeInternLiterals(JsLiteralInterner.INTERN_ALL);
& (byte) (jprogram.typeOracle.isJsInteropEnabled()
? ~JsLiteralInterner.INTERN_STRINGS : ~0)));
FreshNameGenerator freshNameGenerator = JsObfuscateNamer.exec(jsProgram, FreshNameGenerator freshNameGenerator = JsObfuscateNamer.exec(jsProgram,
properties.getConfigurationProperties()); properties.getConfigurationProperties());
if (options.shouldRemoveDuplicateFunctions() if (options.shouldRemoveDuplicateFunctions()
Expand All @@ -1079,9 +1083,8 @@ private Map<JsName, JsLiteral> runPrettyNamer(ConfigurationProperties configurat
} }


// We don't intern strings in pretty mode to improve readability // We don't intern strings in pretty mode to improve readability
Map<JsName, JsLiteral> internedLiteralByVariableName = JsLiteralInterner.exec( Map<JsName, JsLiteral> internedLiteralByVariableName =
jprogram, jsProgram, maybeInternLiterals(JsLiteralInterner.INTERN_ALL & ~JsLiteralInterner.INTERN_STRINGS);
(byte) (JsLiteralInterner.INTERN_ALL & ~JsLiteralInterner.INTERN_STRINGS));


JsPrettyNamer.exec(jsProgram, configurationProperties); JsPrettyNamer.exec(jsProgram, configurationProperties);
return internedLiteralByVariableName; return internedLiteralByVariableName;
Expand Down
18 changes: 9 additions & 9 deletions dev/core/src/com/google/gwt/dev/js/JsLiteralInterner.java
Expand Up @@ -240,7 +240,7 @@ private static class LiteralInterningVisitor extends JsModVisitor {
/** /**
* This is a set of flags indicating what types of literals are to be interned. * This is a set of flags indicating what types of literals are to be interned.
*/ */
private final byte whatToIntern; private final int whatToIntern;


/** /**
* Constructor. * Constructor.
Expand All @@ -252,7 +252,7 @@ private static class LiteralInterningVisitor extends JsModVisitor {
* @param whatToIntern what types of literals are to be interned. * @param whatToIntern what types of literals are to be interned.
*/ */
public LiteralInterningVisitor(JProgram program, JsScope scope, boolean alwaysIntern, public LiteralInterningVisitor(JProgram program, JsScope scope, boolean alwaysIntern,
Multiset<JsLiteral> occurrencesPerLiteral, byte whatToIntern) { Multiset<JsLiteral> occurrencesPerLiteral, int whatToIntern) {


assert alwaysIntern || (occurrencesPerLiteral != null); assert alwaysIntern || (occurrencesPerLiteral != null);


Expand Down Expand Up @@ -442,12 +442,12 @@ public boolean visit(JsVar x, JsContext ctx) {
/** /**
* Flags to control what type of literals to intern. * Flags to control what type of literals to intern.
*/ */
public static final byte INTERN_ARRAY_LITERALS = 0x01; public static final int INTERN_ARRAY_LITERALS = 0x01;
public static final byte INTERN_NUMBERS = 0x02; public static final int INTERN_NUMBERS = 0x02;
public static final byte INTERN_OBJECT_LITERALS = 0x04; public static final int INTERN_OBJECT_LITERALS = 0x04;
public static final byte INTERN_REGEXES = 0x08; public static final int INTERN_REGEXES = 0x08;
public static final byte INTERN_STRINGS = 0x10; public static final int INTERN_STRINGS = 0x10;
public static final byte INTERN_ALL = INTERN_ARRAY_LITERALS | INTERN_NUMBERS | public static final int INTERN_ALL = INTERN_ARRAY_LITERALS | INTERN_NUMBERS |
INTERN_OBJECT_LITERALS | INTERN_REGEXES | INTERN_STRINGS; INTERN_OBJECT_LITERALS | INTERN_REGEXES | INTERN_STRINGS;


private static final String PREFIX = "$intern_"; private static final String PREFIX = "$intern_";
Expand All @@ -464,7 +464,7 @@ public boolean visit(JsVar x, JsContext ctx) {
* @return a map describing the interning that occurred * @return a map describing the interning that occurred
*/ */
public static Map<JsName, JsLiteral> exec(JProgram jprogram, JsProgram program, public static Map<JsName, JsLiteral> exec(JProgram jprogram, JsProgram program,
byte whatToIntern) { int whatToIntern) {
LiteralInterningVisitor v = new LiteralInterningVisitor(jprogram, program.getScope(), false, LiteralInterningVisitor v = new LiteralInterningVisitor(jprogram, program.getScope(), false,
computeOccurrenceCounts(program), whatToIntern); computeOccurrenceCounts(program), whatToIntern);
v.accept(program); v.accept(program);
Expand Down
Expand Up @@ -16,6 +16,7 @@
package com.google.gwt.dev.jjs; package com.google.gwt.dev.jjs;


import com.google.gwt.dev.jjs.test.HasNoSideEffecstTest; import com.google.gwt.dev.jjs.test.HasNoSideEffecstTest;
import com.google.gwt.dev.jjs.test.RunAsyncContentTest;
import com.google.gwt.dev.jjs.test.SpecializationTest; import com.google.gwt.dev.jjs.test.SpecializationTest;
import com.google.gwt.junit.tools.GWTTestSuite; import com.google.gwt.junit.tools.GWTTestSuite;


Expand All @@ -32,6 +33,10 @@ public static Test suite() {
// $JUnit-BEGIN$ // $JUnit-BEGIN$
suite.addTestSuite(SpecializationTest.class); suite.addTestSuite(SpecializationTest.class);
suite.addTestSuite(HasNoSideEffecstTest.class); suite.addTestSuite(HasNoSideEffecstTest.class);
// RunAsyncContentTest relies in string interning for its assertions which is now always off
// in non optimzied compiles.
suite.addTestSuite(RunAsyncContentTest.class);

// $JUnit-END$ // $JUnit-END$


return suite; return suite;
Expand Down
2 changes: 0 additions & 2 deletions user/test/com/google/gwt/dev/jjs/RunAsyncSuite.java
Expand Up @@ -17,7 +17,6 @@


import com.google.gwt.core.client.prefetch.RunAsyncCodeTest; import com.google.gwt.core.client.prefetch.RunAsyncCodeTest;
import com.google.gwt.dev.jjs.test.CodeSplitterCollapsedPropertiesTest; import com.google.gwt.dev.jjs.test.CodeSplitterCollapsedPropertiesTest;
import com.google.gwt.dev.jjs.test.RunAsyncContentTest;
import com.google.gwt.dev.jjs.test.RunAsyncFailureTest; import com.google.gwt.dev.jjs.test.RunAsyncFailureTest;
import com.google.gwt.dev.jjs.test.RunAsyncMetricsIntegrationTest; import com.google.gwt.dev.jjs.test.RunAsyncMetricsIntegrationTest;
import com.google.gwt.dev.jjs.test.RunAsyncTest; import com.google.gwt.dev.jjs.test.RunAsyncTest;
Expand All @@ -38,7 +37,6 @@ public static Test suite() {
suite.addTestSuite(SystemGetPropertyTest.class); suite.addTestSuite(SystemGetPropertyTest.class);
suite.addTestSuite(CodeSplitterCollapsedPropertiesTest.class); suite.addTestSuite(CodeSplitterCollapsedPropertiesTest.class);
suite.addTestSuite(RunAsyncCodeTest.class); suite.addTestSuite(RunAsyncCodeTest.class);
suite.addTestSuite(RunAsyncContentTest.class);
suite.addTestSuite(RunAsyncFailureTest.class); suite.addTestSuite(RunAsyncFailureTest.class);
suite.addTestSuite(RunAsyncMetricsIntegrationTest.class); suite.addTestSuite(RunAsyncMetricsIntegrationTest.class);
suite.addTestSuite(RunAsyncTest.class); suite.addTestSuite(RunAsyncTest.class);
Expand Down

0 comments on commit c017098

Please sign in to comment.