Revision: 10135
Author: jbrosenb...@google.com
Date: Wed May 4 07:26:14 2011
Log: Allows enum ordinalization to proceed for enums with static
methods/fields
-- This offers modest code-size gains, and will open way for further
optimizations
-- Also, fixes a bug in checking for instanceof arguments for
EnumOrdinalizer
-- Fixed some formatting in EnumOrdinalizer
-- Added new tests to EnumOrdinalizerTest
Review at http://gwt-code-reviews.appspot.com/1428808
http://code.google.com/p/google-web-toolkit/source/detail?r=10135
Modified:
/trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JEnumType.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java
/trunk/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java
/trunk/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JEnumType.java Mon Jun
7 12:20:31 2010
+++ /trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JEnumType.java Wed May
4 07:26:14 2011
@@ -29,6 +29,7 @@
*/
private List<JEnumField> enumList = Lists.create();
+ private boolean isOrdinalized = false;
public JEnumType(SourceInfo info, String name, boolean isAbstract) {
super(info, name, isAbstract, false);
@@ -63,4 +64,12 @@
public JEnumType isEnumOrSubclass() {
return this;
}
-}
+
+ public boolean isOrdinalized() {
+ return isOrdinalized;
+ }
+
+ public void setOrdinalized() {
+ isOrdinalized = true;
+ }
+}
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java
Thu Apr 28 08:18:20 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java
Wed May 4 07:26:14 2011
@@ -71,9 +71,13 @@
* This optimizer modifies enum classes to change their field constants to
ints,
* and to remove initialization of those constants in the clinit method. An
* ordinalized enum class will not be removed from the AST by this
optimizer,
- * but as long as all references to it are replaced (which is one of the
- * requirements for ordinalization), then the enum class itself will be
pruned
- * by subsequent optimizer passes.
+ * but as long as all references to it are replaced, then the enum class
itself
+ * will be pruned by subsequent optimizer passes. Some enum classes may
not be
+ * completely removed however. Ordinalization can proceed in cases where
there
+ * are added static fields or methods in the enum class. In such cases a
reduced
+ * version of the original enum class can remain in the AST, containing
only
+ * static fields and methods which aren't part of the enum infrastructure
(in
+ * which case it will no longer behave as an enum class at all).
*
* Regardless of whether an ordinalized enum class ends up being completely
* pruned away, the AST is expected to be in a coherent and usable state
after
@@ -295,13 +299,29 @@
* does this by keeping track of a "black-list" for ordinals which
violate the
* conditions for ordinalization, below.
*
- * An enum cannot be ordinalized, if it: is implicitly upcast. is
implicitly
- * cast to from a nullType. is implicitly cast to or from a
javaScriptObject
- * type. is explicitly cast to another type (or vice-versa). it's class
- * literal is used explicitly. it has an artificial rescue recorded for
it.
- * has any field referenced, except for: one of it's enum constants
- * Enum.ordinal has any method called, except for: ordinal()
Enum.ordinal()
- * Enum() super constructor Enum.createValueOfMap()
+ * An enum cannot be ordinalized, if it:
+ * <ul>
+ * <li>is implicitly upcast.</li>
+ * <li>is implicitly cast to from a nullType.</li>
+ * <li>is implicitly cast to or from a javaScriptObject type.</li>
+ * <li>is explicitly cast to another type (or vice-versa).</li>
+ * <li>is tested in an instanceof expression.</li>
+ * <li>it's class literal is used explicitly.</li>
+ * <li>it has an artificial rescue recorded for it.</li>
+ * <li>has any field referenced, except for:</li>
+ * <ul>
+ * <li>static fields, other than the synthetic $VALUES field.</li>
+ * <li>Enum.ordinal.</li>
+ * </ul>
+ * <li>has any method called, except for:</li>
+ * <ul>
+ * <li>ordinal().</li>
+ * <li>Enum.ordinal().</li>
+ * <li>Enum() super constructor.</li>
+ * <li>Enum.createValueOfMap().</li>
+ * <li>static methods, other than values() or valueOf().</li>
+ * </ul>
+ * </ul>
*
* This visitor extends the ImplicitUpcastAnalyzer, which encapsulates
all the
* conditions where implicit upcasting can occur in an AST. The rest of
the
@@ -394,6 +414,11 @@
JEnumType maybeEnum = x.isEnumOrSubclass();
if (maybeEnum != null) {
enumsVisited.put(program.getClassLiteralName(maybeEnum),
maybeEnum);
+
+ // don't need to re-ordinalize a previously ordinalized enum
+ if (maybeEnum.isOrdinalized()) {
+ addToBlackList(maybeEnum, x.getSourceInfo());
+ }
}
}
@@ -408,44 +433,28 @@
// check any instance field reference other than ordinal
blackListIfEnumExpression(x.getInstance());
} else if (x.getField().isStatic()) {
- // check static field references
-
/*
- * Need to exempt static fieldRefs to the special $VALUES array
that
- * gets generated for all enum classes, if the reference occurs
within
- * the enum class itself (such as happens in the clinit() or
values()
- * method for all enums).
+ * Black list if the $VALUES static field is referenced outside of
an
+ * enum class. This can happen if there's a call to an enum's
values()
+ * method, which then gets inlined.
+ *
+ * TODO (jbrosenberg): Investigate further whether referencing the
+ * $VALUES array (as well as the values() method) should not block
+ * ordinalization. Instead, convert $VALUES to an array of int.
*/
if (x.getField().getName().equals("$VALUES")
- && this.currentMethod.getEnclosingType() ==
x.getField().getEnclosingType()) {
- if (getEnumType(x.getField().getEnclosingType()) != null) {
- return;
- }
- }
-
- /*
- * Need to exempt static fieldRefs for enum constants themselves.
Detect
- * these as final fields, that have the same enum type as their
- * enclosing type.
- */
- if (x.getField().isFinal()
- && (x.getField().getEnclosingType() ==
getEnumType(x.getField().getType()))) {
- return;
- }
-
- /*
- * Check any other refs to static fields of an enum class. This
includes
- * references to $VALUES that might occur outside of the enum class
- * itself. This can occur when a call to the values() method gets
- * inlined, etc. Also check here for any user defined static
fields.
- */
- blackListIfEnum(x.getField().getEnclosingType(),
x.getSourceInfo());
+ && this.currentMethod.getEnclosingType() !=
x.getField().getEnclosingType()) {
+ blackListIfEnum(x.getField().getEnclosingType(),
x.getSourceInfo());
+ }
}
}
@Override
public void endVisit(JInstanceOf x, Context ctx) {
// If any instanceof tests haven't been optimized out, black list.
+ blackListIfEnum(x.getExpr().getType(), x.getSourceInfo());
+ // TODO (jbrosenberg): Investigate further whether ordinalization
can be
+ // allowed in this case.
blackListIfEnum(x.getTestType(), x.getSourceInfo());
}
@@ -461,11 +470,10 @@
if (x.getInstance() != null) {
blackListIfEnumExpression(x.getInstance());
} else if (x.getTarget().isStatic()) {
- /*
- * need to exempt static methodCalls for an enum class if it occurs
- * within the enum class itself (such as in $clinit() or values())
- */
- if (this.currentMethod.getEnclosingType() !=
x.getTarget().getEnclosingType()) {
+ // black-list static method calls on an enum class only for
valueOf()
+ // and values()
+ String methodName = x.getTarget().getName();
+ if (methodName.equals("valueOf") || methodName.equals("values")) {
blackListIfEnum(x.getTarget().getEnclosingType(),
x.getSourceInfo());
}
}
@@ -888,17 +896,14 @@
ordinalAnalyzer.accept(program);
ordinalAnalyzer.afterVisitor();
- if (tracker != null) {
- for (JEnumType type : enumsVisited.values()) {
- tracker.addVisited(type.getName());
- if (!ordinalizationBlackList.contains(type)) {
- tracker.addOrdinalized(type.getName());
- }
- }
- }
-
// Bail if we don't need to do any ordinalization
if (enumsVisited.size() == ordinalizationBlackList.size()) {
+ // Update tracker stats
+ if (tracker != null) {
+ for (JEnumType type : enumsVisited.values()) {
+ tracker.addVisited(type.getName());
+ }
+ }
return stats;
}
@@ -920,6 +925,19 @@
if (tracker != null) {
tracker.maybeDumpAST(program, 2);
}
+
+ // Update enums ordinalized, and tracker stats
+ for (JEnumType type : enumsVisited.values()) {
+ if (tracker != null) {
+ tracker.addVisited(type.getName());
+ }
+ if (!ordinalizationBlackList.contains(type)) {
+ if (tracker != null) {
+ tracker.addOrdinalized(type.getName());
+ }
+ type.setOrdinalized();
+ }
+ }
return stats;
}
=======================================
--- /trunk/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java Thu
Apr 28 08:18:20 2011
+++ /trunk/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java Wed
May 4 07:26:14 2011
@@ -72,10 +72,11 @@
code.append("package com.google.gwt.lang;\n");
code.append("public final class Cast {\n");
code.append(" public static Object dynamicCast(Object src, int
dstId) { return src;}\n");
- code.append(" public static boolean isNull(Object a) { return
false;}\n");
- code.append(" public static boolean isNotNull(Object a) { return
false;}\n");
- code.append(" public static boolean jsEquals(Object a, Object b) {
return false;}\n");
- code.append(" public static boolean jsNotEquals(Object a, Object b)
{ return false;}\n");
+ code.append(" public static boolean instanceOf(Object src, int
dstId) { return false;}\n");
+ code.append(" public static native boolean isNull(Object a) /*-{
}-*/;\n");
+ code.append(" public static native boolean isNotNull(Object a) /*-{
}-*/;\n");
+ code.append(" public static native boolean jsEquals(Object a,
Object b) /*-{ }-*/;\n");
+ code.append(" public static native boolean jsNotEquals(Object a,
Object b) /*-{ }-*/;\n");
code.append("}\n");
return code;
}
=======================================
---
/trunk/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java
Thu Apr 28 08:18:20 2011
+++
/trunk/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java
Wed May 4 07:26:14 2011
@@ -43,12 +43,11 @@
super.setUp();
EnumOrdinalizer.enableTracker();
- // don't need to runDeadCodeElimination, it's after we're done anyway
- runDeadCodeElimination = false;
-
// defaults, can be overridden by individual test cases
runTypeTightener = false;
runMethodCallTightener = false;
+ runMethodInliner = true;
+ runMakeCallsStatic = true;
}
public void testOrdinalizeBasicAssignment()
@@ -239,6 +238,58 @@
EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
assertTrue(tracker.isOrdinalized("test.EntryPoint$Fruit"));
}
+
+ public void testOrdinalizableStaticFieldRef()
+ throws UnableToCompleteException {
+ EnumOrdinalizer.resetTracker();
+
+ // this will cause a static field ref in the enum clinit
+ setupFruitEnumWithStaticField();
+ optimize("void", "String y = Fruit.staticField;");
+
+ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+ assertTrue(tracker.isOrdinalized("test.EntryPoint$Fruit"));
+ }
+
+ public void testOrdinalizableStaticMethod()
+ throws UnableToCompleteException {
+ EnumOrdinalizer.resetTracker();
+
+ // this will cause a static method enum class
+ setupFruitEnumWithStaticMethod();
+ optimize("void", "int y = Fruit.staticMethod();");
+
+ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+ assertTrue(tracker.isOrdinalized("test.EntryPoint$Fruit"));
+ }
+
+ public void testNotOrdinalizableInstanceStaticFieldRef()
+ throws UnableToCompleteException {
+ EnumOrdinalizer.resetTracker();
+
+ // this will cause a static field ref in the enum clinit
+ setupFruitEnumWithStaticField();
+ optimize("void", "Fruit fruit = Fruit.APPLE;",
+ "String y = fruit.staticField;");
+
+ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+ assertTrue(tracker.isVisited("test.EntryPoint$Fruit"));
+ assertFalse(tracker.isOrdinalized("test.EntryPoint$Fruit"));
+ }
+
+ public void testNotOrdinalizableInstanceStaticMethod()
+ throws UnableToCompleteException {
+ EnumOrdinalizer.resetTracker();
+
+ // this will cause a static method enum class
+ setupFruitEnumWithStaticMethod();
+ optimize("void", "Fruit fruit = Fruit.APPLE;",
+ "int y = fruit.staticMethod();");
+
+ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+ assertTrue(tracker.isVisited("test.EntryPoint$Fruit"));
+ assertFalse(tracker.isOrdinalized("test.EntryPoint$Fruit"));
+ }
public void testNotOrdinalizableClassLiteralReference()
throws UnableToCompleteException {
@@ -383,20 +434,40 @@
assertFalse(tracker.isOrdinalized("test.EntryPoint$Fruit"));
}
- public void testNotOrdinalizableInstanceOf() {
- /*
- * TODO(jbrosenberg): determine if this really needs to be true, and
write a
- * test to prove it.
- */
+ public void testNotOrdinalizableInstanceOfEnumExpression()
+ throws UnableToCompleteException {
+ setupFruitEnum();
+ optimize("void", "Fruit fruit = Fruit.APPLE;",
+ "if (fruit instanceof Enum) {",
+ " fruit = Fruit.ORANGE;",
+ "}");
+
+ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+ assertTrue(tracker.isVisited("test.EntryPoint$Fruit"));
+ assertFalse(tracker.isOrdinalized("test.EntryPoint$Fruit"));
}
- public void testNotOrdinalizableStaticFieldRef()
+ public void testNotOrdinalizableInstanceOfEnumTestType()
throws UnableToCompleteException {
+ setupFruitEnum();
+ optimize("void", "Object fruitObj = new Object();",
+ "if (fruitObj instanceof Fruit) {",
+ " fruitObj = null;",
+ "}");
+
+ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+ assertTrue(tracker.isVisited("test.EntryPoint$Fruit"));
+ assertFalse(tracker.isOrdinalized("test.EntryPoint$Fruit"));
+ }
+
+ public void testNotOrdinalizableStaticFieldRefToVALUES()
+ throws UnableToCompleteException {
EnumOrdinalizer.resetTracker();
- // this will cause a static field ref in the enum clinit
- setupFruitEnumWithStaticField();
- optimize("void");
+ // this ends up inlining the values() method call, and thus $VALUES is
referenced external
+ // to the Fruit enum class.
+ setupFruitEnum();
+ optimize("void", "Fruit[] fruits = Fruit.values();");
EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
assertTrue(tracker.isVisited("test.EntryPoint$Fruit"));
@@ -407,7 +478,9 @@
throws UnableToCompleteException {
EnumOrdinalizer.resetTracker();
- // this ends up calling Fruit.clinit() first (which is a static method
call)
+ // make sure values() method call doesn't doesn't get inlined
+ runMethodInliner = false;
+
setupFruitEnum();
optimize("void", "Fruit[] fruits = Fruit.values();");
@@ -824,7 +897,6 @@
private void setupFruitEnum() {
addSnippetClassDecl("public enum Fruit {APPLE, ORANGE}");
- setupExtraDummyEnum();
}
private void setupFruitEnumWithInstanceField() {
@@ -834,31 +906,27 @@
" instanceField = str;",
" }",
"}");
- setupExtraDummyEnum();
}
private void setupFruitEnumWithStaticField() {
addSnippetClassDecl("public enum Fruit {APPLE, ORANGE;",
" public static final String staticField =
\"STATIC\";",
"}");
- setupExtraDummyEnum();
+ }
+
+ private void setupFruitEnumWithStaticMethod() {
+ addSnippetClassDecl("public enum Fruit {APPLE, ORANGE;",
+ " public static final int staticMethod() {",
+ " int x = 0;",
+ " return x;",
+ " }",
+ "}");
}
private void setupVegetableEnum() {
addSnippetClassDecl("public enum Vegetable {CARROT, SPINACH}");
}
- private void setupExtraDummyEnum() {
- /*
- * Assure there are at least more 2 enums in the program, so inlining
or
- * tightening doesn't push a single enum sub-class into the methods of
the
- * Enum super-class itself (which prevents ordinalization in most
cases).
- * TODO(jbrosenberg): Make ordinalization work if there's only 1 enum
in
- * a program.
- */
- addSnippetClassDecl("public enum DummyEnum {DUMMY}");
- }
-
private void setupFruitSwitchMethod() {
addSnippetClassDecl("public static String fruitSwitch(Fruit fruit) {",
" switch(fruit) {",
@@ -878,19 +946,11 @@
private final boolean runCastNormalizer = true;
private final boolean runEqualityNormalizer = true;
- /*
- * EnumOrdinalizer depends MakeCallsStatic and MethodInliner running
before
- * it runs, since they cleanup the internal structure of an enum class to
- * inline instance methods like $init.
- * TODO(jbrosenberg): Update EnumOrdinalizer to be able to succeed
- * irrespective of the ordering and interaction with other optimizers.
- */
- private final boolean runMakeCallsStatic = true;
- private final boolean runMethodInliner = true;
-
- // these can be enabled where needed
- private boolean runMethodCallTightener = false;
- private boolean runTypeTightener = false;
+ // These are enabled as needed for a given test
+ private boolean runMakeCallsStatic;
+ private boolean runMethodInliner;
+ private boolean runMethodCallTightener;
+ private boolean runTypeTightener;
@Override
protected boolean optimizeMethod(JProgram program, JMethod method) {
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
