Skip to content

Commit

Permalink
Revert "Postpone long-running operations to the activation of the "Tr…
Browse files Browse the repository at this point in the history 
…acing" tab."

This reverts commit 7c1f0ed from eclipse-pde#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.
  • Loading branch information
@HeikoKlare @HannesWell
HeikoKlare authored and HannesWell committed Nov 18, 2023
1 parent f169a74 commit eeffb96
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 102 deletions.
44 changes: 18 additions & 26 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/TracingOptionsManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -44,8 +44,8 @@ public TracingOptionsManager() {
super();
}

public Map<String, String> getTemplateTable(String pluginId, IProgressMonitor monitor) {
Map<String, String> tracingTemplate = getTracingTemplate(monitor);
public Map<String, String> getTemplateTable(String pluginId) {
Map<String, String> tracingTemplate = getTracingTemplate();
Map<String, String> defaults = new HashMap<>();
tracingTemplate.forEach((key, value) -> {
if (belongsTo(key, pluginId)) {
Expand All @@ -60,9 +60,9 @@ private boolean belongsTo(String option, String pluginId) {
return pluginId.equalsIgnoreCase(firstSegment);
}

public Map<String, String> getTracingOptions(Map<String, String> storedOptions, IProgressMonitor monitor) {
public Map<String, String> getTracingOptions(Map<String, String> storedOptions) {
// Start with the fresh template from plugins
Map<String, String> defaults = getTracingTemplateCopy(monitor);
Map<String, String> defaults = getTracingTemplateCopy();
if (storedOptions != null) {
// Load stored values, but only for existing keys
storedOptions.forEach((key, value) -> {
Expand All @@ -74,29 +74,21 @@ public Map<String, String> getTracingOptions(Map<String, String> storedOptions,
return defaults;
}

public Map<String, String> getTracingTemplateCopy(IProgressMonitor monitor) {
return new HashMap<>(getTracingTemplate(monitor));
public Map<String, String> getTracingTemplateCopy() {
return new HashMap<>(getTracingTemplate());
}

private synchronized Map<String, String> getTracingTemplate(IProgressMonitor monitor) {
if (template != null) {
return template;
}

Map<String, String> 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<String, String> getTracingTemplate() {
if (template == null) {
Map<String, String> temp = new HashMap<>();
IPluginModelBase[] models = PluginRegistry.getAllModels();
Arrays.stream(models).map(TracingOptionsManager::getOptions).filter(Objects::nonNull).forEach(p -> {
@SuppressWarnings({ "rawtypes", "unchecked" })
Map<String, String> entries = (Map) options;
Map<String, String> entries = (Map) p;
temp.putAll(entries); // All entries are of String/String
}
});
template = temp;
}
template = temp;
return template;
}

Expand Down Expand Up @@ -137,7 +129,7 @@ private void saveOptions(Path file, Map<String, String> entries) {
}

public void save(Path file, Map<String, String> map, Set<String> selected) {
Map<String, String> properties = getTracingOptions(map, null);
Map<String, String> properties = getTracingOptions(map);
properties.keySet().removeIf(key -> {
IPath path = IPath.fromOSString(key);
return path.segmentCount() < 1 || !selected.contains(path.segment(0));
Expand All @@ -146,7 +138,7 @@ public void save(Path file, Map<String, String> map, Set<String> selected) {
}

public void save(Path file, Map<String, String> map) {
saveOptions(file, getTracingOptions(map, null));
saveOptions(file, getTracingOptions(map));
}

private static Properties getOptions(IPluginModelBase model) {
Expand Down
1 change: 0 additions & 1 deletion ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/PDEUIMessages.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
73 changes: 7 additions & 66 deletions ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/TracingBlock.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, String> getTracingTemplateCopy() {
return runShowingProgress(
(tracingOptionsManager, monitor) -> tracingOptionsManager.getTracingTemplateCopy(monitor));
}

public Map<String, String> getTracingOptions(Map<String, String> options) {
return runShowingProgress(
(tracingOptionsManager, monitor) -> tracingOptionsManager.getTracingOptions(options, monitor));
}

public Map<String, String> getTemplateTable(String pluginId) {
return runShowingProgress(
(tracingOptionsManager, monitor) -> tracingOptionsManager.getTemplateTable(pluginId, monitor));
}

private Map<String, String> runShowingProgress(
BiFunction<TracingOptionsManager, IProgressMonitor, Map<String, String>> task) {
try {
String taskName = PDEUIMessages.TracingBlock_initializing_tracing_options;
final Map<String, String> 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;
Expand Down Expand Up @@ -253,7 +201,8 @@ private void createRestoreButtonSection(Composite parent) {
if (selec.getFirstElement() instanceof IPluginModelBase model) {
String modelName = model.getBundleDescription().getSymbolicName();
if (modelName != null) {
Map<String, String> properties = fTracingOptionsManagerDelegate.getTracingTemplateCopy();
Map<String, String> properties = PDECore.getDefault().getTracingOptionsManager()
.getTracingTemplateCopy();
properties.forEach((key, value) -> {
if (key.startsWith(modelName + '/')) {
fMasterOptions.put(key, value);
Expand All @@ -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) {
Expand Down Expand Up @@ -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<String, String> options = config.getAttribute(IPDELauncherConstants.TRACING_OPTIONS, (Map<String, String>) 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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -406,7 +348,6 @@ public void setDefaults(ILaunchConfigurationWorkingCopy configuration) {
}

public void activated(ILaunchConfigurationWorkingCopy workingCopy) {
activateInternal(workingCopy);
fPageBook.getParent().getParent().layout(true);
}

Expand Down Expand Up @@ -494,7 +435,7 @@ private TracingPropertySource getPropertySource(IPluginModelBase model) {
return null;
return fPropertySources.computeIfAbsent(model, m -> {
String id = m.getPluginBase().getId();
Map<String, String> defaults = fTracingOptionsManagerDelegate.getTemplateTable(id);
Map<String, String> defaults = PDECore.getDefault().getTracingOptionsManager().getTemplateTable(id);
TracingPropertySource source = new TracingPropertySource(m, fMasterOptions, defaults, this);
source.setChanged(true);
return source;
Expand Down
1 change: 0 additions & 1 deletion ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 1 addition & 8 deletions ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/TracingTab.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit eeffb96

Please sign in to comment.