Refactor JsInliner to support FORCE_INLINE.

Change-Id: I596daf7eaf04046648834f95cb2c706478953d8d

dev/BUILD

@@ -185,6 +185,7 @@ java_library(
             "core/src/com/google/gwt/dev/PrecompilationResult.java",
             "core/src/com/google/gwt/dev/StringAnalyzableTypeEnvironment.java",
             "core/src/com/google/gwt/dev/cfg/**/*.java",
+            "core/src/com/google/gwt/dev/common/**/*.java",
             "core/src/com/google/gwt/dev/javac/**/*.java",
             "core/src/com/google/gwt/dev/jdt/**/*.java",
             "core/src/com/google/gwt/dev/jjs/**/*.java",

dev/core/src/com/google/gwt/dev/common/InliningMode.java

@@ -0,0 +1,23 @@
+/*
+ * Copyright 2008 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.dev.common;
+
+/**
+ * Used to mark methods to be handled  in specific ways by inliners.
+ */
+public enum InliningMode {
+  NORMAL, DO_NOT_INLINE, FORCE_INLINE;
+}

dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java

@@ -133,6 +133,7 @@
 import com.google.gwt.dev.js.JsBreakUpLargeVarStatements;
 import com.google.gwt.dev.js.JsDuplicateCaseFolder;
 import com.google.gwt.dev.js.JsDuplicateFunctionRemover;
+import com.google.gwt.dev.js.JsForceInliningChecker;
 import com.google.gwt.dev.js.JsIncrementalNamer;
 import com.google.gwt.dev.js.JsInliner;
 import com.google.gwt.dev.js.JsLiteralInterner;
@@ -392,6 +393,9 @@ private PermutationResult compilePermutation(Permutation permutation, UnifiedAst
       // (7) Optimize the JS AST.
       final Set<JsNode> inlinableJsFunctions = jjsMapAndInlineableFunctions.getRight();
       optimizeJs(inlinableJsFunctions);
+      if (options.getOptimizationLevel() > OptionOptimize.OPTIMIZE_LEVEL_DRAFT) {
+        JsForceInliningChecker.check(logger, jjsmap, jsProgram);
+      }
 
       // TODO(stalcup): move to normalization
       // Must run before code splitter and namer.
@@ -497,7 +501,8 @@ private void optimizeJava() throws InterruptedException {
     }
   }
 
-  private void optimizeJs(Set<JsNode> inlinableJsFunctions) throws InterruptedException {
+  private void optimizeJs(Set<JsNode> inlinableJsFunctions)
+      throws InterruptedException, UnableToCompleteException {
     if (shouldOptimize()) {
       optimizeJsLoop(inlinableJsFunctions);
       JsDuplicateCaseFolder.exec(jsProgram);
@@ -994,7 +999,8 @@ private InputStream openWithGunzip(EmittedArtifact artifact)
     return new BufferedInputStream(new GZIPInputStream(artifact.getContents(TreeLogger.NULL)));
   }
 
-  private void optimizeJsLoop(Collection<JsNode> toInline) throws InterruptedException {
+  private void optimizeJsLoop(Collection<JsNode> toInline)
+      throws InterruptedException, UnableToCompleteException {
     int optimizationLevel = options.getOptimizationLevel();
     List<OptimizerStats> allOptimizerStats = Lists.newArrayList();
     int counter = 0;

dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java

@@ -15,6 +15,7 @@
  */
 package com.google.gwt.dev.jjs.ast;
 
+import com.google.gwt.dev.common.InliningMode;
 import com.google.gwt.dev.jjs.InternalCompilerException;
 import com.google.gwt.dev.jjs.SourceInfo;
 import com.google.gwt.dev.jjs.SourceOrigin;
@@ -45,7 +46,7 @@ public class JMethod extends JNode implements JMember, CanBeAbstract, CanBeNativ
    * patterns, then it will marked as {@code UNDEFINED} to be later signaled as error in
    * {@link com.google.gwt.dev.jjs.impl.JsInteropRestrictionChecker}.
    */
-  public static enum JsPropertyAccessorType {
+  public enum JsPropertyAccessorType {
     GETTER, SETTER, UNDEFINED;
   }
 
@@ -61,7 +62,8 @@ public int compare(JMethod m1, JMethod m2) {
   private String exportNamespace;
   private JsPropertyAccessorType jsPropertyType;
   private Specialization specialization;
-  private boolean inliningAllowed = true;
+  private InliningMode inliningMode = InliningMode.NORMAL;
+  private boolean preventDevirtualization = false;
   private boolean hasSideEffects = true;
   private boolean defaultMethod = false;
 
@@ -211,11 +213,15 @@ public void removeSpecialization() {
   }
 
   public boolean isInliningAllowed() {
-    return inliningAllowed;
+    return inliningMode != InliningMode.DO_NOT_INLINE;
   }
 
-  public void setInliningAllowed(boolean inliningAllowed) {
-    this.inliningAllowed = inliningAllowed;
+  public InliningMode getInliningMode() {
+    return inliningMode;
+  }
+
+  public void setInliningMode(InliningMode inliningMode) {
+    this.inliningMode = inliningMode;
   }
 
   public boolean hasSideEffects() {
@@ -234,6 +240,14 @@ public boolean isDefaultMethod() {
     return defaultMethod;
   }
 
+  public boolean isDevirtualizationAllowed() {
+    return !preventDevirtualization;
+  }
+
+  public void disallowDevirtualization() {
+    this.preventDevirtualization = true;
+  }
+
   /**
    * AST representation of @SpecializeMethod.
    */

dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java

@@ -16,6 +16,7 @@
 package com.google.gwt.dev.jjs.ast;
 
 import com.google.gwt.dev.MinimalRebuildCache;
+import com.google.gwt.dev.common.InliningMode;
 import com.google.gwt.dev.jjs.Correlation.Literal;
 import com.google.gwt.dev.jjs.InternalCompilerException;
 import com.google.gwt.dev.jjs.SourceInfo;
@@ -290,31 +291,12 @@ public static void serializeTypes(List<JDeclaredType> types, ObjectOutputStream
 
   private FragmentPartitioningResult fragmentPartitioningResult;
 
-  /**
-   * Set of method that are pinned and should be skipped by optimizations such as
-   * inlining, statification and prunned.
-   */
-  private Set<JMethod> pinnedMethods = Sets.newHashSet();
-
-  /**
-   * Returns true if the inliner should try to inline {@code method}.
-   */
-  public boolean isInliningAllowed(JMethod method) {
-    return !pinnedMethods.contains(method) && method.isInliningAllowed();
-  }
-
-  /**
-   * Returns true if {@link MakeCallsStatic} should try to statify {@code method}.
-   */
-  public boolean isDevitualizationAllowed(JMethod method) {
-    return !pinnedMethods.contains(method);
-  }
-
   /**
    * Add a pinned method.
    */
   public void addPinnedMethod(JMethod method) {
-    pinnedMethods.add(method);
+    method.setInliningMode(InliningMode.DO_NOT_INLINE);
+    method.disallowDevirtualization();
   }
 
   public JProgram(MinimalRebuildCache minimalRebuildCache) {

dev/core/src/com/google/gwt/dev/jjs/impl/Devirtualizer.java

@@ -267,7 +267,7 @@ private JMethod createDevirtualMethodFor(JMethod method, JDeclaredType inClass)
     JMethod devirtualMethod = new JMethod(sourceInfo, prefix + "__devirtual$",
         inClass, method.getType(), false, true, true, AccessModifier.PUBLIC);
     // TODO(rluble): DoNotInline should be carried over if 'any' of the targets is marked so.
-    devirtualMethod.setInliningAllowed(method.isInliningAllowed());
+    devirtualMethod.setInliningMode(method.getInliningMode());
     devirtualMethod.setBody(new JMethodBody(sourceInfo));
     devirtualMethod.setSynthetic();
     inClass.addMethod(devirtualMethod);

dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java

@@ -21,6 +21,7 @@
 import com.google.gwt.dev.MinimalRebuildCache;
 import com.google.gwt.dev.PrecompileTaskOptions;
 import com.google.gwt.dev.cfg.PermutationProperties;
+import com.google.gwt.dev.common.InliningMode;
 import com.google.gwt.dev.jjs.HasSourceInfo;
 import com.google.gwt.dev.jjs.InternalCompilerException;
 import com.google.gwt.dev.jjs.SourceInfo;
@@ -742,11 +743,6 @@ private class GenerateJavaScriptVisitor extends JVisitor {
 
     private final JsName prototype = objectScope.declareName("prototype");
 
-    // JavaScript functions that arise from methods that were not inlined in the Java AST
-    // NOTE: We use a LinkedHashSet to preserve the order of insertion. So that the following passes
-    // that use this result are deterministic.
-    private final Set<JsNode> functionsForJsInlining = Sets.newLinkedHashSet();
-
     {
       globalTemp.setObfuscatable(false);
       prototype.setObfuscatable(false);
@@ -1238,14 +1234,7 @@ public void endVisit(JMethod x, Context ctx) {
 
       javaMethodForJSFunction.put(jsFunc, x);
 
-      if (!program.isInliningAllowed(x)) {
-        jsProgram.disallowInlining(jsFunc);
-      }
-
-      // Collect the resulting function to be considered by the JsInliner.
-      if (methodsForJsInlining.contains(x)) {
-        functionsForJsInlining.add(jsFunc);
-      }
+      jsFunc.setInliningMode(x.getInliningMode());
 
       List<JsParameter> params = popList(x.getParams().size()); // params
 
@@ -1391,8 +1380,11 @@ private JsExpression dispatchToSuper(JMethodCall x, JMethod method, List<JsExpre
          */
         qualifiedMethodName.setQualifier(names.get(method).makeRef(x.getSourceInfo()));
       } else {
-        // Regular super call. This calls are always static and optimizations normally statify them.
-        // They can appear in completely unoptimized code, hence need to be handled here.
+        // These are regular super method call. These calls are always dispatched statically and
+        // optimizations will statify them (except in a few cases, like being target of
+        // {@link Impl.getNameOf}.
+        // For the most part these calls only appear in completely unoptimized code, hence need to
+        // be handled here.
 
         // Construct JCHSU.getPrototypeFor(type).polyname
         // TODO(rluble): Ideally we would want to construct the inheritance chain the JS way and
@@ -1403,8 +1395,6 @@ private JsExpression dispatchToSuper(JMethodCall x, JMethod method, List<JsExpre
         JsInvocation getPrototypeCall = constructInvocation(x.getSourceInfo(),
             "JavaClassHierarchySetupUtil.getClassPrototype",
             convertJavaLiteral(typeMapper.get(superMethodTargetType)));
-        // getClassPrototype is a JSNI call, so we are enabling inlining
-        methodsForJsInlining.add(currentMethod);
 
         JsNameRef methodNameRef = polymorphicNames.get(method).makeRef(x.getSourceInfo());
         methodNameRef.setQualifier(getPrototypeCall);
@@ -3135,15 +3125,22 @@ public static JsUnaryOperator get(JUnaryOperator op) {
     }
   }
 
-  private class RecordJSInlinableMethods extends JVisitor {
+  private class CollectJsFunctionsForInlining extends JVisitor {
 
+    // JavaScript functions that arise from methods that were not inlined in the Java AST
+    // NOTE: We use a LinkedHashSet to preserve the order of insertion. So that the following passes
+    // that use this result are deterministic.
+    private Set<JsNode> functionsForJsInlining = Sets.newLinkedHashSet();
     private JMethod currentMethod;
 
     @Override
     public void endVisit(JMethod x, Context ctx) {
       if (x.isNative()) {
-        // These are methods that were not considered by the Java method inliner.
-        methodsForJsInlining.add(x);
+        // These are methods whose bodies where not traversed by the Java method inliner.
+        JsFunction function = methodBodyMap.get(x.getBody());
+        if (function != null && function.getBody() != null) {
+          functionsForJsInlining.add(function);
+        }
       }
 
       currentMethod = null;
@@ -3152,11 +3149,17 @@ public void endVisit(JMethod x, Context ctx) {
     @Override
     public void endVisit(JMethodCall x, Context ctx) {
       JMethod target = x.getTarget();
-      if (program.isInliningAllowed(target) && (target.isNative()
-          || program.getIndexedTypes().contains(target.getEnclosingType()))) {
-        // the currentMethod calls a method that was not considered by the Java MethodInliner; these
-        // include JSNI methods and methods whose calls were inserted by normalization passes.
-        methodsForJsInlining.add(currentMethod);
+      if (target.isInliningAllowed() && (target.isNative()
+          || program.getIndexedTypes().contains(target.getEnclosingType())
+          || target.getInliningMode() == InliningMode.FORCE_INLINE)) {
+        // These are either: 1) callsites to JSNI functions, in which case MethodInliner did not
+        // attempt to inline; 2) inserted by normalizations passes AFTER all inlining or 3)
+        // calls to methods annotated with @ForceInline that were not inlined by the simple
+        // MethodInliner.
+        JsFunction function = methodBodyMap.get(currentMethod.getBody());
+        if (function != null && function.getBody() != null) {
+          functionsForJsInlining.add(function);
+        }
       }
     }
 
@@ -3165,6 +3168,11 @@ public boolean visit(JMethod x, Context ctx) {
       currentMethod = x;
       return true;
     }
+
+    public Set<JsNode> getFunctionsForJsInlining() {
+      accept(program);
+      return functionsForJsInlining;
+    }
   }
 
   /**
@@ -3324,12 +3332,6 @@ public static Pair<JavaToJavaScriptMap, Set<JsNode>> exec(TreeLogger logger, JPr
 
   private Map<String, JsName> indexedFields = Maps.newHashMap();
 
-  /**
-   * Methods where inlining hasn't happened yet because they are native or contain calls to native
-   * methods. See {@link RecordJSInlinableMethods}.
-   */
-  private Set<JMethod> methodsForJsInlining = Sets.newHashSet();
-
   /**
    * Contains JsNames for all interface methods. A special scope is needed so
    * that independent classes will obfuscate their interface implementation
@@ -3538,7 +3540,6 @@ private Pair<JavaToJavaScriptMap, Set<JsNode>> execImpl() {
     if (!incremental) {
       // TODO(rluble): pull out this analysis and make it a Java AST optimization pass.
       new RecordCrossClassCallsAndConstructorLiveness().accept(program);
-      new RecordJSInlinableMethods().accept(program);
     }
 
     // Map class literals to their respective types.
@@ -3558,7 +3559,10 @@ private Pair<JavaToJavaScriptMap, Set<JsNode>> execImpl() {
     JavaToJavaScriptMap jjsMap = new JavaToJavaScriptMapImpl(program.getDeclaredTypes(),
         names, typeForStatMap, vtableInitForMethodMap);
 
-    return Pair.create(jjsMap, generator.functionsForJsInlining);
+    Set<JsNode> functionsForJsInlining = incremental ? Collections.<JsNode>emptySet() :
+        new CollectJsFunctionsForInlining().getFunctionsForJsInlining();
+
+    return Pair.create(jjsMap, functionsForJsInlining);
   }
 
   private JsFunction getJsFunctionFor(JMethod jMethod) {

dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java

@@ -16,6 +16,7 @@
 package com.google.gwt.dev.jjs.impl;
 
 import com.google.gwt.dev.CompilerContext;
+import com.google.gwt.dev.common.InliningMode;
 import com.google.gwt.dev.javac.JSORestrictionsChecker;
 import com.google.gwt.dev.javac.JdtUtil;
 import com.google.gwt.dev.javac.JsInteropUtil;
@@ -234,6 +235,9 @@
 import java.util.Map.Entry;
 import java.util.Set;
 
+import javaemul.internal.annotations.DoNotInline;
+import javaemul.internal.annotations.ForceInline;
+
 /**
  * Constructs a GWT Java AST from a single isolated compilation unit. The AST is
  * not associated with any {@link com.google.gwt.dev.jjs.ast.JProgram} and will
@@ -2811,7 +2815,7 @@ private void writeEnumValuesMethod(JEnumType type, JMethod method) {
       JNewArray valuesArrayCopy = JNewArray.createInitializers(info, enumArrayType, initializers);
       if (type.getEnumList().size() > MAX_INLINEABLE_ENUM_SIZE) {
         // Only inline values() if it is small.
-        method.setInliningAllowed(false);
+        method.setInliningMode(InliningMode.DO_NOT_INLINE);
       }
       JjsUtils.replaceMethodBody(method, valuesArrayCopy);
     }
@@ -4088,17 +4092,19 @@ private void createMethod(AbstractMethodDeclaration x) {
   private void processAnnotations(AbstractMethodDeclaration x,
       JMethod method) {
     maybeAddMethodSpecialization(x, method);
-    maybeSetDoNotInline(x, method);
+    maybeSetInliningMode(x, method);
     maybeSetHasNoSideEffects(x, method);
     if (isJsInteropEnabled) {
       JsInteropUtil.maybeSetJsInteropProperties(method, x.annotations);
     }
   }
 
-  private void maybeSetDoNotInline(AbstractMethodDeclaration x,
+  private void maybeSetInliningMode(AbstractMethodDeclaration x,
       JMethod method) {
-    if (JdtUtil.getAnnotation(x.binding, "javaemul.internal.annotations.DoNotInline") != null) {
-      method.setInliningAllowed(false);
+    if (JdtUtil.getAnnotation(x.binding, DoNotInline.class.getCanonicalName()) != null) {
+      method.setInliningMode(InliningMode.DO_NOT_INLINE);
+    } else if (JdtUtil.getAnnotation(x.binding, ForceInline.class.getCanonicalName()) != null) {
+      method.setInliningMode(InliningMode.FORCE_INLINE);
     }
   }