Skip to content

Commit

Permalink
Postpone long-running operations to the activation of the "Tracing" tab.
Browse files Browse the repository at this point in the history 
Don't start unnecessary operations that only concern the "Tracing" tab
upon initialization of the PDE-Run configuration (in the dialog "Run
configurations..."). Instead, only do them when the tab is first
selected. This speeds up opening the dialog "Run configurations..." (and
also switching to a PDE-Run configuration when the dialog is already
opened).

Show a progress indicator when doing some (possibly) long-running
operations in the "Tracing" tab.

Fixes to #679
  • Loading branch information
@fedejeanne
fedejeanne committed Oct 30, 2023
1 parent 6573a35 commit 6a39932
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 26 deletions.
44 changes: 26 additions & 18 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) {
Map<String, String> tracingTemplate = getTracingTemplate();
public Map<String, String> getTemplateTable(String pluginId, IProgressMonitor monitor) {
Map<String, String> tracingTemplate = getTracingTemplate(monitor);
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) {
public Map<String, String> getTracingOptions(Map<String, String> storedOptions, IProgressMonitor monitor) {
// Start with the fresh template from plugins
Map<String, String> defaults = getTracingTemplateCopy();
Map<String, String> defaults = getTracingTemplateCopy(monitor);
if (storedOptions != null) {
// Load stored values, but only for existing keys
storedOptions.forEach((key, value) -> {
Expand All @@ -74,21 +74,29 @@ public Map<String, String> getTracingOptions(Map<String, String> storedOptions)
return defaults;
}

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

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 -> {
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) {
@SuppressWarnings({ "rawtypes", "unchecked" })
Map<String, String> entries = (Map) p;
Map<String, String> entries = (Map) options;
temp.putAll(entries); // All entries are of String/String
});
template = temp;
}
}
template = temp;
return template;
}

Expand Down Expand Up @@ -129,7 +137,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);
Map<String, String> properties = getTracingOptions(map, null);
properties.keySet().removeIf(key -> {
IPath path = IPath.fromOSString(key);
return path.segmentCount() < 1 || !selected.contains(path.segment(0));
Expand All @@ -138,7 +146,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));
saveOptions(file, getTracingOptions(map, null));
}

private static Properties getOptions(IPluginModelBase model) {
Expand Down
1 change: 1 addition & 0 deletions 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 @@ -1093,6 +1093,7 @@ 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
75 changes: 68 additions & 7 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,16 +18,21 @@

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 @@ -58,6 +63,55 @@

public class TracingBlock {

/**
* This class encapsulates all calls to the tracing options manager and runs them using a
*/
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 | InterruptedException e) {
PDEPlugin.logException(e);
}

// This should never happen since the task can not be canceled
// (yet). If it does happen then some exception occurred while
// calling the TracingOptionsManager and all bets are off (i.e. NPEs
// will follow)
return null;
}
}

private TracingOptionsManagerDelegate fTracingOptionsManagerDelegate;
private final TracingTab fTab;
private Button fTracingCheck;
private CheckboxTableViewer fPluginViewer;
Expand Down Expand Up @@ -201,8 +255,7 @@ private void createRestoreButtonSection(Composite parent) {
if (selec.getFirstElement() instanceof IPluginModelBase model) {
String modelName = model.getBundleDescription().getSymbolicName();
if (modelName != null) {
Map<String, String> properties = PDECore.getDefault().getTracingOptionsManager()
.getTracingTemplateCopy();
Map<String, String> properties = fTracingOptionsManagerDelegate.getTracingTemplateCopy();
properties.forEach((key, value) -> {
if (key.startsWith(modelName + '/')) {
fMasterOptions.put(key, value);
Expand All @@ -222,7 +275,7 @@ private void createRestoreButtonSection(Composite parent) {
fRestoreDefaultButton.addSelectionListener(SelectionListener.widgetSelectedAdapter(e -> {
disposePropertySources();
fMasterOptions.clear();
fMasterOptions.putAll(PDECore.getDefault().getTracingOptionsManager().getTracingTemplateCopy());
fMasterOptions.putAll(fTracingOptionsManagerDelegate.getTracingTemplateCopy());
Object[] elements = fPluginViewer.getCheckedElements();
for (Object element : elements) {
if (element instanceof IPluginModelBase model) {
Expand Down Expand Up @@ -265,14 +318,17 @@ protected int createPropertySheet(Composite parent) {
return style == SWT.NULL ? 2 : 0;
}

public void initializeFrom(ILaunchConfiguration config) {
private void activateInternal(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);
TracingOptionsManager mgr = PDECore.getDefault().getTracingOptionsManager();
fMasterOptions.putAll(options == null ? mgr.getTracingTemplateCopy() : mgr.getTracingOptions(options));

fMasterOptions.putAll(options == null //
? fTracingOptionsManagerDelegate.getTracingTemplateCopy() //
: fTracingOptionsManagerDelegate.getTracingOptions(options));

masterCheckChanged();
String checked = config.getAttribute(IPDELauncherConstants.TRACING_CHECKED, (String) null);
if (checked == null) {
Expand Down Expand Up @@ -302,6 +358,10 @@ public void initializeFrom(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 @@ -348,6 +408,7 @@ public void setDefaults(ILaunchConfigurationWorkingCopy configuration) {
}

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

Expand Down Expand Up @@ -435,7 +496,7 @@ private TracingPropertySource getPropertySource(IPluginModelBase model) {
return null;
return fPropertySources.computeIfAbsent(model, m -> {
String id = m.getPluginBase().getId();
Map<String, String> defaults = PDECore.getDefault().getTracingOptionsManager().getTemplateTable(id);
Map<String, String> defaults = fTracingOptionsManagerDelegate.getTemplateTable(id);
TracingPropertySource source = new TracingPropertySource(m, fMasterOptions, defaults, this);
source.setChanged(true);
return source;
Expand Down
1 change: 1 addition & 0 deletions 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 @@ -599,6 +599,7 @@ 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: 8 additions & 1 deletion 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,6 +16,7 @@

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 @@ -81,7 +82,13 @@ public void dispose() {

@Override
public void initializeFrom(ILaunchConfiguration config) {
fTracingBlock.initializeFrom(config);
// the heavy lifting occurs in #activated(...)
}

@Override
public void setLaunchConfigurationDialog(ILaunchConfigurationDialog dialog) {
super.setLaunchConfigurationDialog(dialog);
fTracingBlock.setRunnableContext(dialog);
}

@Override
Expand Down

0 comments on commit 6a39932

Please sign in to comment.