Bug 565457 - CDB settings provider/parser's automatic exclusion of files is very slow

Implement a file exclusion algorithm that favors excluding whole folders when
possible.

The way it works is we gather exclusion information of each folder as we visit
each children. When "leaving" the folder, we can act on whether or not it can
be considered for exclusion as a whole or instead individually exclude a subset
of its children.

Using LLVM code base as a test:
Before: 613 sec
After: 2.4 sec

Change-Id: Ib882a72cae157e3db6b6c94a1a09cb6f05b66bc4
Signed-off-by: Marc-Andre Laperle <malaperle@gmail.com>

build/org.eclipse.cdt.managedbuilder.core.tests/tests/org/eclipse/cdt/managedbuilder/language/settings/providers/tests/CompilationDatabaseParserTest.java

@@ -18,6 +18,7 @@ import java.io.InputStream;
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 import org.eclipse.cdt.core.CCorePlugin;
@@ -35,6 +36,7 @@ import org.eclipse.cdt.core.settings.model.ICConfigurationDescription;
 import org.eclipse.cdt.core.settings.model.ICLanguageSettingEntry;
 import org.eclipse.cdt.core.settings.model.ICProjectDescription;
 import org.eclipse.cdt.core.settings.model.ICProjectDescriptionManager;
+import org.eclipse.cdt.core.settings.model.ICSourceEntry;
 import org.eclipse.cdt.core.settings.model.util.CDataUtil;
 import org.eclipse.cdt.core.testplugin.ResourceHelper;
 import org.eclipse.cdt.core.testplugin.util.BaseTestCase;
@@ -47,6 +49,7 @@ import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IFolder;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.NullProgressMonitor;
 import org.eclipse.core.runtime.Path;
 import org.eclipse.core.runtime.jobs.Job;
 
@@ -71,6 +74,7 @@ public class CompilationDatabaseParserTest extends BaseTestCase {
 	private IFile fOutsideCdbSourceFile;
 	private IProject fProject;
 	private IFolder fFolder;
+	private IFolder fFolderAllExcluded;
 
 	@Override
 	protected void setUp() throws Exception {
@@ -101,25 +105,27 @@ public class CompilationDatabaseParserTest extends BaseTestCase {
 			boolean haveCommandLine, boolean validCommandLine, boolean haveCommandArguments) throws Exception {
 		fProject = ResourceHelper.createCDTProjectWithConfig(getName());
 		fFolder = ResourceHelper.createFolder(fProject, "folder");
+		fFolderAllExcluded = ResourceHelper.createFolder(fProject, "folder-all-excluded");
+		IFolder subfolderAllExcluded = fFolderAllExcluded.getFolder("subfolder-all-excluded");
+		subfolderAllExcluded.create(true, true, new NullProgressMonitor());
 
-		IFile sourceFile = fFolder.getFile("test.cpp");
+		fSourceFile = fFolder.getFile("test.cpp");
-		if (sourceFile.exists()) {
+		fSourceFile2 = fProject.getFile("test.cpp");
-			sourceFile.delete(true, null);
+		fOutsideCdbSourceFile = fFolder.getFile("outside.cpp");
-		}
+		List<IFile> files = Arrays.asList(fSourceFile, fSourceFile2, fOutsideCdbSourceFile,
+				fFolderAllExcluded.getFile("file1.cpp"), fFolderAllExcluded.getFile("file2.cpp"),
+				subfolderAllExcluded.getFile("file3.cpp"));
 
-		IFile sourceFile2 = fProject.getFile("test.cpp");
+		files.forEach(file -> {
-		if (sourceFile2.exists()) {
+			try {
-			sourceFile2.delete(true, null);
+				if (file.exists()) {
-		}
+					file.delete(true, null);
-
+				}
-		fSourceFile = ResourceHelper.createFile(sourceFile, "//comment");
+				ResourceHelper.createFile(file, "//comment " + file.getLocation());
-		fSourceFile2 = ResourceHelper.createFile(sourceFile2, "//comment2");
+			} catch (CoreException e) {
-
+				throw new RuntimeException(e);
-		IFile outsideSourceFile = fFolder.getFile("outside.cpp");
+			}
-		if (outsideSourceFile.exists()) {
+		});
-			outsideSourceFile.delete(true, null);
-		}
-		fOutsideCdbSourceFile = ResourceHelper.createFile(outsideSourceFile, "//comment");
 
 		IFile file = fProject.getFile("compile_commands.json");
 		if (file.exists()) {
@@ -334,7 +340,13 @@ public class CompilationDatabaseParserTest extends BaseTestCase {
 		assertExpectedEntries(parser);
 
 		ICConfigurationDescription resCfgDescription = getConfigurationDescription(fProject, false);
-		assertTrue(CDataUtil.isExcluded(tu.getPath(), resCfgDescription.getSourceEntries()));
+		ICSourceEntry[] sourceEntries = resCfgDescription.getSourceEntries();
+		assertTrue(CDataUtil.isExcluded(tu.getPath(), sourceEntries));
+		assertTrue(CDataUtil.isExcluded(fFolderAllExcluded.getFullPath(), sourceEntries));
+		assertFalse(CDataUtil.isExcluded(fFolder.getFullPath(), sourceEntries));
+		assertFalse(CDataUtil.isExcluded(fProject.getFullPath(), sourceEntries));
+		assertFalse(CDataUtil.isExcluded(fSourceFile.getFullPath(), sourceEntries));
+		assertFalse(CDataUtil.isExcluded(fSourceFile2.getFullPath(), sourceEntries));
 	}
 
 	public void testParseCDB_NoBuildCommandParser() throws Exception {

build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/language/settings/providers/CompilationDatabaseParser.java

@@ -22,8 +22,10 @@ import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.nio.file.attribute.FileTime;
 import java.text.MessageFormat;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
+import java.util.Stack;
 
 import org.eclipse.cdt.core.CCorePlugin;
 import org.eclipse.cdt.core.cdtvariables.ICdtVariableManager;
@@ -54,6 +56,7 @@ import org.eclipse.core.resources.IResourceProxyVisitor;
 import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.resources.WorkspaceJob;
 import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Path;
@@ -130,14 +133,54 @@ public class CompilationDatabaseParser extends LanguageSettingsSerializableProvi
 		return lastModifiedTime.toMillis();
 	}
 
-	// Wanted to use this as a base to also count the number of translation unit
+	/**
-	// for the progress monitor but it's too slow so only use it to exclude for now.
+	 * Visit all folders and exclude all files that do not have entries coming from the CDB.
-	private abstract class SourceFilesVisitor implements ICElementVisitor {
+	 * The algorithm detects when whole folders can be excluded to prevent a large number of
+	 * individual file exclusions.
+	 */
+	private final class ExcludeSourceFilesVisitor implements ICElementVisitor {
+		private final ICConfigurationDescription cfgDescription;
+		ICSourceEntry[] entries = null;
+		private final IProgressMonitor monitor;
+		private final int sourceFilesCount;
+		private int nbChecked = 0;
+
+		// Keep track of excluded files and folders as we visit each folder in a depth-first manner.
+		private Stack<FolderExclusionInfo> folderExclusionInfos = new Stack<>();
+
+		private class FolderExclusionInfo {
+			// In case not all files are excluded in this folder, we need to keep track of which folders we'll exclude individually.
+			private ArrayList<IPath> childrenFoldersWithAllFilesExcluded = new ArrayList<>();
+			// In case not all files are excluded in this folder, we need to keep track of which files we'll exclude individually.
+			private ArrayList<IPath> excludedSourceFiles = new ArrayList<>();
+			// True if all children of the folder are excluded, recursively.
+			// Children folders can set this to false on their parent when non-excluded files are detected.
+			// Therefore, this value is reliable only when all children are visited.
+			private boolean allFilesExcluded = true;
+		}
+
+		//Note: monitor already has ticks allocated for number of source files (not considering exclusions though)
+		private ExcludeSourceFilesVisitor(IProgressMonitor monitor, int sourceFilesCount,
+				ICConfigurationDescription cfgDescription) {
+			this.monitor = monitor;
+			this.sourceFilesCount = sourceFilesCount;
+			this.cfgDescription = cfgDescription;
+			this.entries = cfgDescription.getSourceEntries();
+		}
+
+		public ICSourceEntry[] getSourceEntries() {
+			return entries;
+		}
+
 		@Override
 		public boolean visit(ICElement element) throws CoreException {
 			int elementType = element.getElementType();
 			if (elementType != ICElement.C_UNIT) {
-				return elementType == ICElement.C_CCONTAINER || elementType == ICElement.C_PROJECT;
+				boolean isSourceContainer = elementType == ICElement.C_CCONTAINER || elementType == ICElement.C_PROJECT;
+				if (isSourceContainer) {
+					folderExclusionInfos.push(new FolderExclusionInfo());
+				}
+				return isSourceContainer;
 			}
 
 			ITranslationUnit tu = (ITranslationUnit) element;
@@ -147,41 +190,16 @@ public class CompilationDatabaseParser extends LanguageSettingsSerializableProvi
 			return false;
 		}
 
-		abstract void handleTranslationUnit(ITranslationUnit tu) throws CoreException;
+		private void handleTranslationUnit(ITranslationUnit tu) throws CoreException {
-	}
+			FolderExclusionInfo folderInfo = folderExclusionInfos.peek();
-
+			List<ICLanguageSettingEntry> list = getSettingEntries(cfgDescription, tu.getResource(),
-	private final class ExcludeSourceFilesVisitor extends SourceFilesVisitor {
+					tu.getLanguage().getId());
-		private final ICConfigurationDescription cfgDescription;
+			if (list == null) {
-		ICSourceEntry[] entries = null;
+				folderInfo.excludedSourceFiles.add(tu.getResource().getFullPath());
-		private final IProgressMonitor monitor;
+			} else {
-		private final int sourceFilesCount;
+				folderInfo.allFilesExcluded = false;
-		private int nbChecked = 0;
-
-		//Note: monitor already has ticks allocated for number of source files (not considering exclusions though)
-		private ExcludeSourceFilesVisitor(IProgressMonitor monitor, int sourceFilesCount,
-				ICConfigurationDescription cfgDescription) {
-			this.monitor = monitor;
-			this.sourceFilesCount = sourceFilesCount;
-			this.cfgDescription = cfgDescription;
-		}
-
-		public ICSourceEntry[] getSourceEntries() {
-			return entries;
-		}
-
-		@Override
-		void handleTranslationUnit(ITranslationUnit tu) throws CoreException {
-			boolean isExcluded = CDataUtil.isExcluded(tu.getPath(), cfgDescription.getSourceEntries());
-			if (!isExcluded) {
-				List<ICLanguageSettingEntry> list = getSettingEntries(cfgDescription, tu.getResource(),
-						tu.getLanguage().getId());
-				if (list == null) {
-					if (entries == null) {
-						entries = cfgDescription.getSourceEntries();
-					}
-					entries = CDataUtil.setExcluded(tu.getResource().getFullPath(), false, true, entries);
-				}
 			}
+
 			monitor.worked(1);
 			if (nbChecked % 100 == 0) {
 				monitor.subTask(String.format(Messages.CompilationDatabaseParser_ProgressExcludingFiles, nbChecked,
@@ -189,6 +207,35 @@ public class CompilationDatabaseParser extends LanguageSettingsSerializableProvi
 			}
 			nbChecked++;
 		}
+
+		@Override
+		public void leave(ICElement element) throws CoreException {
+			int elementType = element.getElementType();
+			if (elementType == ICElement.C_CCONTAINER || elementType == ICElement.C_PROJECT) {
+
+				FolderExclusionInfo folderInfo = folderExclusionInfos.pop();
+
+				if (folderInfo.allFilesExcluded && !folderExclusionInfos.isEmpty()) {
+					// Consider this folder for exclusion later, maybe the parent will also be excluded
+					folderExclusionInfos.peek().childrenFoldersWithAllFilesExcluded.add(element.getPath());
+				} else {
+					if (!folderExclusionInfos.isEmpty()) {
+						folderExclusionInfos.peek().allFilesExcluded = false;
+					}
+
+					// Exclude all children folders previously considered for exclusion, since the parent
+					// (the currently visited element) cannot be excluded, we have to exclude them individually.
+					for (IPath excludedFolder : folderInfo.childrenFoldersWithAllFilesExcluded) {
+						entries = CDataUtil.setExcluded(excludedFolder, true, true, entries);
+					}
+
+					// Exclude all direct children files that need to be excluded.
+					for (IPath excludedFile : folderInfo.excludedSourceFiles) {
+						entries = CDataUtil.setExcluded(excludedFile, false, true, entries);
+					}
+				}
+			}
+		}
 	}
 
 	private static class CDBWorkingDirectoryTracker implements IWorkingDirectoryTracker {