Revision: 7868
Author: [email protected]
Date: Fri Apr  2 12:42:00 2010
Log: Removes trivially-delegating clinits, plus some.

1) A trivial clinit that merely delegates to a superclass doesn't emit code; instead, call sites that would have targeted the subclass will target the superclass instead.

2) As an added bonus, in situations where you have two classes that sharing a supertype, where only the supertype has a clinit, calls between the two classes won't trigger clinit checks anymore. This also works when a supetype calls into a subtype when only the supertype has a clinit, which is exactly the case that enums are currently tripping over.

http://gwt-code-reviews.appspot.com/184802/show
Review by: spoon

http://code.google.com/p/google-web-toolkit/source/detail?r=7868

Modified:
 /trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java
 /trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
 /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
 /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
 /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java

=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java Mon Feb 8 08:29:30 2010 +++ /trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java Fri Apr 2 12:42:00 2010
@@ -52,10 +52,10 @@
   protected transient List<JMethod> methods = Lists.create();

   /**
-   * Tracks whether this class has a dynamic clinit. Defaults to true until
-   * shown otherwise.
+ * Tracks the target static initialization for this class. Default to self
+   * until removed or set to be a superclass.
    */
-  private boolean hasClinit = true;
+  private JDeclaredType clinitTarget = this;

   /**
    * This type's super class.
@@ -124,17 +124,12 @@
       // Target has no clinit (common case).
       return false;
     }
-
-    // See if I'm a subclass.
-    JClassType checkType = this.getSuperClass();
-    while (checkType != null) {
-      if (checkType == targetType) {
-        // I am a subclass.
-        return false;
-      }
-      checkType = checkType.getSuperClass();
-    }
-    return true;
+    /*
+ * The clinit for the source of the reference must already have run, so if + * it's the same as this one, there it must have already run. One example is
+     * a reference from a subclass to something in a superclass.
+     */
+    return this.getClinitTarget() != targetType.getClinitTarget();
   }

   public JAnnotation findAnnotation(String className) {
@@ -148,6 +143,14 @@
   public List<JNode> getArtificialRescues() {
     return artificialRescues;
   }
+
+  /**
+   * Returns the class that must be initialized to use this class. May be a
+ * superclass, or <code>null</code> if this class has no static initializer.
+   */
+  public JDeclaredType getClinitTarget() {
+    return clinitTarget;
+  }

   /**
* Returns this type's fields;does not include fields defined in a super type
@@ -202,7 +205,7 @@
* Returns <code>true</code> when this class's clinit must be run dynamically.
    */
   public boolean hasClinit() {
-    return hasClinit;
+    return clinitTarget != null;
   }

   /**
@@ -265,7 +268,7 @@
     annotations = (List<JAnnotation>) stream.readObject();
   }

-/**
+  /**
    * See {...@link #writeMethodBodies(ObjectOutputStream).
    *
    * @see #writeMethodBodies(ObjectOutputStream)
@@ -278,13 +281,19 @@
   }

   /**
- * Called when this class's clinit is empty or can be run at the top level. + * Called to set this class's trivial initializer to point to a superclass.
    */
-  void removeClinit() {
-    assert hasClinit();
-    JMethod clinitMethod = methods.get(0);
-    assert JProgram.isClinit(clinitMethod);
-    hasClinit = false;
+  void setClinitTarget(JDeclaredType newClinitTarget) {
+    if (clinitTarget == newClinitTarget) {
+      return;
+    }
+    if (getClass().desiredAssertionStatus()) {
+      // Make sure this is a pure upgrade to a superclass or null.
+ for (JDeclaredType current = clinitTarget; current != newClinitTarget; current = current.getSuperClass()) {
+        assert current.getSuperClass() != null;
+      }
+    }
+    clinitTarget = newClinitTarget;
   }

   /**
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java Thu Mar 11 17:23:20 2010 +++ /trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java Fri Apr 2 12:42:00 2010
@@ -222,6 +222,10 @@
    */
private final Map<JInterfaceType, Set<JClassType>> isImplementedMap = new IdentityHashMap<JInterfaceType, Set<JClassType>>();

+  private JDeclaredType javaIoSerializable;
+
+  private JDeclaredType javaLangCloneable;
+
   /**
    * Caches the {...@link Object} class.
    */
@@ -261,7 +265,6 @@
    * indirectly.
    */
private final Map<JInterfaceType, Set<JInterfaceType>> superInterfaceMap = new IdentityHashMap<JInterfaceType, Set<JInterfaceType>>();
-
   /**
    * A map of all methods with virtual overrides, onto the collection of
* overridden methods. Each key method's collections is a map of the set of
@@ -271,9 +274,6 @@
    */
private final Map<JMethod, Map<JClassType, Set<JMethod>>> virtualUpRefMap = new IdentityHashMap<JMethod, Map<JClassType, Set<JMethod>>>();

-  private JDeclaredType javaIoSerializable;
-  private JDeclaredType javaLangCloneable;
-
   public JTypeOracle(JProgram program) {
     this.program = program;
   }
@@ -348,16 +348,6 @@
     return true;
   }

-  public boolean canTriviallyCast(JType type, JType qType) {
-    if (type instanceof JPrimitiveType &&
-        qType instanceof JPrimitiveType) {
-      return type == qType;
-    } else if (type instanceof JReferenceType &&
-        qType instanceof JReferenceType) {
- return canTriviallyCast((JReferenceType) type, (JReferenceType) qType);
-    }
-    return false;
-  }
public boolean canTriviallyCast(JReferenceType type, JReferenceType qType) {
     if (type.canBeNull() && !qType.canBeNull()) {
       // Cannot reliably cast nullable to non-nullable
@@ -431,6 +421,16 @@

     return false;
   }
+
+  public boolean canTriviallyCast(JType type, JType qType) {
+ if (type instanceof JPrimitiveType && qType instanceof JPrimitiveType) {
+      return type == qType;
+    } else if (type instanceof JReferenceType
+        && qType instanceof JReferenceType) {
+ return canTriviallyCast((JReferenceType) type, (JReferenceType) qType);
+    }
+    return false;
+  }

   public void computeBeforeAST() {
     javaLangObject = program.getTypeJavaLangObject();
@@ -595,11 +595,8 @@
    */
   public void recomputeAfterOptimizations() {
     Set<JDeclaredType> computed = new IdentityHashSet<JDeclaredType>();
-    for (int i = 0; i < program.getDeclaredTypes().size(); ++i) {
-      JDeclaredType type = program.getDeclaredTypes().get(i);
-      if (type.hasClinit()) {
-        computeHasClinit(type, computed);
-      }
+    for (JDeclaredType type : program.getDeclaredTypes()) {
+      computeHasClinitTarget(type, computed);
     }
   }

@@ -638,16 +635,32 @@
     }
   }

- private void computeHasClinit(JDeclaredType type, Set<JDeclaredType> computed) {
-    if (computeHasClinitRecursive(type, computed,
-        new IdentityHashSet<JDeclaredType>())) {
-      computed.add(type);
+  private void computeHasClinitTarget(JDeclaredType type,
+      Set<JDeclaredType> computed) {
+    if (!type.hasClinit() || computed.contains(type)) {
+      return;
+    }
+    if (type.getSuperClass() != null) {
+      /*
+ * Compute super first so that it's already been tightened to the tightest + * possible target; this ensures if we're tightened as well it's to the
+       * transitively tightest target.
+       */
+      computeHasClinitTarget(type.getSuperClass(), computed);
+    }
+    if (type.getClinitTarget() != type) {
+      // I already have a trivial clinit, just follow my super chain.
+      type.setClinitTarget(type.getSuperClass().getClinitTarget());
     } else {
-      type.removeClinit();
-    }
+      // I still have a real clinit, actually compute.
+ JDeclaredType target = computeHasClinitTargetRecursive(type, computed,
+          new IdentityHashSet<JDeclaredType>());
+      type.setClinitTarget(target);
+    }
+    computed.add(type);
   }

-  private boolean computeHasClinitRecursive(JDeclaredType type,
+  private JDeclaredType computeHasClinitTargetRecursive(JDeclaredType type,
       Set<JDeclaredType> computed, Set<JDeclaredType> alreadySeen) {
     // Track that we've been seen.
     alreadySeen.add(type);
@@ -657,9 +670,18 @@
     CheckClinitVisitor v = new CheckClinitVisitor();
     v.accept(method);
     if (v.hasLiveCode()) {
-      return true;
-    }
-    for (JDeclaredType target : v.getClinitTargets()) {
+      return type;
+    }
+    // Check for trivial super clinit.
+    JDeclaredType[] clinitTargets = v.getClinitTargets();
+    if (clinitTargets.length == 1) {
+      JDeclaredType singleTarget = clinitTargets[0];
+      if (type instanceof JClassType && singleTarget instanceof JClassType
+          && isSuperClass((JClassType) type, (JClassType) singleTarget)) {
+        return singleTarget.getClinitTarget();
+      }
+    }
+    for (JDeclaredType target : clinitTargets) {
       if (!target.hasClinit()) {
         // A false result is always accurate.
         continue;
@@ -670,7 +692,7 @@
        * recomputed this run.
        */
       if (target.hasClinit() && computed.contains(target)) {
-        return true;
+        return type;
       }

       /*
@@ -681,15 +703,15 @@
         continue;
       }

-      if (computeHasClinitRecursive(target, computed, alreadySeen)) {
+ if (computeHasClinitTargetRecursive(target, computed, alreadySeen) != null) {
         // Calling a non-empty clinit means I am a real clinit.
-        return true;
+        return type;
       } else {
         // This clinit is okay, keep going.
         continue;
       }
     }
-    return false;
+    return null;
   }

   /**
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java Fri Apr 2 09:39:56 2010 +++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java Fri Apr 2 12:42:00 2010
@@ -15,6 +15,7 @@
  */
 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.JBinaryOperation;
 import com.google.gwt.dev.jjs.ast.JBinaryOperator;
@@ -330,9 +331,9 @@
         multi.exprs.add(instance);
       }

-      JMethodCall clinit = maybeCreateClinitCall(x);
-      if (clinit != null) {
-        multi.exprs.add(clinit);
+      if (x.hasClinit()) {
+        multi.exprs.add(createClinitCall(x.getSourceInfo(),
+            x.getField().getEnclosingType()));
       }

       if (literal != null) {
@@ -415,6 +416,10 @@
         // Eliminate the call if the target is now empty.
         if (!targetType.hasClinit()) {
           ctx.replaceMe(program.getLiteralNull());
+        } else if (targetType != targetType.getClinitTarget()) {
+          // Tighten the target.
+          ctx.replaceMe(createClinitCall(x.getSourceInfo(),
+              targetType.getClinitTarget()));
         }
       }
     }
@@ -464,9 +469,9 @@
       // Replace the new operation with a multi.
       JMultiExpression multi = new JMultiExpression(x.getSourceInfo());
       multi.exprs.addAll(x.getArgs());
-      JMethodCall clinit = maybeCreateClinitCall(x);
-      if (clinit != null) {
-        multi.exprs.add(clinit);
+      if (x.hasClinit()) {
+        multi.exprs.add(createClinitCall(x.getSourceInfo(),
+            x.getTarget().getEnclosingType()));
       }

       ctx.replaceMe(accept(multi));
@@ -691,6 +696,13 @@
       }
       return true;
     }
+
+    private JMethodCall createClinitCall(SourceInfo sourceInfo,
+        JDeclaredType targetType) {
+      JMethod clinit = targetType.getClinitTarget().getMethods().get(0);
+      assert (JProgram.isClinit(clinit));
+      return new JMethodCall(sourceInfo, null, clinit);
+    }

private void evalConcat(JExpression lhs, JExpression rhs, Context ctx) {
       if (lhs instanceof JValueLiteral && rhs instanceof JValueLiteral) {
@@ -1203,24 +1215,6 @@
     private Class<?> mapType(JType type) {
       return typeClassMap.get(type);
     }
-
-    private JMethodCall maybeCreateClinitCall(JFieldRef x) {
-      if (x.hasClinit()) {
- JMethod clinit = x.getField().getEnclosingType().getMethods().get(0);
-        assert (JProgram.isClinit(clinit));
-        return new JMethodCall(x.getSourceInfo(), null, clinit);
-      }
-      return null;
-    }
-
-    private JMethodCall maybeCreateClinitCall(JNewInstance x) {
-      if (x.hasClinit()) {
- JMethod clinit = x.getTarget().getEnclosingType().getMethods().get(0);
-        assert (JProgram.isClinit(clinit));
-        return new JMethodCall(x.getSourceInfo(), null, clinit);
-      }
-      return null;
-    }

     private int numRemovableExpressions(JMultiExpression x) {
       if (ignoringExpressionOutput.contains(x)) {
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java Thu Mar 25 12:00:47 2010 +++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java Fri Apr 2 12:42:00 2010
@@ -634,7 +634,7 @@
       List<JsFunction> jsFuncs = popList(x.getMethods().size()); // methods
       List<JsNode> jsFields = popList(x.getFields().size()); // fields

-      if (x.hasClinit()) {
+      if (x.getClinitTarget() == x) {
         JsFunction superClinit = clinitMap.get(x.getSuperClass());
         JsFunction myClinit = jsFuncs.get(0);
         handleClinit(myClinit, superClinit);
@@ -898,7 +898,7 @@
       List<JsVar> jsFields = popList(x.getFields().size()); // fields
List<JsStatement> globalStmts = jsProgram.getGlobalBlock().getStatements();

-      if (x.hasClinit()) {
+      if (x.getClinitTarget() == x) {
         JsFunction clinitFunc = jsFuncs.get(0);
         handleClinit(clinitFunc, null);
         globalStmts.add(clinitFunc.makeStmt());
@@ -1771,7 +1771,7 @@
         return null;
       }

-      JDeclaredType targetType = x.getEnclosingType();
+      JDeclaredType targetType = x.getEnclosingType().getClinitTarget();
       if (!currentMethod.getEnclosingType().checkClinitTo(targetType)) {
         return null;
       } else if (targetType.equals(program.getTypeClassLiteralHolder())) {
@@ -1802,7 +1802,7 @@
         return null;
       }

-      JMethod clinitMethod = enclosingType.getMethods().get(0);
+ JMethod clinitMethod = enclosingType.getClinitTarget().getMethods().get(0);
       SourceInfo sourceInfo = x.getSourceInfo().makeChild(
           GenerateJavaScriptVisitor.class, "clinit call");
       JsInvocation jsInvocation = new JsInvocation(sourceInfo);
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java Fri Apr 2 09:39:56 2010 +++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java Fri Apr 2 12:42:00 2010
@@ -155,8 +155,8 @@
     }

     private JMethodCall createClinitCall(JMethodCall x) {
-      JDeclaredType targetEnclosingType = x.getTarget().getEnclosingType();
- if (!currentMethod.getEnclosingType().checkClinitTo(targetEnclosingType)) { + JDeclaredType targetType = x.getTarget().getEnclosingType().getClinitTarget();
+      if (!currentMethod.getEnclosingType().checkClinitTo(targetType)) {
// Access from this class to the target class won't trigger a clinit
         return null;
       }
@@ -164,12 +164,12 @@
         // No clinit needed; target is really an instance method.
         return null;
       }
- if (x.getTarget() == x.getTarget().getEnclosingType().getMethods().get(0)) {
+      if (JProgram.isClinit(x.getTarget())) {
         // This is a clinit call, doesn't need another clinit
         return null;
       }

-      JMethod clinit = targetEnclosingType.getMethods().get(0);
+      JMethod clinit = targetType.getMethods().get(0);

// If the clinit is a non-native, empty body we can optimize it out here
       if (!clinit.isNative()

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe, reply using "remove me" as the subject.

Reply via email to