From 7ca2be99f9e5729c7e0d9ea6c3e402a01d63cffa Mon Sep 17 00:00:00 2001 From: Fred Bricon Date: Wed, 17 Jun 2020 01:50:15 +0200 Subject: [PATCH] Optimize default VM management to avoid unnecessary project updates Signed-off-by: Fred Bricon --- .../jdt/ls/core/internal/JVMConfigurator.java | 99 ++++++++++--------- .../internal/handlers/BaseInitHandler.java | 2 +- .../internal/handlers/JDTLanguageServer.java | 2 +- .../org.eclipse.core.resources.prefs | 2 + .../.settings/org.eclipse.jdt.core.prefs | 8 ++ .../ls/core/internal/JVMConfiguratorTest.java | 4 +- 6 files changed, 68 insertions(+), 49 deletions(-) create mode 100644 org.eclipse.jdt.ls.tests.syntaxserver/.settings/org.eclipse.core.resources.prefs create mode 100644 org.eclipse.jdt.ls.tests.syntaxserver/.settings/org.eclipse.jdt.core.prefs diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JVMConfigurator.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JVMConfigurator.java index ad1992448a..49607f53e8 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JVMConfigurator.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JVMConfigurator.java @@ -52,55 +52,45 @@ */ public class JVMConfigurator implements IVMInstallChangedListener { - public static boolean configureDefaultVM(Preferences preferences) throws CoreException { - if (preferences == null) { + public static boolean configureDefaultVM(String javaHome) throws CoreException { + if (StringUtils.isBlank(javaHome)) { return false; } - String javaHome = preferences.getJavaHome(); - boolean changed = false; - if (javaHome != null) { - File jvmHome = new File(javaHome); - if (jvmHome.isDirectory()) { - IVMInstall vm = findVM(jvmHome, null); - if (vm == null) { - IVMInstallType installType = JavaRuntime.getVMInstallType(StandardVMType.ID_STANDARD_VM_TYPE); - long unique = System.currentTimeMillis(); - while (installType.findVMInstall(String.valueOf(unique)) != null) { - unique++; - } - String vmId = String.valueOf(unique); - VMStandin vmStandin = new VMStandin(installType, vmId); - String name = StringUtils.defaultIfBlank(jvmHome.getName(), "JRE"); - vmStandin.setName(name); - vmStandin.setInstallLocation(jvmHome); - vm = vmStandin.convertToRealVM(); - JDTUtils.setCompatibleVMs(vm.getId()); - changed = true; - } - boolean hasDefault = false; - for (RuntimeEnvironment runtime : preferences.getRuntimes()) { - if (runtime.isDefault()) { - hasDefault = true; - break; - } - } - if (!hasDefault) { - IVMInstall defaultVM = JavaRuntime.getDefaultVMInstall(); - File location = defaultVM.getInstallLocation(); - if (!location.equals(jvmHome)) { - JavaRuntime.setDefaultVMInstall(vm, new NullProgressMonitor()); - JDTUtils.setCompatibleVMs(vm.getId()); - changed = true; - } - } + File jvmHome = new File(javaHome); + if (jvmHome.isDirectory()) { + IVMInstall defaultVM = JavaRuntime.getDefaultVMInstall(); + if (defaultVM != null && jvmHome.equals(defaultVM.getInstallLocation())) { + return false; } + } else { + JavaLanguageServerPlugin.logInfo("java.home " + jvmHome + " is not a directory"); + return false; } - boolean jvmChanged = configureJVMs(preferences); - return changed || jvmChanged; + + IVMInstall vm = findVM(jvmHome, null); + if (vm == null) { + IVMInstallType installType = JavaRuntime.getVMInstallType(StandardVMType.ID_STANDARD_VM_TYPE); + long unique = System.currentTimeMillis(); + while (installType.findVMInstall(String.valueOf(unique)) != null) { + unique++; + } + String vmId = String.valueOf(unique); + VMStandin vmStandin = new VMStandin(installType, vmId); + String name = StringUtils.defaultIfBlank(jvmHome.getName(), "JRE"); + vmStandin.setName(name); + vmStandin.setInstallLocation(jvmHome); + vm = vmStandin.convertToRealVM(); + } + JavaLanguageServerPlugin.logInfo("Setting java.home " + jvmHome + " as default global VM"); + JavaRuntime.setDefaultVMInstall(vm, new NullProgressMonitor()); + JDTUtils.setCompatibleVMs(vm.getId()); + + return true; } public static boolean configureJVMs(Preferences preferences) throws CoreException { boolean changed = false; + boolean defaultVMSet = false; Set runtimes = preferences.getRuntimes(); for (RuntimeEnvironment runtime : runtimes) { if (runtime.isValid()) { @@ -123,13 +113,16 @@ public static boolean configureJVMs(Preferences preferences) throws CoreExceptio vmStandin = new VMStandin(vm); changed = changed || !runtime.getName().equals(vm.getName()) || !runtime.getInstallationFile().equals(vm.getInstallLocation()); } + IStatus status = installType.validateInstallLocation(file); if (!status.isOK()) { JavaLanguageServerPlugin.log(status); continue; } + vmStandin.setName(runtime.getName()); vmStandin.setInstallLocation(file); + if (sourcePath != null || javadocURL != null) { LibraryLocation[] libs; if (vm != null && vm.getLibraryLocations() != null) { @@ -157,8 +150,12 @@ public static boolean configureJVMs(Preferences preferences) throws CoreExceptio } vm = vmStandin.convertToRealVM(); if (runtime.isDefault()) { - JavaRuntime.setDefaultVMInstall(vm, new NullProgressMonitor()); - JavaLanguageServerPlugin.logInfo("Runtime " + runtime.getName() + " set as default"); + defaultVMSet = true; + if (!Objects.equals(vm, JavaRuntime.getDefaultVMInstall())) { + JavaLanguageServerPlugin.logInfo("Setting runtime " + runtime.getName() + "-" + runtime.getInstallationFile() + " as default global VM"); + JavaRuntime.setDefaultVMInstall(vm, new NullProgressMonitor()); + changed = true; + } } if (!setDefaultEnvironmentVM(vm, runtime.getName())) { JavaLanguageServerPlugin.logError("Runtime at '" + runtime.getPath() + "' is not compatible with the '" + runtime.getName() + "' environment"); @@ -168,6 +165,11 @@ public static boolean configureJVMs(Preferences preferences) throws CoreExceptio } } } + + if (!defaultVMSet) { + changed = configureDefaultVM(preferences.getJavaHome()) || changed; + } + if (changed) { JavaLanguageServerPlugin.logInfo("JVM Runtimes changed, saving new configuration"); JavaRuntime.saveVMConfiguration(); @@ -178,6 +180,9 @@ public static boolean configureJVMs(Preferences preferences) throws CoreExceptio private static boolean setDefaultEnvironmentVM(IVMInstall vm, String name) { IExecutionEnvironment environment = getExecutionEnvironment(name); if (environment != null) { + if (Objects.equals(vm, environment.getDefaultVM())) { + return true; + } IVMInstall[] compatibleVMs = environment.getCompatibleVMs(); for (IVMInstall compatibleVM : compatibleVMs) { if (compatibleVM.equals(vm)) { @@ -225,12 +230,16 @@ public void defaultVMInstallChanged(IVMInstall previous, IVMInstall current) { if (Objects.equals(previous, current)) { return; } + String prev = (previous == null) ? null : previous.getId() + "-" + previous.getInstallLocation(); + String curr = (current == null) ? null : current.getId() + "-" + current.getInstallLocation(); + + JavaLanguageServerPlugin.logInfo("Default VM Install changed from " + prev + " to " + curr); //Reset global compliance settings - Hashtable options = JavaCore.getOptions(); AbstractVMInstall jvm = (AbstractVMInstall) current; long jdkLevel = CompilerOptions.versionToJdkLevel(jvm.getJavaVersion()); String compliance = CompilerOptions.versionFromJdkLevel(jdkLevel); + Hashtable options = JavaCore.getOptions(); JavaCore.setComplianceOptions(compliance, options); JavaCore.setOptions(options); @@ -242,6 +251,8 @@ public void defaultVMInstallChanged(IVMInstall previous, IVMInstall current) { } ProjectsManager projectsManager = JavaLanguageServerPlugin.getProjectsManager(); if (projectsManager != null) { + //TODO Only trigger update if the project uses the default JVM + JavaLanguageServerPlugin.logInfo("defaultVMInstallChanged -> force update of " + project.getName()); projectsManager.updateProject(project, true); } } diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseInitHandler.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseInitHandler.java index 250c15a73b..cf2e9c62f5 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseInitHandler.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseInitHandler.java @@ -117,7 +117,7 @@ public InitializeResult initialize(InitializeParams param) { if (!isWorkspaceInitialized()) { // We don't care about triggering a full build here, like in onDidChangeConfiguration try { - JVMConfigurator.configureDefaultVM(prefs); + JVMConfigurator.configureJVMs(prefs); registerWorkspaceInitialized(); } catch (CoreException e) { JavaLanguageServerPlugin.logException("Failed to configure Java Runtimes", e); diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java index 53480f3b86..123c8d0682 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java @@ -428,7 +428,7 @@ public void didChangeConfiguration(DidChangeConfigurationParams params) { syncCapabilitiesToSettings(); boolean jvmChanged = false; try { - jvmChanged = jvmConfigurator.configureDefaultVM(preferenceManager.getPreferences()); + jvmChanged = jvmConfigurator.configureJVMs(preferenceManager.getPreferences()); } catch (Exception e) { JavaLanguageServerPlugin.logException(e.getMessage(), e); } diff --git a/org.eclipse.jdt.ls.tests.syntaxserver/.settings/org.eclipse.core.resources.prefs b/org.eclipse.jdt.ls.tests.syntaxserver/.settings/org.eclipse.core.resources.prefs new file mode 100644 index 0000000000..99f26c0203 --- /dev/null +++ b/org.eclipse.jdt.ls.tests.syntaxserver/.settings/org.eclipse.core.resources.prefs @@ -0,0 +1,2 @@ +eclipse.preferences.version=1 +encoding/=UTF-8 diff --git a/org.eclipse.jdt.ls.tests.syntaxserver/.settings/org.eclipse.jdt.core.prefs b/org.eclipse.jdt.ls.tests.syntaxserver/.settings/org.eclipse.jdt.core.prefs new file mode 100644 index 0000000000..9f6ece88bd --- /dev/null +++ b/org.eclipse.jdt.ls.tests.syntaxserver/.settings/org.eclipse.jdt.core.prefs @@ -0,0 +1,8 @@ +eclipse.preferences.version=1 +org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled +org.eclipse.jdt.core.compiler.codegen.targetPlatform=1.8 +org.eclipse.jdt.core.compiler.compliance=1.8 +org.eclipse.jdt.core.compiler.problem.assertIdentifier=error +org.eclipse.jdt.core.compiler.problem.enumIdentifier=error +org.eclipse.jdt.core.compiler.release=disabled +org.eclipse.jdt.core.compiler.source=1.8 diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/JVMConfiguratorTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/JVMConfiguratorTest.java index a2e3626e75..a827ce7404 100644 --- a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/JVMConfiguratorTest.java +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/JVMConfiguratorTest.java @@ -71,10 +71,8 @@ public void cleanUp() throws CoreException { @Test public void testDefaultVM() throws CoreException { - Preferences prefs = new Preferences(); String javaHome = new File(TestVMType.getFakeJDKsLocation(), "9").getAbsolutePath(); - prefs.setJavaHome(javaHome); - boolean changed = JVMConfigurator.configureDefaultVM(prefs); + boolean changed = JVMConfigurator.configureDefaultVM(javaHome); IVMInstall newDefaultVM = JavaRuntime.getDefaultVMInstall(); assertTrue("A VM hasn't been changed", changed); assertNotEquals(originalVm, newDefaultVM);