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
diff --git a/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/internal/ExternalExecutablesState.java b/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/internal/ExternalExecutablesState.java
index 4b69253..3cec5ce 100644
--- a/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/internal/ExternalExecutablesState.java
+++ b/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 @@
private boolean enabled;
public ExternalExecutablesState() {
- List<Map<String, String>> externals = ExternalExecutablesManager.load();
- this.enabled = (externals != null && !externals.isEmpty());
+ this.enabled = ExternalExecutablesManager.hasEntries();
}
@Override
diff --git a/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/ExternalExecutablesManager.java b/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/ExternalExecutablesManager.java
index 466593d..54afd50 100644
--- a/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/ExternalExecutablesManager.java
+++ b/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/ExternalExecutablesManager.java
@@ -41,6 +41,37 @@
// 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 @@
}
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);
diff --git a/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/IDetectExternalExecutable.java b/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/IDetectExternalExecutable.java
index b49d5ff..4fd8083 100644
--- a/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/IDetectExternalExecutable.java
+++ b/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/IDetectExternalExecutable.java
@@ -15,9 +15,14 @@
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.
*
* This method is sometimes called in the UI thread when displaying context menus, so should
@@ -31,5 +36,6 @@
* 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);
+
}
diff --git a/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/detectors/DetectGitBash.java b/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/detectors/DetectGitBash.java
index 170f3f6..4d056b4 100644
--- a/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/detectors/DetectGitBash.java
+++ b/terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/detectors/DetectGitBash.java
@@ -29,7 +29,20 @@
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 @@
return Collections.emptyList();
}
- // If not found in the existing entries, check the path
- 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 not found in the existing entries, check the path, then
// if it is not found in the PATH, check the default install locations
- result = result.or(() -> {
- 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<File> result = getGitInstallDirectoryFromPATH().or(this::getGitInstallDirectory);
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 @@
}
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) {
