Bug 573712: Reduce work done at Eclipse startup time on main thread

In Bug 466650, the git bash detector was removed from startup to reduce
overhead on startup. Later during a rewrite it was added back in under
a different code path for Bug 473107. In the context of Bug 573712
which is going to add more detectors that may do much more work on
a full load, reduce the amount of work needed to do the presence check.

Change-Id: If1ae3f12ec51b1edc2d419f0efd89fed81a7b56e

terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/internal/ExternalExecutablesState.java

@@ -12,7 +12,6 @@
 package org.eclipse.tm.terminal.view.ui.internal;
 
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 
 import org.eclipse.tm.terminal.view.ui.local.showin.ExternalExecutablesManager;
@@ -27,8 +26,7 @@ public class ExternalExecutablesState extends AbstractSourceProvider {
 	private boolean enabled;
 
 	public ExternalExecutablesState() {
-		List<Map<String, String>> externals = ExternalExecutablesManager.load();
+		this.enabled = ExternalExecutablesManager.hasEntries();
-		this.enabled = (externals != null && !externals.isEmpty());
 	}
 
 	@Override

terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/ExternalExecutablesManager.java

@@ -41,6 +41,37 @@ public class ExternalExecutablesManager {
 	// XXX: This may make a useful extension point?
 	private static List<IDetectExternalExecutable> detectors = List.of(new DetectGitBash());
 
+	public static boolean hasEntries() {
+		IPath stateLocation = UIPlugin.getDefault().getStateLocation();
+		if (stateLocation != null) {
+			File f = stateLocation.append(".executables/data.properties").toFile(); //$NON-NLS-1$
+			if (f.canRead()) {
+
+				try (FileReader r = new FileReader(f)) {
+					Properties data = new Properties();
+					data.load(r);
+					if (!data.isEmpty()) {
+						return true;
+					}
+				} catch (IOException e) {
+					if (Platform.inDebugMode()) {
+						e.printStackTrace();
+					}
+				}
+			}
+		}
+
+		// There are not any saved entries - run the detectors and if any of them
+		// have entries, then we can stop.
+		for (IDetectExternalExecutable iDetectExternalExecutable : detectors) {
+			if (iDetectExternalExecutable.hasEntries()) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
 	/**
 	 * Loads the list of all saved external executables.
 	 *
@@ -111,7 +142,7 @@ public class ExternalExecutablesManager {
 		}
 
 		var readOnly = Collections.unmodifiableList(externalExecutables);
-		var detected = detectors.stream().flatMap(detector -> detector.run(readOnly).stream())
+		var detected = detectors.stream().flatMap(detector -> detector.getEntries(readOnly).stream())
 				.collect(Collectors.toList());
 		if (!detected.isEmpty()) {
 			externalExecutables.addAll(detected);

terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/IDetectExternalExecutable.java

@@ -15,8 +15,13 @@ import java.util.Map;
 
 import org.eclipse.tm.terminal.view.ui.interfaces.IExternalExecutablesProperties;
 
-@FunctionalInterface
 public interface IDetectExternalExecutable {
+	/**
+	 * Detect if {@link #getEntries(List)} will return a non-empty list of entries, assuming externalEntries is an empty
+	 * list. This method is used during critical UI times (such as startup) and should be very fast.
+	 */
+	boolean hasEntries();
+
 	/**
 	 * Detect any additional external executables that can be added to the Show In list.
 	 *
@@ -31,5 +36,6 @@ public interface IDetectExternalExecutable {
 	 * that match {@link IExternalExecutablesProperties}. Must not return <code>null</code>, return
 	 * an empty list instead.
 	 */
-	List<Map<String, String>> run(List<Map<String, String>> externalExecutables);
+	List<Map<String, String>> getEntries(List<Map<String, String>> externalExecutables);
+
 }

terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/detectors/DetectGitBash.java

@@ -29,7 +29,20 @@ public class DetectGitBash implements IDetectExternalExecutable {
 	private static boolean gitBashSearchDone = false;
 
 	@Override
-	public List<Map<String, String>> run(List<Map<String, String>> externalExecutables) {
+	public boolean hasEntries() {
+		if (Platform.OS_WIN32.equals(Platform.getOS())) {
+			// the order of operations here is different than getEntries because we just
+			// want to know if anything can match. We avoid iterating over the whole PATH
+			// if the user has git installed in the default location. This gets us the
+			// correct answer to hasEntries quickly without having to iterate over the whole
+			// PATH (especially if PATH is long!).
+			return getGitInstallDirectory().or(this::getGitInstallDirectoryFromPATH).isPresent();
+		}
+		return false;
+	}
+
+	@Override
+	public List<Map<String, String>> getEntries(List<Map<String, String>> externalExecutables) {
 		// Lookup git bash (Windows Hosts only)
 		if (!gitBashSearchDone && Platform.OS_WIN32.equals(Platform.getOS())) {
 			// Do not search again for git bash while the session is running
@@ -42,29 +55,9 @@ public class DetectGitBash implements IDetectExternalExecutable {
 				return Collections.emptyList();
 			}
 
-			// If not found in the existing entries, check the path
+			// If not found in the existing entries, check the path, then
-			Optional<File> result = ExternalExecutablesUtils.visitPATH(entry -> {
-				File f = new File(entry, "git.exe"); //$NON-NLS-1$
-				if (f.canRead()) {
-					File check = f.getParentFile().getParentFile();
-					if (new File(check, "bin/sh.exe").canExecute()) { //$NON-NLS-1$
-						return Optional.of(check);
-					}
-				}
-				return Optional.empty();
-			});
 			// if it is not found in the PATH, check the default install locations
-			result = result.or(() -> {
+			Optional<File> result = getGitInstallDirectoryFromPATH().or(this::getGitInstallDirectory);
-				File f = new File("C:/Program Files (x86)/Git/bin/sh.exe"); //$NON-NLS-1$
-				if (!f.exists()) {
-					f = new File("C:/Program Files/Git/bin/sh.exe"); //$NON-NLS-1$
-				}
-				if (f.exists() && f.canExecute()) {
-					return Optional.of(f.getParentFile().getParentFile());
-				}
-				return Optional.empty();
-			});
-
 			Optional<String> gitPath = result.map(f -> new File(f, "bin/sh.exe").getAbsolutePath()); //$NON-NLS-1$
 			Optional<String> iconPath = result.flatMap(f -> getGitIconPath(f));
 
@@ -81,7 +74,33 @@ public class DetectGitBash implements IDetectExternalExecutable {
 
 		}
 		return Collections.emptyList();
+	}
 
+	private Optional<File> getGitInstallDirectoryFromPATH() {
+		return ExternalExecutablesUtils.visitPATH(entry -> {
+			File f = new File(entry, "git.exe"); //$NON-NLS-1$
+			if (f.canRead()) {
+				File check = f.getParentFile().getParentFile();
+				if (new File(check, "bin/sh.exe").canExecute()) { //$NON-NLS-1$
+					return Optional.of(check);
+				}
+			}
+			return Optional.empty();
+		});
+	}
+
+	private Optional<File> getGitInstallDirectory() {
+		// If 32-bit and 64-bit are both present, prefer the 64-bit one
+		// as it is probably newer and more likely to be present
+		// for the fast check required by hasEntries
+		File f = new File("C:/Program Files/Git/bin/sh.exe"); //$NON-NLS-1$
+		if (!f.exists()) {
+			f = new File("C:/Program Files (x86)/Git/bin/sh.exe"); //$NON-NLS-1$
+		}
+		if (f.exists() && f.canExecute()) {
+			return Optional.of(f.getParentFile().getParentFile());
+		}
+		return Optional.empty();
 	}
 
 	private static Optional<String> getGitIconPath(File parent) {