From eeffb962b568b6208d1de4d8b897a78371a4eec2 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Thu, 16 Nov 2023 17:37:36 +0100 Subject: [PATCH] Revert "Postpone long-running operations to the activation of the "Tracing" tab." This reverts commit 7c1f0ed1479916ffe425a87e2fae689dbb72975a from #674. The change introduced a regression causing Tracing tabs not to be initialized properly in specific situation. It requires an update to the Eclipse platform, which will not be integrated into the upcoming release anymore. Therefore, the change is reverted for the release to be reapplied after the next release. The effect of reverting the change will be a loss of the performance improvement in the launch configurations dialog that was gained by the change. --- .../internal/core/TracingOptionsManager.java | 44 +++++------ .../pde/internal/ui/PDEUIMessages.java | 1 - .../internal/ui/launcher/TracingBlock.java | 73 ++----------------- .../pde/internal/ui/pderesources.properties | 1 - .../eclipse/pde/ui/launcher/TracingTab.java | 9 +-- 5 files changed, 26 insertions(+), 102 deletions(-) diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/TracingOptionsManager.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/TracingOptionsManager.java index 8db8215cc7..5d53461899 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/TracingOptionsManager.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/TracingOptionsManager.java @@ -24,16 +24,16 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.Properties; import java.util.Set; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import org.eclipse.core.runtime.IPath; -import org.eclipse.core.runtime.IProgressMonitor; -import org.eclipse.core.runtime.SubMonitor; import org.eclipse.pde.core.plugin.IPluginModelBase; import org.eclipse.pde.core.plugin.PluginRegistry; @@ -44,8 +44,8 @@ public TracingOptionsManager() { super(); } - public Map getTemplateTable(String pluginId, IProgressMonitor monitor) { - Map tracingTemplate = getTracingTemplate(monitor); + public Map getTemplateTable(String pluginId) { + Map tracingTemplate = getTracingTemplate(); Map defaults = new HashMap<>(); tracingTemplate.forEach((key, value) -> { if (belongsTo(key, pluginId)) { @@ -60,9 +60,9 @@ private boolean belongsTo(String option, String pluginId) { return pluginId.equalsIgnoreCase(firstSegment); } - public Map getTracingOptions(Map storedOptions, IProgressMonitor monitor) { + public Map getTracingOptions(Map storedOptions) { // Start with the fresh template from plugins - Map defaults = getTracingTemplateCopy(monitor); + Map defaults = getTracingTemplateCopy(); if (storedOptions != null) { // Load stored values, but only for existing keys storedOptions.forEach((key, value) -> { @@ -74,29 +74,21 @@ public Map getTracingOptions(Map storedOptions, return defaults; } - public Map getTracingTemplateCopy(IProgressMonitor monitor) { - return new HashMap<>(getTracingTemplate(monitor)); + public Map getTracingTemplateCopy() { + return new HashMap<>(getTracingTemplate()); } - private synchronized Map getTracingTemplate(IProgressMonitor monitor) { - if (template != null) { - return template; - } - - Map temp = new HashMap<>(); - IPluginModelBase[] models = PluginRegistry.getAllModels(); - SubMonitor subMonitor = SubMonitor.convert(monitor, models.length); - - for (IPluginModelBase model : models) { - subMonitor.split(1); - Properties options = TracingOptionsManager.getOptions(model); - if (options != null) { + private synchronized Map getTracingTemplate() { + if (template == null) { + Map temp = new HashMap<>(); + IPluginModelBase[] models = PluginRegistry.getAllModels(); + Arrays.stream(models).map(TracingOptionsManager::getOptions).filter(Objects::nonNull).forEach(p -> { @SuppressWarnings({ "rawtypes", "unchecked" }) - Map entries = (Map) options; + Map entries = (Map) p; temp.putAll(entries); // All entries are of String/String - } + }); + template = temp; } - template = temp; return template; } @@ -137,7 +129,7 @@ private void saveOptions(Path file, Map entries) { } public void save(Path file, Map map, Set selected) { - Map properties = getTracingOptions(map, null); + Map properties = getTracingOptions(map); properties.keySet().removeIf(key -> { IPath path = IPath.fromOSString(key); return path.segmentCount() < 1 || !selected.contains(path.segment(0)); @@ -146,7 +138,7 @@ public void save(Path file, Map map, Set selected) { } public void save(Path file, Map map) { - saveOptions(file, getTracingOptions(map, null)); + saveOptions(file, getTracingOptions(map)); } private static Properties getOptions(IPluginModelBase model) { diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/PDEUIMessages.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/PDEUIMessages.java index 0f2f1b1288..1aa2cc0263 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/PDEUIMessages.java +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/PDEUIMessages.java @@ -1092,7 +1092,6 @@ public class PDEUIMessages extends NLS { public static String TracingTab_AttributeLabel_TracingChecked; public static String TracingTab_AttributeLabel_TracingNone; - public static String TracingBlock_initializing_tracing_options; public static String TracingBlock_restore_default; public static String TracingBlock_restore_default_selected; diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/TracingBlock.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/TracingBlock.java index a4e80376fc..759c45bccc 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/TracingBlock.java +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/TracingBlock.java @@ -18,21 +18,16 @@ import static org.eclipse.swt.events.SelectionListener.widgetSelectedAdapter; -import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.function.BiFunction; import org.eclipse.core.runtime.CoreException; -import org.eclipse.core.runtime.IProgressMonitor; -import org.eclipse.core.runtime.SubMonitor; import org.eclipse.debug.core.ILaunchConfiguration; import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy; import org.eclipse.jface.dialogs.IDialogSettings; -import org.eclipse.jface.operation.IRunnableContext; import org.eclipse.jface.viewers.ArrayContentProvider; import org.eclipse.jface.viewers.CheckboxTableViewer; import org.eclipse.jface.viewers.IStructuredSelection; @@ -63,53 +58,6 @@ public class TracingBlock { - /** - * This class encapsulates all calls to the tracing options manager and runs - * them using a progress monitor. - */ - private static record TracingOptionsManagerDelegate(IRunnableContext fRunnableContext) { - - Map getTracingTemplateCopy() { - return runShowingProgress( - (tracingOptionsManager, monitor) -> tracingOptionsManager.getTracingTemplateCopy(monitor)); - } - - public Map getTracingOptions(Map options) { - return runShowingProgress( - (tracingOptionsManager, monitor) -> tracingOptionsManager.getTracingOptions(options, monitor)); - } - - public Map getTemplateTable(String pluginId) { - return runShowingProgress( - (tracingOptionsManager, monitor) -> tracingOptionsManager.getTemplateTable(pluginId, monitor)); - } - - private Map runShowingProgress( - BiFunction> task) { - try { - String taskName = PDEUIMessages.TracingBlock_initializing_tracing_options; - final Map result = new HashMap<>(); - - // due to a bug - // (https://github.com/eclipse-platform/eclipse.platform/issues/769), - // the task needs to be forked otherwise the UI - // does not show the progress indicator - fRunnableContext.run(true, false, monitor -> { - SubMonitor subMonitor = SubMonitor.convert(monitor, taskName, 1); - result.putAll(task.apply(PDECore.getDefault().getTracingOptionsManager(), subMonitor.split(1))); - }); - - return result; - } catch (InvocationTargetException e) { - throw new IllegalStateException(e); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IllegalStateException(e); - } - } - } - - private TracingOptionsManagerDelegate fTracingOptionsManagerDelegate; private final TracingTab fTab; private Button fTracingCheck; private CheckboxTableViewer fPluginViewer; @@ -253,7 +201,8 @@ private void createRestoreButtonSection(Composite parent) { if (selec.getFirstElement() instanceof IPluginModelBase model) { String modelName = model.getBundleDescription().getSymbolicName(); if (modelName != null) { - Map properties = fTracingOptionsManagerDelegate.getTracingTemplateCopy(); + Map properties = PDECore.getDefault().getTracingOptionsManager() + .getTracingTemplateCopy(); properties.forEach((key, value) -> { if (key.startsWith(modelName + '/')) { fMasterOptions.put(key, value); @@ -273,7 +222,7 @@ private void createRestoreButtonSection(Composite parent) { fRestoreDefaultButton.addSelectionListener(SelectionListener.widgetSelectedAdapter(e -> { disposePropertySources(); fMasterOptions.clear(); - fMasterOptions.putAll(fTracingOptionsManagerDelegate.getTracingTemplateCopy()); + fMasterOptions.putAll(PDECore.getDefault().getTracingOptionsManager().getTracingTemplateCopy()); Object[] elements = fPluginViewer.getCheckedElements(); for (Object element : elements) { if (element instanceof IPluginModelBase model) { @@ -316,17 +265,14 @@ protected int createPropertySheet(Composite parent) { return style == SWT.NULL ? 2 : 0; } - private void activateInternal(ILaunchConfiguration config) { + public void initializeFrom(ILaunchConfiguration config) { fMasterOptions.clear(); disposePropertySources(); try { fTracingCheck.setSelection(config.getAttribute(IPDELauncherConstants.TRACING, false)); Map options = config.getAttribute(IPDELauncherConstants.TRACING_OPTIONS, (Map) null); - - fMasterOptions.putAll(options == null // - ? fTracingOptionsManagerDelegate.getTracingTemplateCopy() // - : fTracingOptionsManagerDelegate.getTracingOptions(options)); - + TracingOptionsManager mgr = PDECore.getDefault().getTracingOptionsManager(); + fMasterOptions.putAll(options == null ? mgr.getTracingTemplateCopy() : mgr.getTracingOptions(options)); masterCheckChanged(); String checked = config.getAttribute(IPDELauncherConstants.TRACING_CHECKED, (String) null); if (checked == null) { @@ -356,10 +302,6 @@ private void activateInternal(ILaunchConfiguration config) { } } - public void setRunnableContext(IRunnableContext runnableContext) { - fTracingOptionsManagerDelegate = new TracingOptionsManagerDelegate(runnableContext); - } - public void performApply(ILaunchConfigurationWorkingCopy config) { boolean tracingEnabled = fTracingCheck.getSelection(); config.setAttribute(IPDELauncherConstants.TRACING, tracingEnabled); @@ -406,7 +348,6 @@ public void setDefaults(ILaunchConfigurationWorkingCopy configuration) { } public void activated(ILaunchConfigurationWorkingCopy workingCopy) { - activateInternal(workingCopy); fPageBook.getParent().getParent().layout(true); } @@ -494,7 +435,7 @@ private TracingPropertySource getPropertySource(IPluginModelBase model) { return null; return fPropertySources.computeIfAbsent(model, m -> { String id = m.getPluginBase().getId(); - Map defaults = fTracingOptionsManagerDelegate.getTemplateTable(id); + Map defaults = PDECore.getDefault().getTracingOptionsManager().getTemplateTable(id); TracingPropertySource source = new TracingPropertySource(m, fMasterOptions, defaults, this); source.setChanged(true); return source; diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties index 9bb8eae9c2..b277c3c5bc 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties @@ -596,7 +596,6 @@ TracingTab_AttributeLabel_TracingOptions=Tracing options TracingTab_AttributeLabel_TracingChecked=Tracing select all TracingTab_AttributeLabel_TracingNone=Tracing select none -TracingBlock_initializing_tracing_options=Initializing tracing options TracingBlock_restore_default=Restore &All to Defaults TracingBlock_restore_default_selected=Restore &Selected to Defaults TracingLauncherTab_name = Trac&ing diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/TracingTab.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/TracingTab.java index 5fc25c0371..636dd712d3 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/TracingTab.java +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/TracingTab.java @@ -16,7 +16,6 @@ import org.eclipse.debug.core.ILaunchConfiguration; import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy; -import org.eclipse.debug.ui.ILaunchConfigurationDialog; import org.eclipse.jface.dialogs.Dialog; import org.eclipse.pde.internal.ui.IHelpContextIds; import org.eclipse.pde.internal.ui.PDEPlugin; @@ -82,13 +81,7 @@ public void dispose() { @Override public void initializeFrom(ILaunchConfiguration config) { - // the heavy lifting occurs in #activated(...) - } - - @Override - public void setLaunchConfigurationDialog(ILaunchConfigurationDialog dialog) { - super.setLaunchConfigurationDialog(dialog); - fTracingBlock.setRunnableContext(dialog); + fTracingBlock.initializeFrom(config); } @Override