[431547] Bottleneck in SelectionEngine.select() when completing type bindingsv201404111521

The patch reduces the recursion usage when traversing/resolving/analysing as well as removes some exceeding (and unneeded)
cycling in visitors.

Signed-off-by: vrubezhny <vrubezhny@exadel.com>

3 files changed, 67 insertions, 24 deletions

diff --git a/bundles/org.eclipse.wst.jsdt.core/src/org/eclipse/wst/jsdt/internal/compiler/ast/AbstractMethodDeclaration.java b/bundles/org.eclipse.wst.jsdt.core/src/org/eclipse/wst/jsdt/internal/compiler/ast/AbstractMethodDeclaration.java
index f7360751d..d9bc26ceb 100644
--- a/bundles/org.eclipse.wst.jsdt.core/src/org/eclipse/wst/jsdt/internal/compiler/ast/AbstractMethodDeclaration.java
+++ b/bundles/org.eclipse.wst.jsdt.core/src/org/eclipse/wst/jsdt/internal/compiler/ast/AbstractMethodDeclaration.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2013 IBM Corporation and others.
+ * Copyright (c) 2000, 2014 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -680,7 +680,7 @@ public abstract class AbstractMethodDeclaration extends Statement
 					
 					return true;
 				}
-				
+
 				/**
 				 * @see org.eclipse.wst.jsdt.internal.compiler.ASTVisitor#visit(org.eclipse.wst.jsdt.internal.compiler.ast.LocalDeclaration, org.eclipse.wst.jsdt.internal.compiler.lookup.BlockScope)
 				 */
@@ -688,13 +688,29 @@ public abstract class AbstractMethodDeclaration extends Statement
 					if(scope != null && scope instanceof MethodScope) {
 						/* be sure to add all variable declarations
 						 * 
-						 * var b, c, d = "foo" */
-						AbstractVariableDeclaration currVarDecl = localDeclaration;
-						while(currVarDecl != null) {
-							((MethodScope) scope).addUnresolvedLocalVar(currVarDecl.getName(), currVarDecl);
-							
-							currVarDecl = currVarDecl.nextLocal;
+						 * var b, c, d = "foo" 
+						 * 
+						 * do nothing in case of localDeclaration is already added in order to prevent 
+						 * repeat on adding the same variables
+						 * (which means that all the variable declarations were added before
+						 *
+						MethodScope methodScope = (MethodScope)scope;
+						if (methodScope.getUnresolvedLocalVar(localDeclaration.getName()) == null) {
+							AbstractVariableDeclaration currVarDecl = localDeclaration;
+							while(currVarDecl != null) {
+								methodScope.addUnresolvedLocalVar(currVarDecl.getName(), currVarDecl);
+								currVarDecl = currVarDecl.nextLocal;
+							}
 						}
+						*/
+						/* 
+						 * No need to add all localDeclaration.nextLocal-s here, 
+						 * because it's done by LocalDeclaration.traverse() method 
+						 * (the only place that calls this visit method.
+						 * 
+						 * See: https://bugs.eclipse.org/bugs/show_bug.cgi?id=431547
+						 */  
+						((MethodScope)scope).addUnresolvedLocalVar(localDeclaration.getName(), localDeclaration);
 					}
 					
 					return true;
diff --git a/bundles/org.eclipse.wst.jsdt.core/src/org/eclipse/wst/jsdt/internal/compiler/ast/LocalDeclaration.java b/bundles/org.eclipse.wst.jsdt.core/src/org/eclipse/wst/jsdt/internal/compiler/ast/LocalDeclaration.java
index b3193b3f2..32dee8bcb 100644
--- a/bundles/org.eclipse.wst.jsdt.core/src/org/eclipse/wst/jsdt/internal/compiler/ast/LocalDeclaration.java
+++ b/bundles/org.eclipse.wst.jsdt.core/src/org/eclipse/wst/jsdt/internal/compiler/ast/LocalDeclaration.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2013 IBM Corporation and others.
+ * Copyright (c) 2000, 2014 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -61,6 +61,20 @@ public class LocalDeclaration extends AbstractVariableDeclaration implements ILo
 	}
 
 	public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, FlowInfo flowInfo) {
+		// Do not analyseCode for the nextLocal local variables in recursion
+		// because it may cause StackOverflowError
+		AbstractVariableDeclaration currVarDecl = this;
+		while (currVarDecl != null) {
+			if (currVarDecl instanceof LocalDeclaration) {
+				flowInfo = ((LocalDeclaration)currVarDecl).analyseCodeLocal(currentScope, flowContext, flowInfo);
+			}
+			currVarDecl = currVarDecl.nextLocal;
+		}
+		
+		return flowInfo;
+	}
+	
+	public FlowInfo analyseCodeLocal(BlockScope currentScope, FlowContext flowContext, FlowInfo flowInfo) {
 		// record variable initialization if any
 		if ((flowInfo.tagBits & FlowInfo.UNREACHABLE) == 0) {
 			// only set if actually reached
@@ -91,9 +105,6 @@ public class LocalDeclaration extends AbstractVariableDeclaration implements ILo
 			// no need to inform enclosing try block since its locals won't get
 			// known by the finally block
 		}
-		if (this.nextLocal != null) {
-			flowInfo = this.nextLocal.analyseCode(currentScope, flowContext, flowInfo);
-		}
 
 		return flowInfo;
 	}
@@ -134,14 +145,18 @@ public class LocalDeclaration extends AbstractVariableDeclaration implements ILo
 	}
 
 	public void resolve(BlockScope scope) {
-		resolve0(scope);
-		if (this.nextLocal != null) {
-			this.nextLocal.resolve(scope);
+		// Do not resolve the nextLocal local variables in recursion
+		// because it may cause StackOverflowError
+		AbstractVariableDeclaration currVarDecl = this;
+		while (currVarDecl != null) {
+			if (currVarDecl instanceof LocalDeclaration) {
+				((LocalDeclaration)currVarDecl).resolveLocal(scope);
+			}
+			currVarDecl = currVarDecl.nextLocal;
 		}
 	}
 
-
-	private void resolve0(BlockScope scope) {
+	public void resolveLocal(BlockScope scope) {
 		// create a binding and add it to the scope
 		TypeBinding variableType = resolveVarType(scope);
 
@@ -258,7 +273,18 @@ public class LocalDeclaration extends AbstractVariableDeclaration implements ILo
 	}
 
 	public void traverse(ASTVisitor visitor, BlockScope scope) {
-
+		// Do not traverse the nextLocal local variables in recursion
+		// because it may cause StackOverflowError
+		AbstractVariableDeclaration currVarDecl = this;
+		while (currVarDecl != null) {
+			if (currVarDecl instanceof LocalDeclaration) {
+				((LocalDeclaration)currVarDecl).traverseLocal(visitor, scope);
+			}
+			currVarDecl = currVarDecl.nextLocal;
+		}
+	}
+	
+	private void traverseLocal(ASTVisitor visitor, BlockScope scope) {
 		if (visitor.visit(this, scope)) {
 			if (type != null) {
 				type.traverse(visitor, scope);
@@ -273,9 +299,6 @@ public class LocalDeclaration extends AbstractVariableDeclaration implements ILo
 			}
 		}
 		visitor.endVisit(this, scope);
-		if (this.nextLocal != null) {
-			this.nextLocal.traverse(visitor, scope);
-		}
 	}
 
 	public String getTypeName() {
diff --git a/bundles/org.eclipse.wst.jsdt.core/src/org/eclipse/wst/jsdt/internal/compiler/lookup/MethodScope.java b/bundles/org.eclipse.wst.jsdt.core/src/org/eclipse/wst/jsdt/internal/compiler/lookup/MethodScope.java
index 3a5930ba7..df71321ce 100644
--- a/bundles/org.eclipse.wst.jsdt.core/src/org/eclipse/wst/jsdt/internal/compiler/lookup/MethodScope.java
+++ b/bundles/org.eclipse.wst.jsdt.core/src/org/eclipse/wst/jsdt/internal/compiler/lookup/MethodScope.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2013 IBM Corporation and others.
+ * Copyright (c) 2000, 2014 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -17,6 +17,7 @@ import org.eclipse.wst.jsdt.internal.compiler.ast.ASTNode;
 import org.eclipse.wst.jsdt.internal.compiler.ast.AbstractMethodDeclaration;
 import org.eclipse.wst.jsdt.internal.compiler.ast.Argument;
 import org.eclipse.wst.jsdt.internal.compiler.ast.ConstructorDeclaration;
+import org.eclipse.wst.jsdt.internal.compiler.ast.LocalDeclaration;
 import org.eclipse.wst.jsdt.internal.compiler.ast.ProgramElement;
 import org.eclipse.wst.jsdt.internal.compiler.ast.QualifiedNameReference;
 import org.eclipse.wst.jsdt.internal.compiler.ast.SingleNameReference;
@@ -53,7 +54,6 @@ public class MethodScope extends BlockScope {
 	public long[] definiteInits = new long[4];
 	public long[][] extraDefiniteInits = new long[4][];
 
-
 	public static final char [] ARGUMENTS_NAME={'a','r','g','u','m','e','n','t','s'};
 
 	public LocalVariableBinding argumentsBinding;
@@ -453,7 +453,11 @@ public class MethodScope extends BlockScope {
 			IAbstractVariableDeclaration statement = (IAbstractVariableDeclaration)this.fUnresolvedLocalVars.removeKey(variableName);
 			if(statement != null && statement instanceof ProgramElement) {
 				//resolve and then call super again
-				((ProgramElement)statement).resolve(this);
+				if (statement instanceof LocalDeclaration) {
+					((LocalDeclaration)statement).resolveLocal(this);
+				} else {
+					((ProgramElement)statement).resolve(this);
+				}
 				binding = super.findVariable(variableName);
 			}
 		}