From ca8e321a878df4720f4131f873e47b8d0f1eef47 Mon Sep 17 00:00:00 2001 From: aclement Date: Thu, 23 Jul 2009 19:50:23 +0000 Subject: [PATCH] 269652: incremental builds and prodding StatefulNameEnvironment --- .../internal/core/builder/AjBuildManager.java | 5 + .../ajdt/internal/core/builder/AjState.java | 110 +++++++++++------- .../core/builder/StatefulNameEnvironment.java | 76 ++++++------ 3 files changed, 113 insertions(+), 78 deletions(-) diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java index 202ff91bc..43a7dc0fe 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java +++ b/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; } } diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjState.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjState.java index d74394c4e..457d82cb7 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjState.java +++ b/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/* */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... diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/StatefulNameEnvironment.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/StatefulNameEnvironment.java index c4ce42d49..6167e7aaf 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/StatefulNameEnvironment.java +++ b/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; + } + } -- 2.39.5