269652: incremental builds and prodding StatefulNameEnvironment

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java

@@ -80,6 +80,7 @@ import org.aspectj.org.eclipse.jdt.internal.compiler.batch.FileSystem;
 import org.aspectj.org.eclipse.jdt.internal.compiler.env.ICompilationUnit;
 import org.aspectj.org.eclipse.jdt.internal.compiler.env.INameEnvironment;
 import org.aspectj.org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
+import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment;
 import org.aspectj.org.eclipse.jdt.internal.compiler.parser.Parser;
 import org.aspectj.org.eclipse.jdt.internal.compiler.problem.AbortCompilation;
 import org.aspectj.org.eclipse.jdt.internal.compiler.problem.DefaultProblemFactory;
@@ -990,6 +991,9 @@ public class AjBuildManager implements IOutputClassFileNameProvider, IBinarySour
 			}
 			environment = new StatefulNameEnvironment(getLibraryAccess(classpaths, filenames), state.getClassNameToFileMap(), state);
 			state.setNameEnvironment(environment);
+		} else {
+			((StatefulNameEnvironment) environment).update(state.getClassNameToFileMap(), state.deltaAddedClasses);
+			state.deltaAddedClasses.clear();
 		}
 
 		org.aspectj.ajdt.internal.compiler.CompilerAdapter.setCompilerAdapterFactory(this);
@@ -1015,6 +1019,7 @@ public class AjBuildManager implements IOutputClassFileNameProvider, IBinarySour
 		if (environment != null) {
 			environment.cleanup();
 			environment = null;
+			// le = null;
 		}
 	}
 

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjState.java

@@ -168,6 +168,9 @@ public class AjState implements CompilerConfigurationChangeFlags {
 	 */
 	private final Map/* <String, File> */classesFromName = new HashMap();
 
+	// For a particular build run, this set records the changes to classesFromName
+	public final Set deltaAddedClasses = new HashSet();
+
 	/**
 	 * Populated by AjBuildManager to record the aspects with the file name in which they're contained. This is later used when
 	 * writing the outxml file in AjBuildManager. Need to record the file name because want to write an outxml file for each of the
@@ -410,7 +413,7 @@ public class AjState implements CompilerConfigurationChangeFlags {
 			return pathname.getName().endsWith(".class");
 		}
 	};
-	
+
 	private void recordDecision(String decision) {
 		getListener().recordDecision(decision);
 	}
@@ -425,7 +428,7 @@ public class AjState implements CompilerConfigurationChangeFlags {
 
 		if (!dir.isDirectory()) {
 			if (listenerDefined()) {
-				recordDecision("ClassFileChangeChecking: not a directory so forcing full build: '"+dir.getPath()+"'");
+				recordDecision("ClassFileChangeChecking: not a directory so forcing full build: '" + dir.getPath() + "'");
 			}
 			return CLASS_FILE_CHANGED_THAT_NEEDS_FULL_BUILD;
 		}
@@ -439,7 +442,6 @@ public class AjState implements CompilerConfigurationChangeFlags {
 				recordDecision("ClassFileChangeChecking: failed to find a state instance managing output location : " + dir);
 			}
 		}
-		
 
 		// pr268827 - this guard will cause us to exit quickly if the state says there really is
 		// nothing of interest. This will not catch the case where a user modifies the .class files outside of
@@ -451,19 +453,21 @@ public class AjState implements CompilerConfigurationChangeFlags {
 			}
 			return CLASS_FILE_NO_CHANGES;
 		}
-		
+
 		if (state == null) {
 			// This may be because the directory is the output path of a Java project upon which we depend
 			// we need to call back into AJDT to ask about that projects state.
 			CompilationResultDestinationManager crdm = buildConfig.getCompilationResultDestinationManager();
-			if (crdm!=null) {
-				int i = crdm.discoverChangesSince(dir,lastSuccessfulBuildTime);
+			if (crdm != null) {
+				int i = crdm.discoverChangesSince(dir, lastSuccessfulBuildTime);
 				// 0 = dontknow if it has changed
 				// 1 = definetly not changed at all
 				// further numbers can determine more granular changes
-				if (i==1) {
+				if (i == 1) {
 					if (listenerDefined()) {
-						getListener().recordDecision("ClassFileChangeChecking: queried JDT and '"+dir+"' is apparently unchanged so not performing timestamp check");
+						getListener().recordDecision(
+								"ClassFileChangeChecking: queried JDT and '" + dir
+										+ "' is apparently unchanged so not performing timestamp check");
 					}
 					return CLASS_FILE_NO_CHANGES;
 				}
@@ -475,34 +479,40 @@ public class AjState implements CompilerConfigurationChangeFlags {
 		for (Iterator iterator = classFiles.iterator(); iterator.hasNext();) {
 			File classFile = (File) iterator.next();
 			if (CHECK_STATE_FIRST && state != null) {
-				// Next section reworked based on bug 270033: 
+				// Next section reworked based on bug 270033:
 				// if it is an aspect we may or may not be in trouble depending on whether (a) we depend on it (b) it is on the
 				// classpath or the aspectpath
 				if (state.isAspect(classFile)) {
-					if (state.hasStructuralChangedSince(classFile, lastSuccessfulBuildTime) ||
-						isTypeWeReferTo(classFile)) {     
-						// further improvements possible    
+					if (state.hasStructuralChangedSince(classFile, lastSuccessfulBuildTime) || isTypeWeReferTo(classFile)) {
+						// further improvements possible
 						if (listenerDefined()) {
-							getListener().recordDecision("ClassFileChangeChecking: aspect found that has structurally changed or that this project depends upon : " + classFile);
+							getListener().recordDecision(
+									"ClassFileChangeChecking: aspect found that has structurally changed or that this project depends upon : "
+											+ classFile);
 						}
 						return CLASS_FILE_CHANGED_THAT_NEEDS_FULL_BUILD;
 					} else {
-					    // it is an aspect but we don't refer to it: 
-					    // - for CLASSPATH I think this is OK, we can continue and try an
-					    //   incremental build
-					    // - for ASPECTPATH we don't know what else might be touched in this project
-					    //   and must rebuild
-						if (pathid==PATHID_CLASSPATH) {
+						// it is an aspect but we don't refer to it:
+						// - for CLASSPATH I think this is OK, we can continue and try an
+						// incremental build
+						// - for ASPECTPATH we don't know what else might be touched in this project
+						// and must rebuild
+						if (pathid == PATHID_CLASSPATH) {
 							if (listenerDefined()) {
-								getListener().recordDecision("ClassFileChangeChecking: found aspect on classpath but this project doesn't reference it, continuing to try for incremental build : " + classFile);
-							}							
+								getListener()
+										.recordDecision(
+												"ClassFileChangeChecking: found aspect on classpath but this project doesn't reference it, continuing to try for incremental build : "
+														+ classFile);
+							}
 						} else {
 							if (listenerDefined()) {
-								getListener().recordDecision("ClassFileChangeChecking: found aspect on aspectpath/inpath - can't determine if this project is affected, must full build: " + classFile);
+								getListener().recordDecision(
+										"ClassFileChangeChecking: found aspect on aspectpath/inpath - can't determine if this project is affected, must full build: "
+												+ classFile);
 							}
 							return CLASS_FILE_CHANGED_THAT_NEEDS_FULL_BUILD;
 						}
-					  }
+					}
 
 				}
 				if (state.hasStructuralChangedSince(classFile, lastSuccessfulBuildTime)) {
@@ -524,39 +534,47 @@ public class AjState implements CompilerConfigurationChangeFlags {
 					// structurally changed or not
 					if (state != null) {
 						if (state.isAspect(classFile)) {
-							if (state.hasStructuralChangedSince(classFile, lastSuccessfulBuildTime) ||
-								      isTypeWeReferTo(classFile)) {     
-								// further improvements possible                      
+							if (state.hasStructuralChangedSince(classFile, lastSuccessfulBuildTime) || isTypeWeReferTo(classFile)) {
+								// further improvements possible
 								if (listenerDefined()) {
-									getListener().recordDecision("ClassFileChangeChecking: aspect found that has structurally changed or that this project depends upon : " + classFile);
+									getListener().recordDecision(
+											"ClassFileChangeChecking: aspect found that has structurally changed or that this project depends upon : "
+													+ classFile);
 								}
 								return CLASS_FILE_CHANGED_THAT_NEEDS_FULL_BUILD;
 							} else {
-							    // it is an aspect but we don't refer to it: 
-							    // - for CLASSPATH I think this is OK, we can continue and try an
-							    //   incremental build
-							    // - for ASPECTPATH we don't know what else might be touched in this project
-							    //   and must rebuild
-								if (pathid==PATHID_CLASSPATH) {
+								// it is an aspect but we don't refer to it:
+								// - for CLASSPATH I think this is OK, we can continue and try an
+								// incremental build
+								// - for ASPECTPATH we don't know what else might be touched in this project
+								// and must rebuild
+								if (pathid == PATHID_CLASSPATH) {
 									if (listenerDefined()) {
-										getListener().recordDecision("ClassFileChangeChecking: found aspect on classpath but this project doesn't reference it, continuing to try for incremental build : " + classFile);
+										getListener()
+												.recordDecision(
+														"ClassFileChangeChecking: found aspect on classpath but this project doesn't reference it, continuing to try for incremental build : "
+																+ classFile);
 									}
 								} else {
 									if (listenerDefined()) {
-										getListener().recordDecision("ClassFileChangeChecking: found aspect on aspectpath/inpath - can't determine if this project is affected, must full build: " + classFile);
+										getListener().recordDecision(
+												"ClassFileChangeChecking: found aspect on aspectpath/inpath - can't determine if this project is affected, must full build: "
+														+ classFile);
 									}
 									return CLASS_FILE_CHANGED_THAT_NEEDS_FULL_BUILD;
 								}
-							  }
+							}
 						}
 						if (state.hasStructuralChangedSince(classFile, lastSuccessfulBuildTime)) {
 							if (listenerDefined()) {
-								getListener().recordDecision("ClassFileChangeChecking: structural change detected in : " + classFile);
+								getListener().recordDecision(
+										"ClassFileChangeChecking: structural change detected in : " + classFile);
 							}
 							isTypeWeReferTo(classFile);
 						} else {
 							if (listenerDefined())
-								getListener().recordDecision("ClassFileChangeChecking: change detected in " + classFile + " but it is not structural");
+								getListener().recordDecision(
+										"ClassFileChangeChecking: change detected in " + classFile + " but it is not structural");
 						}
 					} else {
 						// No state object to ask, so it only matters if we know which type depends on this file
@@ -714,11 +732,11 @@ public class AjState implements CompilerConfigurationChangeFlags {
 				}
 				newlyAffectedFiles++;
 				// possibly the beginnings of addressing the second point in 270033 comment 3
-//				List/*ClassFile*/ cfs = (List)this.fullyQualifiedTypeNamesResultingFromCompilationUnit.get(entry.getKey());
+				// List/*ClassFile*/ cfs = (List)this.fullyQualifiedTypeNamesResultingFromCompilationUnit.get(entry.getKey());
 				affectedFiles.add(entry.getKey());
 			}
 		}
-		if (newlyAffectedFiles>0) {
+		if (newlyAffectedFiles > 0) {
 			return true;
 		}
 		if (listenerDefined()) {
@@ -794,7 +812,7 @@ public class AjState implements CompilerConfigurationChangeFlags {
 		}
 		return (this.lastSuccessfulFullBuildTime > lastSuccessfulBuildTime);
 	}
-	
+
 	static int PATHID_CLASSPATH = 0;
 	static int PATHID_ASPECTPATH = 1;
 	static int PATHID_INPATH = 2;
@@ -954,7 +972,7 @@ public class AjState implements CompilerConfigurationChangeFlags {
 				if (!foundMatch) {
 					if (!alreadyAnalysedPaths.contains(f.getAbsolutePath())) { // Do not check paths more than once
 						alreadyAnalysedPaths.add(f.getAbsolutePath());
-						int classFileChanges = classFileChangedInDirSinceLastBuildRequiringFullBuild(f,pathid);
+						int classFileChanges = classFileChangedInDirSinceLastBuildRequiringFullBuild(f, pathid);
 						if (classFileChanges == CLASS_FILE_CHANGED_THAT_NEEDS_FULL_BUILD) {
 							return true;
 						}
@@ -1006,7 +1024,7 @@ public class AjState implements CompilerConfigurationChangeFlags {
 				if (!foundMatch) {
 					if (!alreadyAnalysedPaths.contains(f.getAbsolutePath())) { // Do not check paths more than once
 						alreadyAnalysedPaths.add(f.getAbsolutePath());
-						int classFileChanges = classFileChangedInDirSinceLastBuildRequiringFullBuild(f,PATHID_CLASSPATH);
+						int classFileChanges = classFileChangedInDirSinceLastBuildRequiringFullBuild(f, PATHID_CLASSPATH);
 						if (classFileChanges == CLASS_FILE_CHANGED_THAT_NEEDS_FULL_BUILD)
 							return true;
 					}
@@ -1269,7 +1287,11 @@ public class AjState implements CompilerConfigurationChangeFlags {
 		for (int i = 0; i < unwovenClassFiles.length; i++) {
 			File lastTimeRound = (File) classesFromName.get(unwovenClassFiles[i].getClassName());
 			recordClassFile(unwovenClassFiles[i], lastTimeRound);
-			classesFromName.put(unwovenClassFiles[i].getClassName(), new File(unwovenClassFiles[i].getFilename()));
+			String name = unwovenClassFiles[i].getClassName();
+			if (lastTimeRound == null) {
+				deltaAddedClasses.add(name);
+			}
+			classesFromName.put(name, new File(unwovenClassFiles[i].getFilename()));
 		}
 
 		// need to do this before types are deleted from the World...

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/StatefulNameEnvironment.java

@@ -10,7 +10,6 @@
  *     PARC     initial implementation 
  * ******************************************************************/
 
-
 package org.aspectj.ajdt.internal.core.builder;
 
 //import java.util.HashMap;
@@ -31,94 +30,103 @@ import org.aspectj.org.eclipse.jdt.internal.compiler.env.INameEnvironment;
 import org.aspectj.org.eclipse.jdt.internal.compiler.env.NameEnvironmentAnswer;
 import org.aspectj.util.FileUtil;
 
-
 public class StatefulNameEnvironment implements INameEnvironment {
 	private Map classesFromName;
 	private Map inflatedClassFilesCache;
 	private Set packageNames;
 	private AjState state;
 	private INameEnvironment baseEnvironment;
-	
+
 	public StatefulNameEnvironment(INameEnvironment baseEnvironment, Map classesFromName, AjState state) {
 		this.classesFromName = classesFromName;
 		this.inflatedClassFilesCache = new HashMap();
 		this.baseEnvironment = baseEnvironment;
-		this.state= state;
-		
+		this.state = state;
+
 		packageNames = new HashSet();
-		for (Iterator i = classesFromName.keySet().iterator(); i.hasNext(); ) {
-			String className = (String)i.next();
+		for (Iterator i = classesFromName.keySet().iterator(); i.hasNext();) {
+			String className = (String) i.next();
 			addAllPackageNames(className);
 		}
-//		System.err.println(packageNames);
+		// System.err.println(packageNames);
 	}
 
 	private void addAllPackageNames(String className) {
 		int dot = className.indexOf('.');
 		while (dot != -1) {
 			packageNames.add(className.substring(0, dot));
-			dot = className.indexOf('.', dot+1);
+			dot = className.indexOf('.', dot + 1);
 		}
 	}
 
 	public void cleanup() {
 		baseEnvironment.cleanup();
 		this.classesFromName = Collections.EMPTY_MAP;
-		this.packageNames = Collections.EMPTY_SET;
+		this.packageNames.clear();// = Collections.EMPTY_SET;
 	}
 
 	private NameEnvironmentAnswer findType(String name) {
-	    // pr133532 - ask the state for the type first
+		// pr133532 - ask the state for the type first
 		IBinaryType seenOnPreviousBuild = state.checkPreviousBuild(name);
-		if (seenOnPreviousBuild!=null) {
-			return new NameEnvironmentAnswer(seenOnPreviousBuild,null);
+		if (seenOnPreviousBuild != null) {
+			return new NameEnvironmentAnswer(seenOnPreviousBuild, null);
 		}
 		if (this.inflatedClassFilesCache.containsKey(name)) {
 			return (NameEnvironmentAnswer) this.inflatedClassFilesCache.get(name);
-		}
-		else {
-			File fileOnDisk = (File)classesFromName.get(name);
-			//System.err.println("find: " + name + " found: " + cf);
-			
-			if (fileOnDisk == null) return null;
-	
+		} else {
+			File fileOnDisk = (File) classesFromName.get(name);
+			// System.err.println("find: " + name + " found: " + cf);
+
+			if (fileOnDisk == null)
+				return null;
+
 			try {
-				//System.out.println("from cache: " + name);
+				// System.out.println("from cache: " + name);
 				byte[] bytes = FileUtil.readAsByteArray(fileOnDisk);
-				NameEnvironmentAnswer ret = 
-					new NameEnvironmentAnswer(
-						new ClassFileReader(bytes, fileOnDisk.getAbsolutePath().toCharArray()),
-						null /* no access restriction */);
-				this.inflatedClassFilesCache.put(name,ret);
+				NameEnvironmentAnswer ret = new NameEnvironmentAnswer(new ClassFileReader(bytes, fileOnDisk.getAbsolutePath()
+						.toCharArray()), null /* no access restriction */);
+				this.inflatedClassFilesCache.put(name, ret);
 				return ret;
 			} catch (ClassFormatException e) {
-				return null; //!!! seems to match FileSystem behavior
+				return null; // !!! seems to match FileSystem behavior
 			} catch (IOException ex) {
 				return null; // see above...
 			}
 		}
 	}
 
-	public NameEnvironmentAnswer findType(
-		char[] typeName,
-		char[][] packageName)
-	{
+	public NameEnvironmentAnswer findType(char[] typeName, char[][] packageName) {
 		NameEnvironmentAnswer ret = findType(new String(CharOperation.concatWith(packageName, typeName, '.')));
-		if (ret != null) return ret;
+		if (ret != null)
+			return ret;
 		return baseEnvironment.findType(typeName, packageName);
 	}
 
 	public NameEnvironmentAnswer findType(char[][] compoundName) {
 		NameEnvironmentAnswer ret = findType(new String(CharOperation.concatWith(compoundName, '.')));
-		if (ret != null) return ret;
+		if (ret != null)
+			return ret;
 		return baseEnvironment.findType(compoundName);
 	}
 
 	public boolean isPackage(char[][] parentPackageName, char[] packageName) {
-		if (baseEnvironment.isPackage(parentPackageName, packageName)) return true;
+		if (baseEnvironment.isPackage(parentPackageName, packageName))
+			return true;
 
 		String fullPackageName = new String(CharOperation.concatWith(parentPackageName, packageName, '.'));
 		return packageNames.contains(fullPackageName);
 	}
 
+	/**
+	 * Needs to be told about changes. The 'added' set is a subset of classNameToFileMap consisting of just those names added during
+	 * this build - to reduce any impact on incremental compilation times.
+	 */
+	public void update(Map classNameToFileMap, Set added) {
+		for (Iterator i = added.iterator(); i.hasNext();) {
+			String className = (String) i.next();
+			addAllPackageNames(className);
+		}
+		this.classesFromName = classNameToFileMap;
+	}
+
 }