Revision: 9694
Author: [email protected]
Date: Tue Feb 8 11:52:43 2011
Log: Only intern strings on non-IE browsers when they occur 2 or more times.
Review at http://gwt-code-reviews.appspot.com/1336802
http://code.google.com/p/google-web-toolkit/source/detail?r=9694
Modified:
/trunk/dev/core/src/com/google/gwt/core/ext/linker/impl/StandardLinkerContext.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
/trunk/dev/core/src/com/google/gwt/dev/js/JsStringInterner.java
=======================================
---
/trunk/dev/core/src/com/google/gwt/core/ext/linker/impl/StandardLinkerContext.java
Wed Feb 2 13:13:58 2011
+++
/trunk/dev/core/src/com/google/gwt/core/ext/linker/impl/StandardLinkerContext.java
Tue Feb 8 11:52:43 2011
@@ -92,7 +92,7 @@
@Override
public boolean visit(JsFunction x, JsContext ctx) {
- didChange |= JsStringInterner.exec(program, x.getBody(),
x.getScope());
+ didChange |= JsStringInterner.exec(program, x.getBody(),
x.getScope(), true);
return false;
}
}
=======================================
---
/trunk/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
Mon Feb 7 13:29:15 2011
+++
/trunk/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
Tue Feb 8 11:52:43 2011
@@ -354,12 +354,29 @@
"dependencies" + permutationId + ".xml.gz",
baos.toByteArray());
}
}
+
+ // detect if browser is ie6 or not known
+ boolean isIE6orUnknown = false;
+ for (PropertyOracle oracle : propertyOracles) {
+ try {
+ SelectionProperty userAgentProperty =
oracle.getSelectionProperty(
+ logger, "user.agent");
+ if ("ie6".equals(userAgentProperty.getCurrentValue())) {
+ isIE6orUnknown = true;
+ break;
+ }
+ } catch (BadPropertyValueException e) {
+ // user agent unknown; play it safe
+ isIE6orUnknown = true;
+ break;
+ }
+ }
// (10.5) Obfuscate
Map<JsName, String> obfuscateMap = Maps.create();
switch (options.getOutput()) {
case OBFUSCATED:
- obfuscateMap = JsStringInterner.exec(jprogram, jsProgram);
+ obfuscateMap = JsStringInterner.exec(jprogram, jsProgram,
isIE6orUnknown);
JsObfuscateNamer.exec(jsProgram);
if (options.isAggressivelyOptimize()) {
if (JsStackEmulator.getStackMode(propertyOracles) ==
JsStackEmulator.StackMode.STRIP) {
@@ -380,7 +397,7 @@
JsPrettyNamer.exec(jsProgram);
break;
case DETAILED:
- obfuscateMap = JsStringInterner.exec(jprogram, jsProgram);
+ obfuscateMap = JsStringInterner.exec(jprogram, jsProgram,
isIE6orUnknown);
JsVerboseNamer.exec(jsProgram);
break;
default:
@@ -398,21 +415,7 @@
// http://code.google.com/p/google-web-toolkit/issues/detail?id=1440
// note, JsIEBlockTextTransformer now handles restructuring top level
// blocks, this class now handles non-top level blocks only.
- boolean splitBlocks = false;
- for (PropertyOracle oracle : propertyOracles) {
- try {
- SelectionProperty userAgentProperty =
oracle.getSelectionProperty(
- logger, "user.agent");
- if ("ie6".equals(userAgentProperty.getCurrentValue())) {
- splitBlocks = true;
- break;
- }
- } catch (BadPropertyValueException e) {
- // user agent unknown; play it safe
- splitBlocks = true;
- break;
- }
- }
+ boolean splitBlocks = isIE6orUnknown;
if (splitBlocks) {
JsIEBlockSizeVisitor.exec(jsProgram);
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/js/JsStringInterner.java Wed
Feb 2 13:13:58 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/js/JsStringInterner.java Tue
Feb 8 08:49:23 2011
@@ -22,6 +22,7 @@
import com.google.gwt.dev.js.ast.JsContext;
import com.google.gwt.dev.js.ast.JsModVisitor;
import com.google.gwt.dev.js.ast.JsName;
+import com.google.gwt.dev.js.ast.JsNode;
import com.google.gwt.dev.js.ast.JsPostfixOperation;
import com.google.gwt.dev.js.ast.JsPrefixOperation;
import com.google.gwt.dev.js.ast.JsProgram;
@@ -31,6 +32,7 @@
import com.google.gwt.dev.js.ast.JsStringLiteral;
import com.google.gwt.dev.js.ast.JsVars;
import com.google.gwt.dev.js.ast.JsVars.JsVar;
+import com.google.gwt.dev.js.ast.JsVisitor;
import java.util.Collection;
import java.util.Comparator;
@@ -44,13 +46,86 @@
import java.util.TreeSet;
/**
- * Interns all String literals in a JsProgram. Each unique String will be
- * assigned to a variable in an appropriate program fragment and the
- * JsStringLiteral will be replaced with a JsNameRef. This optimization is
- * complete in a single pass, although it may be performed multiple times
- * without duplicating the intern pool.
+ * Interns conditionally either all String literals in a JsProgram, or
Strings
+ * which exceed a certain usage count. Each unique String will be assigned
to a
+ * variable in an appropriate program fragment and the JsStringLiteral
will be
+ * replaced with a JsNameRef. This optimization is complete in a single
pass,
+ * although it may be performed multiple times without duplicating the
intern
+ * pool.
*/
public class JsStringInterner {
+
+ /**
+ * Counts occurences of each potentially internable String literal.
+ */
+ private static class OccurenceCounter extends JsVisitor {
+
+ private Map<String, Integer> occurenceMap = new HashMap<String,
Integer>();
+
+ public Map<String, Integer> buildOccurenceMap() {
+ return occurenceMap;
+ }
+
+ /**
+ * Prevents 'fixing' an otherwise illegal operation.
+ */
+ @Override
+ public boolean visit(JsBinaryOperation x, JsContext ctx) {
+ return !x.getOperator().isAssignment()
+ || !(x.getArg1() instanceof JsStringLiteral);
+ }
+
+ /**
+ * Prevents 'fixing' an otherwise illegal operation.
+ */
+ @Override
+ public boolean visit(JsPostfixOperation x, JsContext ctx) {
+ return !(x.getArg() instanceof JsStringLiteral);
+ }
+
+ /**
+ * Prevents 'fixing' an otherwise illegal operation.
+ */
+ @Override
+ public boolean visit(JsPrefixOperation x, JsContext ctx) {
+ return !(x.getArg() instanceof JsStringLiteral);
+ }
+
+ /**
+ * We ignore property initializer labels in object literals, but do
process
+ * the expression. This is because the LHS is always treated as a
string,
+ * and never evaluated as an expression.
+ */
+ @Override
+ public boolean visit(JsPropertyInitializer x, JsContext ctx) {
+ accept(x.getValueExpr());
+ return false;
+ }
+
+ /**
+ * Count occurences of String literal.
+ */
+ @Override
+ public boolean visit(JsStringLiteral x, JsContext ctx) {
+ String literal = x.getValue();
+ Integer occurs = occurenceMap.get(literal);
+ if (occurs == null) {
+ occurs = 0;
+ }
+ occurenceMap.put(literal, ++occurs);
+ return false;
+ }
+
+ /**
+ * This prevents duplicating the intern pool by not traversing JsVar
+ * declarations that look like they were created by the interner.
+ */
+ @Override
+ public boolean visit(JsVar x, JsContext ctx) {
+ return !(x.getName().getIdent().startsWith(PREFIX));
+ }
+ }
+
/**
* Replaces JsStringLiterals with JsNameRefs, creating new JsName
allocations
* on the fly.
@@ -68,6 +143,12 @@
private final SortedMap<JsStringLiteral, Integer> fragmentAssignment =
new TreeMap<JsStringLiteral, Integer>(
LITERAL_COMPARATOR);
+ /**
+ * Minimum number of times a string must occur to be interned.
+ */
+ private static final Integer INTERN_THRESHOLD = Integer.parseInt(
+ System.getProperty("gwt.jjs.stringInternerThreshold", "2"));
+
/**
* A counter used for assigning ids to Strings. Even though it's
unlikely
* that someone would actually have two billion strings in their
@@ -75,6 +156,12 @@
*/
private long lastId = 0;
+ /**
+ * Count of # of occurences of each String literal, or null if
+ * count-sensitive interning is off
+ */
+ private Map<String, Integer> occurenceMap;
+
/**
* Only used to get fragment load order so strings used in multiple
* fragments need only be downloaded once.
@@ -97,11 +184,13 @@
* Constructor.
*
* @param scope specifies the scope in which the interned strings
should be
- * created.
+ * @param occurenceMap
*/
- public StringVisitor(JProgram program, JsScope scope) {
+ public StringVisitor(JProgram program, JsScope scope,
+ Map<String, Integer> occurenceMap) {
this.program = program;
this.scope = scope;
+ this.occurenceMap = occurenceMap;
}
@Override
@@ -150,6 +239,13 @@
*/
@Override
public boolean visit(JsStringLiteral x, JsContext ctx) {
+ if (occurenceMap != null) {
+ Integer occurences = occurenceMap.get(x.getValue());
+ assert occurences != null;
+ if (occurences < INTERN_THRESHOLD) {
+ return false;
+ }
+ }
JsName name = toCreate.get(x);
if (name == null) {
String ident = PREFIX + lastId++;
@@ -208,10 +304,13 @@
* @param jprogram the JProgram that has fragment dependency data for
* <code>program</code>
* @param program the JsProgram
+ * @param alwaysIntern true for browsers like IE which must always
intern literals
* @return a map describing the interning that occurred
*/
- public static Map<JsName, String> exec(JProgram jprogram, JsProgram
program) {
- StringVisitor v = new StringVisitor(jprogram, program.getScope());
+ public static Map<JsName, String> exec(JProgram jprogram, JsProgram
program,
+ boolean alwaysIntern) {
+ StringVisitor v = new StringVisitor(jprogram, program.getScope(),
alwaysIntern ? null :
+ getOccurenceMap(program));
v.accept(program);
Map<Integer, SortedSet<JsStringLiteral>> bins = new HashMap<Integer,
SortedSet<JsStringLiteral>>();
@@ -238,10 +337,13 @@
*
* @param block the block to visit
* @param scope the JsScope in which to reserve the new identifiers
+ * @param alwaysIntern true for browsers like IE which must always
intern literals
* @return <code>true</code> if any changes were made to the block
*/
- public static boolean exec(JsProgram program, JsBlock block, JsScope
scope) {
- StringVisitor v = new StringVisitor(null, scope);
+ public static boolean exec(JsProgram program, JsBlock block, JsScope
scope,
+ boolean alwaysIntern) {
+ StringVisitor v = new StringVisitor(null, scope, alwaysIntern ? null :
+ getOccurenceMap(block));
v.accept(block);
createVars(program, block, v.toCreate.keySet(), v.toCreate);
@@ -269,6 +371,12 @@
block.getStatements().add(0, vars);
}
}
+
+ private static Map<String, Integer> getOccurenceMap(JsNode node) {
+ OccurenceCounter oc = new OccurenceCounter();
+ oc.accept(node);
+ return oc.buildOccurenceMap();
+ }
private static Map<JsName, String> reverse(
SortedMap<JsStringLiteral, JsName> toCreate) {
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
