Improve the compile time cost for Optimize Calls from #1 to google#17 …

…(roughly 25% of its previous time).

Instead of using the DefinitionUseSiteFinder, as simpler data structure is introduced: OptimizeCalls.ReferenceMap.  The ReferenceMap provides a simple list of nodes for every global name and property. This avoids building objects and structures, and identifiers that are unneeded, and avoided redundant analysis.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=173304853

src/com/google/javascript/jscomp/AbstractCompiler.java

@@ -281,15 +281,6 @@ static enum MostRecentTypechecker {
    */
   abstract <T extends TypeIRegistry> T getGlobalTypeInfo();
 
-  /**
-   * Used by three passes that run in sequence (optimize-returns,
-   * optimize-parameters, remove-unused-variables), to avoid having them
-   * recompute it independently.
-   */
-  abstract DefinitionUseSiteFinder getDefinitionFinder();
-
-  abstract void setDefinitionFinder(DefinitionUseSiteFinder defFinder);
-
   abstract void setExternExports(String externExports);
 
   /**

src/com/google/javascript/jscomp/AstValidator.java

@@ -475,12 +475,8 @@ private void validateTemplateLit(Node n) {
     if (!n.hasChildren()) {
       return;
     }
-    int i = 0;
-    for (Node child = n.getFirstChild(); child != null; child = child.getNext(), i++) {
-      // If the first child is not a STRING, this is a tagged template.
-      if (i == 0 && !child.isString()) {
-        validateExpression(child);
-      } else if (child.isString()) {
+    for (Node child = n.getFirstChild(); child != null; child = child.getNext()) {
+      if (child.isString()) {
         validateString(child);
       } else {
         validateTemplateLitSub(child);
@@ -604,6 +600,7 @@ private void validateClassMember(Node n, boolean isAmbient) {
       case MEMBER_FUNCTION_DEF:
       case GETTER_DEF:
       case SETTER_DEF:
+        validateObjectLiteralKeyName(n);
         validateChildCount(n);
         Node function = n.getFirstChild();
         if (isAmbient) {

src/com/google/javascript/jscomp/ChangeVerifier.java

@@ -211,6 +211,12 @@ private static boolean isEquivalentToExcludingFunctions(
       }
     }
 
+    if (thisNode.getParent() != null && thisNode.getParent().isParamList()) {
+      if (thisNode.isUnusedParameter() != thatNode.isUnusedParameter()) {
+        return false;
+      }
+    }
+
     Node thisChild = thisNode.getFirstChild();
     Node thatChild = thatNode.getFirstChild();
     while (thisChild != null && thatChild != null) {

src/com/google/javascript/jscomp/Compiler.java

@@ -227,9 +227,6 @@ public SourceFile loadSource(String filename) {
 
   public PerformanceTracker tracker;
 
-  // Used by optimize-returns, optimize-parameters and remove-unused-variables
-  private DefinitionUseSiteFinder defFinder = null;
-
   // Types that have been forward declared
   private Set<String> forwardDeclaredTypes = new HashSet<>();
 
@@ -1683,16 +1680,6 @@ GlobalTypeInfo getGlobalTypeInfo() {
     return this.globalTypeInfo;
   }
 
-  @Override
-  DefinitionUseSiteFinder getDefinitionFinder() {
-    return this.defFinder;
-  }
-
-  @Override
-  void setDefinitionFinder(DefinitionUseSiteFinder defFinder) {
-    this.defFinder = defFinder;
-  }
-
   public void maybeSetTracker() {
     if (options.getTracerMode().isOn()) {
       PrintStream tracerOutput =

src/com/google/javascript/jscomp/DefaultPassConfig.java

@@ -970,10 +970,6 @@ private List<PassFactory> getMainOptimizationLoop() {
       passes.add(inlineProperties);
     }
 
-    boolean runOptimizeCalls = options.optimizeCalls
-        || options.optimizeParameters
-        || options.optimizeReturns;
-
     if (options.removeUnusedVars || options.removeUnusedLocalVars) {
       if (options.deadAssignmentElimination) {
         passes.add(deadAssignmentsElimination);
@@ -986,19 +982,14 @@ private List<PassFactory> getMainOptimizationLoop() {
           passes.add(deadPropertyAssignmentElimination);
         }
       }
-      if (!runOptimizeCalls) {
-        passes.add(getRemoveUnusedVars(PassNames.REMOVE_UNUSED_VARS, false));
-      }
     }
 
-    if (runOptimizeCalls) {
+    if (options.optimizeCalls || options.optimizeParameters || options.optimizeReturns) {
       passes.add(optimizeCalls);
-      // RemoveUnusedVars cleans up after optimizeCalls, so we run it here.
-      // It has a special name because otherwise PhaseOptimizer would change its
-      // position in the optimization loop.
-      if (options.optimizeCalls) {
-        passes.add(getRemoveUnusedVars("removeUnusedVars_afterOptimizeCalls", true));
-      }
+    }
+
+    if (options.removeUnusedVars || options.removeUnusedLocalVars) {
+      passes.add(getRemoveUnusedVars());
     }
 
     if (options.j2clPassMode.shouldAddJ2clPasses()) {
@@ -2749,18 +2740,17 @@ protected CompilerPass create(AbstractCompiler compiler) {
         }
       };
 
-  private PassFactory getRemoveUnusedVars(String name, final boolean modifyCallSites) {
-    return getRemoveUnusedVars(name, modifyCallSites, false /* isOneTimePass */);
+  private PassFactory getRemoveUnusedVars() {
+    return getRemoveUnusedVars(false /* isOneTimePass */);
   }
 
   private PassFactory lastRemoveUnusedVars() {
-    return getRemoveUnusedVars(PassNames.REMOVE_UNUSED_VARS, false, true /* isOneTimePass */);
+    return getRemoveUnusedVars(true /* isOneTimePass */);
   }
 
-  private PassFactory getRemoveUnusedVars(
-      String name, final boolean modifyCallSites, boolean isOneTimePass) {
+  private PassFactory getRemoveUnusedVars(boolean isOneTimePass) {
     /** Removes variables that are never used. */
-    return new PassFactory(name, isOneTimePass) {
+    return new PassFactory(PassNames.REMOVE_UNUSED_VARS, isOneTimePass) {
       @Override
       protected CompilerPass create(AbstractCompiler compiler) {
         boolean removeOnlyLocals = options.removeUnusedLocalVars && !options.removeUnusedVars;
@@ -2769,8 +2759,7 @@ protected CompilerPass create(AbstractCompiler compiler) {
         return new RemoveUnusedVars(
             compiler,
             !removeOnlyLocals,
-            preserveAnonymousFunctionNames,
-            modifyCallSites);
+            preserveAnonymousFunctionNames);
       }
 
       @Override

src/com/google/javascript/jscomp/DevirtualizePrototypeMethods.java

@@ -58,8 +58,7 @@
  * </pre>
  *
  */
-class DevirtualizePrototypeMethods
-    implements OptimizeCalls.CallGraphCompilerPass, CompilerPass {
+class DevirtualizePrototypeMethods implements CompilerPass {
   private final AbstractCompiler compiler;
 
   DevirtualizePrototypeMethods(AbstractCompiler compiler) {
@@ -73,7 +72,6 @@ public void process(Node externs, Node root) {
     process(externs, root, defFinder);
   }
 
-  @Override
   public void process(
       Node externs, Node root, DefinitionUseSiteFinder definitions) {
     for (DefinitionSite defSite : definitions.getDefinitionSites()) {

src/com/google/javascript/jscomp/NodeUtil.java

@@ -3061,6 +3061,13 @@ static boolean isFunctionExpression(Node n) {
     return n.isFunction() && (!isNamedFunction(n) || !isDeclarationParent(n.getParent()));
   }
 
+  /**
+   * @return Whether the node is both a function expression and the function is named.
+   */
+  static boolean isNamedFunctionExpression(Node n) {
+    return NodeUtil.isFunctionExpression(n) && !n.getFirstChild().getString().isEmpty();
+  }
+
   /**
    * see {@link #isFunctionExpression}
    *
@@ -3124,13 +3131,39 @@ static boolean isEmptyFunctionExpression(Node node) {
    * Determines if a function takes a variable number of arguments by
    * looking for references to the "arguments" var_args object.
    */
-  static boolean isVarArgsFunction(Node function) {
-    // TODO(johnlenz): rename this function
-    checkArgument(function.isFunction());
-    return !function.isArrowFunction() && isNameReferenced(
-        function.getLastChild(),
-        "arguments",
-        MATCH_NOT_THIS_BINDING);
+  @Deprecated
+  static boolean isVarArgsFunction(Node fn) {
+    return doesFunctionReferenceOwnArgumentsObject(fn);
+  }
+
+  /**
+   * @return Whether a function has a reference to its own "arguments" object.
+   */
+  static boolean doesFunctionReferenceOwnArgumentsObject(Node fn) {
+    checkArgument(fn.isFunction());
+    if (fn.isArrowFunction()) {
+      return false;
+    }
+    return referencesArgumentsHelper(fn.getLastChild());
+  }
+
+  /** @return Whether the predicate is true for the node or any of its descendants. */
+  private static boolean referencesArgumentsHelper(Node node) {
+    if (node.isName() && node.getString().equals("arguments")) {
+      return true;
+    }
+
+    if (NodeUtil.isVanillaFunction(node)) {
+      return false;
+    }
+
+    for (Node c = node.getFirstChild(); c != null; c = c.getNext()) {
+      if (referencesArgumentsHelper(c)) {
+        return true;
+      }
+    }
+
+    return false;
   }
 
   /**
@@ -4882,10 +4915,9 @@ static String getBestLValueName(@Nullable Node lValue) {
   }
 
   /**
-   * @return false iff the result of the expression is not consumed.
+   * @return true iff the result of the expression is consumed.
    */
   static boolean isExpressionResultUsed(Node expr) {
-    // TODO(johnlenz): consider sharing some code with trySimpleUnusedResult.
     Node parent = expr.getParent();
     switch (parent.getToken()) {
       case BLOCK:
@@ -4914,13 +4946,9 @@ static boolean isExpressionResultUsed(Node expr) {
 
         return (expr == parent.getFirstChild()) ? false : isExpressionResultUsed(parent);
       case FOR:
-      case FOR_IN:
-        if (!parent.isForIn()) {
-          // Only an expression whose result is in the condition part of the
-          // expression is used.
-          return (parent.getSecondChild() == expr);
-        }
-        break;
+        // Only an expression whose result is in the condition part of the
+        // expression is used.
+        return (parent.getSecondChild() == expr);
       default:
         break;
     }