From ae8ad775618cb89ad10db8f92f30e677a40bce4e Mon Sep 17 00:00:00 2001 From: Dmitry Trofimov Date: Thu, 19 Nov 2015 19:09:27 +0100 Subject: [PATCH] Commit roots only once after all changes are applied (PY-17670) Commits of roots causes performance problems, let's commit them only once after everything is set up. It is important to have only one instance of sdkModifier at a time as it is not thread safe. Also we should assert that no changes are permormed to it after commit. --- .../jetbrains/python/sdk/PySdkUpdater.java | 84 +++++++++++++++++-- .../jetbrains/python/sdk/PythonSdkType.java | 34 +++++--- .../python/sdk/PythonSdkUpdater.java | 3 +- 3 files changed, 99 insertions(+), 22 deletions(-) diff --git a/python/src/com/jetbrains/python/sdk/PySdkUpdater.java b/python/src/com/jetbrains/python/sdk/PySdkUpdater.java index 01728cd0b306..cab4fd63a638 100644 --- a/python/src/com/jetbrains/python/sdk/PySdkUpdater.java +++ b/python/src/com/jetbrains/python/sdk/PySdkUpdater.java @@ -28,18 +28,19 @@ import org.jetbrains.annotations.Nullable; * Working with sdk instance instead of this class can be wrong, because an instance can become * obsolete being substituted in sdk table by a new one. Or already created sdk modificator can be committed, * discarding the changes that we doing with the sdk. - * - * + *

+ *

* There are two ways of creation of the facade: - * 1) by sdk path - in this case we'll get the current actual sdk instance + * 1) by sdk path - in this case we'll get the current actual sdk instance * from sdk table by that path, creating and committing SdkModificator on every change - * 2) by sdkModificator - in that case we'll make changes to that modificator, but it is not committed, - * because it has been created outside of the updater. - * + * 2) by sdkModificator - in that case we'll make changes to that modificator, but it is not committed, + * because it has been created outside of the updater. * * @author traff */ public abstract class PySdkUpdater { + private static PySdkUpdater mySingletonUpdater = null; + @NotNull public abstract Sdk getSdk(); @@ -63,14 +64,47 @@ public abstract class PySdkUpdater { }); } - public static PySdkUpdater fromSdkPath(@Nullable String sdkPath) { + public abstract void commit(); + + public static synchronized PySdkUpdater fromSdkPath(@Nullable String sdkPath) { + checkSingleton(); return new JdkTableUpdater(sdkPath); } - public static PySdkUpdater fromSdkModificator(@NotNull Sdk sdk, @NotNull SdkModificator sdkModificator) { + public static synchronized PySdkUpdater fromSdkModificator(@NotNull Sdk sdk, @NotNull SdkModificator sdkModificator) { + checkSingleton(); return new SdkModificatorUpdater(sdk, sdkModificator); } + public static synchronized PySdkUpdater singletonJdkTableUpdater(@NotNull String sdkPath) { + if (mySingletonUpdater == null) { + Sdk sdk = PythonSdkType.findSdkByPath(sdkPath); + if (sdk != null) { + mySingletonUpdater = new SingletonSdkModificatorUpdater(sdk, sdk.getSdkModificator()); + } else { + throw new PySdkNotFoundException(); + } + } + return mySingletonUpdater; + } + + + private static synchronized void checkSingleton() { + if (mySingletonUpdater != null) { + throw new IllegalStateException("unintentionally changing more then one sdkModificator at a time"); + } + } + + private static synchronized void checkNotDisposed() { + if (mySingletonUpdater == null) { + throw new IllegalStateException("sdk modificator is already committed, further changes will go nowhere"); + } + } + + private static synchronized void disposeSingleton() { + mySingletonUpdater = null; + } + @Nullable public abstract String getHomePath(); @@ -100,6 +134,8 @@ public abstract class PySdkUpdater { } public void modifySdk(@NotNull SdkModificationProcessor processor) { + checkSingleton(); + ApplicationManager.getApplication().assertIsDispatchThread(); Sdk sdk = PythonSdkType.findSdkByPath(mySdkPath); @@ -110,6 +146,11 @@ public abstract class PySdkUpdater { modificator.commitChanges(); } } + + @Override + public void commit() { + // all changes are already committed + } } private static class SdkModificatorUpdater extends PySdkUpdater { @@ -136,6 +177,30 @@ public abstract class PySdkUpdater { public void modifySdk(@NotNull SdkModificationProcessor processor) { processor.process(getSdk(), myModificator); } + + @Override + public void commit() { + checkSingleton(); + myModificator.commitChanges(); + } + } + + private static class SingletonSdkModificatorUpdater extends SdkModificatorUpdater { + public SingletonSdkModificatorUpdater(@NotNull Sdk sdk, @NotNull SdkModificator modificator) { + super(sdk, modificator); + } + + @Override + public void modifySdk(@NotNull SdkModificationProcessor processor) { + checkNotDisposed(); + super.modifySdk(processor); + } + + @Override + public void commit() { + disposeSingleton(); + super.commit(); + } } @@ -143,5 +208,6 @@ public abstract class PySdkUpdater { void process(@NotNull Sdk sdk, @NotNull SdkModificator sdkModificator); } - public class PySdkNotFoundException extends RuntimeException {} + public static class PySdkNotFoundException extends RuntimeException { + } } diff --git a/python/src/com/jetbrains/python/sdk/PythonSdkType.java b/python/src/com/jetbrains/python/sdk/PythonSdkType.java index b589806057b9..392ab15d39c7 100644 --- a/python/src/com/jetbrains/python/sdk/PythonSdkType.java +++ b/python/src/com/jetbrains/python/sdk/PythonSdkType.java @@ -256,9 +256,8 @@ public class PythonSdkType extends SdkType { } public static boolean isDocker(@Nullable final Sdk sdk) { - return sdk != null && sdk.getSdkAdditionalData() instanceof RemoteSdkAdditionalData && - ((RemoteSdkAdditionalData) sdk.getSdkAdditionalData()).getRemoteConnectionType() == CredentialsType.DOCKER; - + return sdk != null && sdk.getSdkAdditionalData() instanceof RemoteSdkAdditionalData && + ((RemoteSdkAdditionalData)sdk.getSdkAdditionalData()).getRemoteConnectionType() == CredentialsType.DOCKER; } public static boolean isRemote(@Nullable String sdkPath) { @@ -304,7 +303,9 @@ public class PythonSdkType extends SdkType { return true; } - public void showCustomCreateUI(@NotNull SdkModel sdkModel, @NotNull final JComponent parentComponent, @NotNull final Consumer sdkCreatedCallback) { + public void showCustomCreateUI(@NotNull SdkModel sdkModel, + @NotNull final JComponent parentComponent, + @NotNull final Consumer sdkCreatedCallback) { Project project = CommonDataKeys.PROJECT.getData(DataManager.getInstance().getDataContext(parentComponent)); final PointerInfo pointerInfo = MouseInfo.getPointerInfo(); if (pointerInfo == null) return; @@ -458,7 +459,8 @@ public class PythonSdkType extends SdkType { } @Nullable - public AdditionalDataConfigurable createAdditionalDataConfigurable(@NotNull final SdkModel sdkModel, @NotNull final SdkModificator sdkModificator) { + public AdditionalDataConfigurable createAdditionalDataConfigurable(@NotNull final SdkModel sdkModel, + @NotNull final SdkModificator sdkModificator) { return null; } @@ -528,9 +530,9 @@ public class PythonSdkType extends SdkType { } public void setupSdkPaths(@NotNull final Sdk sdk, - @Nullable final Project project, - @Nullable final Component ownerComponent, - @NotNull final SdkModificator sdkModificator) { + @Nullable final Project project, + @Nullable final Component ownerComponent, + @NotNull final SdkModificator sdkModificator) { scheduledToRefresh.add(sdk.getHomePath()); doSetupSdkPaths(project, ownerComponent, PySdkUpdater.fromSdkModificator(sdk, sdkModificator)); } @@ -542,7 +544,8 @@ public class PythonSdkType extends SdkType { @Override public void run() { try { - final boolean success = doSetupSdkPaths(project, ownerComponent, PySdkUpdater.fromSdkPath(sdk.getHomePath())); + PySdkUpdater updater = PySdkUpdater.singletonJdkTableUpdater(sdk.getHomePath()); + final boolean success = doSetupSdkPaths(project, ownerComponent, updater); if (!success) { Messages.showErrorDialog( @@ -573,8 +576,8 @@ public class PythonSdkType extends SdkType { } private boolean doSetupSdkPaths(@Nullable final Project project, - @Nullable final Component ownerComponent, - @NotNull final PySdkUpdater sdkUpdater) { + @Nullable final Component ownerComponent, + @NotNull final PySdkUpdater sdkUpdater) { if (isRemote(sdkUpdater.getSdk()) && project == null && ownerComponent == null) { LOG.error("For refreshing skeletons of remote SDK, either project or owner component must be specified"); } @@ -616,6 +619,13 @@ public class PythonSdkType extends SdkType { else if (!isInvalid(sdkUpdater.getSdk())) { LOG.error(e); } + } finally { + UIUtil.invokeAndWaitIfNeeded(new Runnable() { + @Override + public void run() { + sdkUpdater.commit(); + } + }); } } }); @@ -845,7 +855,7 @@ public class PythonSdkType extends SdkType { // directory of the script itself - otherwise the dir in which we run the script (e.g. /usr/bin) will be added to SDK path GeneralCommandLine cmd = PythonHelper.SYSPATH.newCommandLine(binaryPath, Lists.newArrayList()); final ProcessOutput runResult = PySdkUtil.getProcessOutput(cmd, new File(binaryPath).getParent(), - getVirtualEnvExtraEnv(binaryPath), MINUTE); + getVirtualEnvExtraEnv(binaryPath), MINUTE); if (!runResult.checkSuccess(LOG)) { throw new InvalidSdkException(String.format("Failed to determine Python's sys.path value:\nSTDOUT: %s\nSTDERR: %s", runResult.getStdout(), diff --git a/python/src/com/jetbrains/python/sdk/PythonSdkUpdater.java b/python/src/com/jetbrains/python/sdk/PythonSdkUpdater.java index 03b6a121973d..f2d305b2a422 100644 --- a/python/src/com/jetbrains/python/sdk/PythonSdkUpdater.java +++ b/python/src/com/jetbrains/python/sdk/PythonSdkUpdater.java @@ -36,6 +36,7 @@ import com.intellij.openapi.vfs.LocalFileSystem; import com.intellij.openapi.vfs.StandardFileSystems; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.util.PathMappingSettings; +import com.intellij.util.ui.UIUtil; import com.jetbrains.python.PyBundle; import com.jetbrains.python.codeInsight.userSkeletons.PyUserSkeletonsUtil; import com.jetbrains.python.remote.PyRemoteSdkAdditionalDataBase; @@ -179,7 +180,7 @@ public class PythonSdkUpdater implements StartupActivity { if (file != null) { sysPath.add(file.getPath()); } - ApplicationManager.getApplication().invokeLater(new Runnable() { + UIUtil.invokeAndWaitIfNeeded(new Runnable() { @Override public void run() { updateSdkPath(sdkUpdater, sysPath);