Does a better job of code splitting for static fields. Previously,
there was a bug where a field could get loaded but initialized
to a string literal that had not yet been loaded. This revision
fixes that bug. Additionally, static fields can now be moved out
of the initial download. Further, for fields initialized to string literals,
the string literals are loaded in the fragment the field is first used in,
not necessarily the fragment the enclosing class's initializer
runs in. That way, the strings for different fields can be loaded
in different fragments.
Review by: kprobst
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@4209 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/CodeSplitter.java b/dev/core/src/com/google/gwt/dev/jjs/impl/CodeSplitter.java
index 42afd16..f5014cb 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/CodeSplitter.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/CodeSplitter.java
@@ -36,6 +36,8 @@
import com.google.gwt.dev.js.ast.JsFunction;
import com.google.gwt.dev.js.ast.JsProgram;
import com.google.gwt.dev.js.ast.JsStatement;
+import com.google.gwt.dev.js.ast.JsVars;
+import com.google.gwt.dev.js.ast.JsVars.JsVar;
import com.google.gwt.dev.util.PerfLogger;
import java.util.ArrayList;
@@ -97,6 +99,20 @@
}
}
}
+
+ if (stat instanceof JsVars) {
+ JsVars vars = (JsVars) stat;
+ for (JsVar var : vars) {
+ JField field = map.nameToField(var.getName());
+ if (field != null) {
+ System.out.println(fullNameString(field));
+ }
+ String string = map.stringLiteralForName(var.getName());
+ if (string != null) {
+ System.out.println("STRING " + var.getName());
+ }
+ }
+ }
}
}
}
@@ -211,6 +227,23 @@
new CodeSplitter(logger, jprogram, jsprogram, map).execImpl();
}
+ private static Map<JField, JClassLiteral> buildFieldToClassLiteralMap(
+ JProgram jprogram) {
+ final Map<JField, JClassLiteral> map = new HashMap<JField, JClassLiteral>();
+ class BuildFieldToLiteralVisitor extends JVisitor {
+ @Override
+ public void endVisit(JClassLiteral lit, Context ctx) {
+ map.put(lit.getField(), lit);
+ }
+ }
+ (new BuildFieldToLiteralVisitor()).accept(jprogram);
+ return map;
+ }
+
+ private static String fullNameString(JField field) {
+ return field.getEnclosingType().getName() + "." + field.getName();
+ }
+
private static String fullNameString(JMethod method) {
return method.getEnclosingType().getName() + "."
+ JProgram.getJsniSig(method);
@@ -238,8 +271,8 @@
}
private final Map<JField, JClassLiteral> fieldToLiteralOfClass;
-
private final FragmentExtractor fragmentExtractor;
+
/**
* Code that is initially live when the program first downloads.
*/
@@ -262,7 +295,7 @@
numEntries = jprogram.entryMethods.size();
logging = Boolean.getBoolean(PROP_LOG_FRAGMENT_MAP);
- fieldToLiteralOfClass = FragmentExtractor.buildFieldToClassLiteralMap(jprogram);
+ fieldToLiteralOfClass = buildFieldToClassLiteralMap(jprogram);
fragmentExtractor = new FragmentExtractor(jprogram, jsprogram, map);
initiallyLive = new ControlFlowAnalyzer(jprogram);
@@ -397,22 +430,28 @@
}
/**
- * Mostly it is okay to load code before it is needed. However, there are some
- * exceptions, where merely loading a code atom requires that some other atom
- * has also been loaded. To address such situations, move the load-time
- * dependencies to fragment 0, so they are sure to be available.
+ * <p>
+ * Patch up the fragment map to satisfy load-order dependencies, as described
+ * in the comment of {@link LivenessPredicate}. Load-order dependencies can
+ * be violated when an atom is mapped to 0 as a leftover, but it has some
+ * load-order dependency on an atom that was put in an exclusive fragment.
+ * </p>
+ *
+ * <p>
+ * In general, it might be possible to split things better by considering load
+ * order dependencies when building the fragment map. However, fixing them
+ * after the fact makes CodeSplitter simpler. In practice, for programs tried
+ * so far, there are very few load order dependency fixups that actually
+ * happen, so it seems better to keep the compiler simpler.
+ * </p>
*/
private void fixUpLoadOrderDependencies(FragmentMap fragmentMap) {
fixUpLoadOrderDependenciesForMethods(fragmentMap);
fixUpLoadOrderDependenciesForTypes(fragmentMap);
fixUpLoadOrderDependenciesForClassLiterals(fragmentMap);
+ fixUpLoadOrderDependenciesForFieldsInitializedToStrings(fragmentMap);
}
- /**
- * A class literal cannot be loaded until its associate strings are. Make sure
- * that the strings are available for all class literals at the time they are
- * loaded.
- */
private void fixUpLoadOrderDependenciesForClassLiterals(
FragmentMap fragmentMap) {
int numClassLitStrings = 0;
@@ -437,10 +476,31 @@
+ numClassLitStrings);
}
- /**
- * Fixes up the load-order dependencies from instance methods to their
- * enclosing types.
- */
+ private void fixUpLoadOrderDependenciesForFieldsInitializedToStrings(
+ FragmentMap fragmentMap) {
+ int numFixups = 0;
+ int numFieldStrings = 0;
+
+ for (JField field : fragmentMap.fields.keySet()) {
+ if (field.getInitializer() instanceof JStringLiteral) {
+ numFieldStrings++;
+
+ String string = ((JStringLiteral) field.getInitializer()).getValue();
+ int fieldFrag = getOrZero(fragmentMap.fields, field);
+ int stringFrag = getOrZero(fragmentMap.strings, string);
+ if (fieldFrag != stringFrag && stringFrag != 0) {
+ numFixups++;
+ fragmentMap.strings.put(string, 0);
+ }
+ }
+ }
+
+ logger.log(TreeLogger.DEBUG, "Fixed up load-order dependencies by moving "
+ + numFixups
+ + " strings used to initialize fields to fragment 0, out of "
+ + +numFieldStrings);
+ }
+
private void fixUpLoadOrderDependenciesForMethods(FragmentMap fragmentMap) {
int numFixups = 0;
@@ -471,9 +531,6 @@
+ jprogram.getDeclaredTypes().size());
}
- /**
- * Fixes up load order dependencies from types to their supertypes.
- */
private void fixUpLoadOrderDependenciesForTypes(FragmentMap fragmentMap) {
int numFixups = 0;
Queue<JReferenceType> typesToCheck = new ArrayBlockingQueue<JReferenceType>(
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
index 51d0721..a7eee8f 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
@@ -204,7 +204,14 @@
* initializer). Writes don't count, only reads.
*/
if (x.getInitializer() != null) {
- accept(x.getInitializer());
+
+ if (!isStaticFieldInitializedToLiteral(x.getVariableRef().getTarget())) {
+ /*
+ * Don't traverse literal initializers, because those become live when
+ * the variable is accessed, not when its declaration runs.
+ */
+ accept(x.getInitializer());
+ }
}
// If the lhs is a field ref, we have to visit its qualifier.
@@ -415,6 +422,14 @@
return true;
}
+ private boolean isStaticFieldInitializedToLiteral(JVariable var) {
+ if (var instanceof JField) {
+ JField field = (JField) var;
+ return field.isStatic() && field.getLiteralInitializer() != null;
+ }
+ return false;
+ }
+
private boolean isVolatileField(JExpression x) {
if (x instanceof JFieldRef) {
JFieldRef xFieldRef = (JFieldRef) x;
@@ -506,7 +521,17 @@
private void rescue(JVariable var) {
if (var != null) {
- liveFieldsAndMethods.add(var);
+ if (liveFieldsAndMethods.add(var)) {
+ if (isStaticFieldInitializedToLiteral(var)) {
+ /*
+ * Rescue literal initializers when the field is rescued, not when
+ * the static initializer runs. This allows fields initialized to
+ * string literals to only need the string literals when the field
+ * itself becomes live.
+ */
+ accept(((JField) var).getLiteralInitializer());
+ }
+ }
}
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/FragmentExtractor.java b/dev/core/src/com/google/gwt/dev/jjs/impl/FragmentExtractor.java
index a3a75eb..a3c49a4 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/FragmentExtractor.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/FragmentExtractor.java
@@ -16,13 +16,10 @@
package com.google.gwt.dev.jjs.impl;
import com.google.gwt.dev.jjs.SourceInfo;
-import com.google.gwt.dev.jjs.ast.Context;
-import com.google.gwt.dev.jjs.ast.JClassLiteral;
import com.google.gwt.dev.jjs.ast.JField;
import com.google.gwt.dev.jjs.ast.JMethod;
import com.google.gwt.dev.jjs.ast.JProgram;
import com.google.gwt.dev.jjs.ast.JReferenceType;
-import com.google.gwt.dev.jjs.ast.JVisitor;
import com.google.gwt.dev.js.ast.JsBinaryOperation;
import com.google.gwt.dev.js.ast.JsBinaryOperator;
import com.google.gwt.dev.js.ast.JsEmpty;
@@ -39,10 +36,8 @@
import java.util.ArrayList;
import java.util.Collections;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
-import java.util.Map;
import java.util.Set;
/**
@@ -83,7 +78,28 @@
}
/**
+ * <p>
* A predicate on whether statements and variables should be considered live.
+ * </p>
+ *
+ *
+ * <p>
+ * Any supplied predicate must satisfy load-order dependencies. For any atom
+ * considered live, the atoms it depends on at load time should also be live.
+ * The following load-order dependencies exist:
+ * </p>
+ *
+ * <ul>
+ * <li>A class literal depends on the strings contained in its instantiation
+ * instruction.</li>
+ *
+ * <li>Types depend on their supertype.</li>
+ *
+ * <li>Instance methods depend on their enclosing type.</li>
+ *
+ * <li>Static fields that are initialized to strings depend on the string
+ * they are initialized to.</li>
+ * </ul>
*/
public static interface LivenessPredicate {
boolean isLive(JField field);
@@ -143,23 +159,8 @@
}
}
- public static Map<JField, JClassLiteral> buildFieldToClassLiteralMap(
- JProgram jprogram) {
- final Map<JField, JClassLiteral> map = new HashMap<JField, JClassLiteral>();
- class BuildFieldToLiteralVisitor extends JVisitor {
- @Override
- public void endVisit(JClassLiteral lit, Context ctx) {
- map.put(lit.getField(), lit);
- }
- }
- (new BuildFieldToLiteralVisitor()).accept(jprogram);
- return map;
- }
-
private Set<JsName> entryMethodNames;
- private Map<JField, JClassLiteral> fieldToLiteralOfClass;
-
private final JProgram jprogram;
private final JsProgram jsprogram;
@@ -179,7 +180,6 @@
this.jsprogram = jsprogram;
this.map = map;
- fieldToLiteralOfClass = buildFieldToClassLiteralMap(jprogram);
buildEntryMethodSet();
}
@@ -242,8 +242,8 @@
boolean keepIt;
- if (containsInternedLiterals(stat)) {
- stat = removeSomeInternedLiterals((JsVars) stat, livenessPredicate,
+ if (containsRemovableVars(stat)) {
+ stat = removeSomeVars((JsVars) stat, livenessPredicate,
alreadyLoadedPredicate);
keepIt = !(stat instanceof JsEmpty);
} else {
@@ -310,12 +310,12 @@
/**
* Check whether this statement is a <code>JsVars</code> that contains
- * interned literals. If it does, then
- * {@link #removeSomeInternedLiterals(JsVars, LivenessPredicate, LivenessPredicate)}
- * is sensible for this statement and should be used instead of
+ * individual vars that could be removed. If it does, then
+ * {@link #removeSomeVars(JsVars, LivenessPredicate, LivenessPredicate)} is
+ * sensible for this statement and should be used instead of
* {@link #isLive(JsStatement, com.google.gwt.fragserv.FragmentExtractor.LivenessPredicate)}.
*/
- private boolean containsInternedLiterals(JsStatement stat) {
+ private boolean containsRemovableVars(JsStatement stat) {
if (stat instanceof JsVars) {
for (JsVar var : (JsVars) stat) {
String lit = map.stringLiteralForName(var.getName());
@@ -325,8 +325,7 @@
}
JField field = map.nameToField(var.getName());
- if (field != null && fieldToLiteralOfClass.containsKey(field)) {
- // It's an intern variable for a class literal
+ if (field != null) {
return true;
}
}
@@ -387,11 +386,12 @@
/**
* Check whether a variable is needed. If the variable is an intern variable,
- * then return whether the interned value is live. Otherwise, assume the
- * variable is needed and return <code>true</code>.
+ * then return whether the interned value is live. If the variable corresponds
+ * to a Java field, then return whether the Java field is live. Otherwise,
+ * assume the variable is needed and return <code>true</code>.
*
* Whenever this method is updated, also look at
- * {@link #containsInternedLiterals(JsStatement)}.
+ * {@link #containsRemovableVars(JsStatement)}.
*/
private boolean isLive(JsVar var, LivenessPredicate livenessPredicate) {
String lit = map.stringLiteralForName(var.getName());
@@ -401,8 +401,8 @@
}
JField field = map.nameToField(var.getName());
- if (field != null && fieldToLiteralOfClass.containsKey(field)) {
- // It's an intern variable for a class literal
+ if (field != null) {
+ // It's a field
return livenessPredicate.isLive(field);
}
@@ -435,7 +435,7 @@
* <code>currentLivenessPredicate</code> but not by
* <code>alreadyLoadedPredicate</code>.
*/
- private JsStatement removeSomeInternedLiterals(JsVars stat,
+ private JsStatement removeSomeVars(JsVars stat,
LivenessPredicate currentLivenessPredicate,
LivenessPredicate alreadyLoadedPredicate) {
JsVars newVars = new JsVars(stat.getSourceInfo().makeChild(
