Community
Participate
Working Groups
Created attachment 285243 [details] Example plug-in to reproduce the problem with. To reproduce: 1. Import plug-in from attached archive "TestProgressViewJobs.zip". Alternatively, create a plug-in with an example command, in the command add code to trigger many jobs. 2. Open the Progress view. 3. Execute the command from step 1.. 4. Monitor the memory consumption of the Eclipse process over time, observe a gradual increase of RES, while the JDK heap remains stable. We observe this with 4.15 as platform for our product, during stress tests. But I also see this with recent SWT on 4.19.
Can you please explain all steps necessary? I don't use Eclipse and pretty much don't know anything about it. If you are able to reduce the problem to a SWT snippet, that would be even better.
With the attached plugin, what is the leak rate you're observing?
(In reply to Alexandr Miloslavskiy from comment #2) > With the attached plugin, what is the leak rate you're observing? Slow. If I give the Eclipse JVM only -Xmx1g, usually it takes over an hour until the RES to go over the max heap size. If I let the plug-in command run overnight, its at 3 GB+. In our product memory is leaked at a higher rate, I'm not sure why.
(In reply to Alexandr Miloslavskiy from comment #1) > Can you please explain all steps necessary? I don't use Eclipse and pretty > much don't know anything about it. If you are able to reduce the problem to > a SWT snippet, that would be even better. Sorry I didn't see that comment. There is a new command that should be in the main toolbar, that you can run for the reproduction (e.g. debug Eclipse with the attached plug-in in your workspace). I've not been able to reduce this to an SWT snippet yet; so far I don't know where the Progress view is leaking memory (I've not had much time to look into it either). From what I see, the Progress view is creating a new widget for every job it displays. The widget is ProgressInfoItem, which extends Composite. I've tried replacing all children of ProgressInfoItem with just a Text widget; either the leak is then very small, or its gone. I'm trying to identify which particular child causes the leak (to use in an SWT snippet), so far no success. The ProgressInfoItem widgets themselves seem to be correctly disposed (all of them, as far as I can tell).
I'm currently looking for any leaks in SWT's JUnit tests and I'm seeing something related to Composite.imContext(), don't yet have more specific results. > There is a new command that should be in the main toolbar Apparently you're underestimating what "don't know anything about it" means :) How do I even import this plugin into Eclipse? Where do I find the "Progress view" ?
> Apparently you're underestimating what "don't know anything about it" means > :) > How do I even import this plugin into Eclipse? Where do I find the "Progress > view" ? You can open views (e.g. the Progress view) with "quick access", Ctrl+3 to invoke this. Then you can type e.g. "progress" and click the Progress view from the list of results. There should also be a widget to the top right (with a looking glass), with a search field that also lets you use quick access in a similar manner. 1. In the Package Explorer view (top left in the default Java perspective, if its not open you can open it same way as the Progress view), you can right click and choose Import. 2. From the dialog, choose General -> "Existing Projects into Workspace". 3. In the dialog, use the "Select archive file" radio box and Browse to the attached archive (from this ticket, "TestProgressViewJobs.zip". 4. Once the plug-in is imported, debug Eclipse from the running Eclipse instance. An Eclipse SDK should be enough for this (e.g. some new integration build from https://download.eclipse.org/eclipse/downloads/): 4.1. From the main menu, go to "Run". 4.2. Go to "Debug Configurations...". 4.3. Double click on the "Eclipse Application" (on the left part of the dialog), this will create an Eclipse launch. 4.4. You can click on the Debug button (bottom right). You can re-use the launch to debug later on. 5. The main toolbar (below the main menu) should have a button with the Eclipse icon, clicking that button will start running tons of jobs for a long while (for the Progress view to display). Let me know if I've missed something in the steps above, or if something is not clear.
In step 4.3, I don't have "Eclipse Application" on the left. "Java Application" is the closest thing.
(In reply to Alexandr Miloslavskiy from comment #7) > In step 4.3, I don't have "Eclipse Application" on the left. "Java > Application" is the closest thing. Probably you have a Java IDE Eclipse, and not an SDK. Could you download e.g. a build from here: https://download.eclipse.org/eclipse/downloads/drops4/I20210110-1800/
Thanks, I was now able to run the plugin and I see items in Progress view. Will study it.
I waited some 3 hours, but RES is still below 1gb.
(In reply to Alexandr Miloslavskiy from comment #10) > I waited some 3 hours, but RES is still below 1gb. I'll try to reduce the problem to an SWT snippet, hopefully that works.
With the following change, I no longer see the leak (ran for 2 days and a bit, below 1 GB RES): diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/DetailedProgressViewer.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/DetailedProgressViewer.java index 0fafa080ad..735467610e 100644 --- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/DetailedProgressViewer.java +++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/DetailedProgressViewer.java @@ -33,15 +33,11 @@ import org.eclipse.swt.events.ControlListener; import org.eclipse.swt.events.FocusAdapter; import org.eclipse.swt.events.FocusEvent; import org.eclipse.swt.graphics.Point; -import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; -import org.eclipse.swt.widgets.Label; import org.eclipse.swt.widgets.Widget; -import org.eclipse.ui.PlatformUI; import org.eclipse.ui.internal.IPreferenceConstants; -import org.eclipse.ui.internal.IWorkbenchHelpContextIds; import org.eclipse.ui.internal.WorkbenchPlugin; /** @@ -90,7 +86,7 @@ public class DetailedProgressViewer extends AbstractProgressViewer { layout.marginWidth = 0; control.setLayout(layout); control.setBackground(parent.getDisplay().getSystemColor(SWT.COLOR_LIST_BACKGROUND)); - +// control.addFocusListener(new FocusAdapter() { private boolean settingFocus = false; @@ -119,8 +115,8 @@ public class DetailedProgressViewer extends AbstractProgressViewer { updateVisibleItems(); } }); - - PlatformUI.getWorkbench().getHelpSystem().setHelp(control, IWorkbenchHelpContextIds.RESPONSIVE_UI); +// +// PlatformUI.getWorkbench().getHelpSystem().setHelp(control, IWorkbenchHelpContextIds.RESPONSIVE_UI); scrolled.setContent(control); hookControl(control); @@ -128,15 +124,15 @@ public class DetailedProgressViewer extends AbstractProgressViewer { noEntryArea = new Composite(scrolled, SWT.NONE); noEntryArea.setLayout(new GridLayout()); noEntryArea.setBackground(noEntryArea.getDisplay().getSystemColor(SWT.COLOR_LIST_BACKGROUND)); - - Label noEntryLabel = new Label(noEntryArea, SWT.NONE); - noEntryLabel.setText(ProgressMessages.ProgressView_NoOperations); - noEntryLabel.setBackground(noEntryArea.getDisplay().getSystemColor(SWT.COLOR_LIST_BACKGROUND)); - GridData textData = new GridData(GridData.VERTICAL_ALIGN_BEGINNING); - noEntryLabel.setLayoutData(textData); - - PlatformUI.getWorkbench().getHelpSystem().setHelp(noEntryLabel, IWorkbenchHelpContextIds.RESPONSIVE_UI); - +// +// Label noEntryLabel = new Label(noEntryArea, SWT.NONE); +// noEntryLabel.setText(ProgressMessages.ProgressView_NoOperations); +// noEntryLabel.setBackground(noEntryArea.getDisplay().getSystemColor(SWT.COLOR_LIST_BACKGROUND)); +// GridData textData = new GridData(GridData.VERTICAL_ALIGN_BEGINNING); +// noEntryLabel.setLayoutData(textData); +// +// PlatformUI.getWorkbench().getHelpSystem().setHelp(noEntryLabel, IWorkbenchHelpContextIds.RESPONSIVE_UI); +// IPreferenceStore prefs = WorkbenchPlugin.getDefault().getPreferenceStore(); updateMaxDisplayedValue(prefs); IPropertyChangeListener listener = this::propertyChange; @@ -225,27 +221,27 @@ public class DetailedProgressViewer extends AbstractProgressViewer { } }); - item.setIndexListener(new ProgressInfoItem.IndexListener() { - @Override - public void selectNext() { - DetailedProgressViewer.this.selectNext(item); - } - - @Override - public void selectPrevious() { - DetailedProgressViewer.this.selectPrevious(item); - } - - @Override - public void select() { - for (ProgressInfoItem child : jobItemControls.values()) { - if (!item.equals(child)) { - child.selectWidgets(false); - } - } - item.selectWidgets(true); - } - }); +// item.setIndexListener(new ProgressInfoItem.IndexListener() { +// @Override +// public void selectNext() { +// DetailedProgressViewer.this.selectNext(item); +// } +// +// @Override +// public void selectPrevious() { +// DetailedProgressViewer.this.selectPrevious(item); +// } +// +// @Override +// public void select() { +// for (ProgressInfoItem child : jobItemControls.values()) { +// if (!item.equals(child)) { +// child.selectWidgets(false); +// } +// } +// item.selectWidgets(true); +// } +// }); return item; } @@ -256,22 +252,22 @@ public class DetailedProgressViewer extends AbstractProgressViewer { * @param item the reference item. The item previous to this will be selected. */ protected void selectPrevious(ProgressInfoItem item) { - Control[] children = control.getChildren(); - for (int i = 0; i < children.length; i++) { - ProgressInfoItem child = (ProgressInfoItem) children[i]; - if (item.equals(child)) { - ProgressInfoItem previous; - if (i == 0) { - previous = (ProgressInfoItem) children[children.length - 1]; - } else { - previous = (ProgressInfoItem) children[i - 1]; - } - - item.selectWidgets(false); - previous.selectWidgets(true); - return; - } - } +// Control[] children = control.getChildren(); +// for (int i = 0; i < children.length; i++) { +// ProgressInfoItem child = (ProgressInfoItem) children[i]; +// if (item.equals(child)) { +// ProgressInfoItem previous; +// if (i == 0) { +// previous = (ProgressInfoItem) children[children.length - 1]; +// } else { +// previous = (ProgressInfoItem) children[i - 1]; +// } +// +//// item.selectWidgets(false); +//// previous.selectWidgets(true); +// return; +// } +// } } /** @@ -280,22 +276,22 @@ public class DetailedProgressViewer extends AbstractProgressViewer { * @param item the reference item. The item next to this will be selected. */ protected void selectNext(ProgressInfoItem item) { - Control[] children = control.getChildren(); - for (int i = 0; i < children.length; i++) { - ProgressInfoItem child = (ProgressInfoItem) children[i]; - if (item.equals(child)) { - ProgressInfoItem next; - if (i == children.length - 1) { - next = (ProgressInfoItem) children[0]; - } else { - next = (ProgressInfoItem) children[i + 1]; - } - item.selectWidgets(false); - next.selectWidgets(true); - - return; - } - } +// Control[] children = control.getChildren(); +// for (int i = 0; i < children.length; i++) { +// ProgressInfoItem child = (ProgressInfoItem) children[i]; +// if (item.equals(child)) { +// ProgressInfoItem next; +// if (i == children.length - 1) { +// next = (ProgressInfoItem) children[0]; +// } else { +// next = (ProgressInfoItem) children[i + 1]; +// } +//// item.selectWidgets(false); +//// next.selectWidgets(true); +// +// return; +// } +// } } @@ -390,15 +386,18 @@ public class DetailedProgressViewer extends AbstractProgressViewer { jobItemControls.remove(element); unmapElement(element); item.dispose(); + } else { + // System.out.println("failed to find item for: " + + // treeElement.getDisplayString()); //$NON-NLS-1$ } } } - Control[] existingChildren = control.getChildren(); - for (int i = 0; i < existingChildren.length; i++) { - ProgressInfoItem item = (ProgressInfoItem) existingChildren[i]; - item.setColor(i); - } +// Control[] existingChildren = control.getChildren(); +// for (int i = 0; i < existingChildren.length; i++) { +// ProgressInfoItem item = (ProgressInfoItem) existingChildren[i]; +// item.setColor(i); +// } updateForShowingProgress(); } @@ -425,12 +424,12 @@ public class DetailedProgressViewer extends AbstractProgressViewer { * */ public void setFocus() { - Control[] children = control.getChildren(); - if (children.length > 0) { - ((ProgressInfoItem) children[0]).setButtonFocus(); - } else { - noEntryArea.setFocus(); - } +// Control[] children = control.getChildren(); +// if (children.length > 0) { +// ((ProgressInfoItem) children[0]).setButtonFocus(); +// } else { +// noEntryArea.setFocus(); +// } } /** @@ -517,7 +516,7 @@ public class DetailedProgressViewer extends AbstractProgressViewer { exIndex++; } } - item.setColor(i); + // item.setColor(i); lastControl = item; } for (int i = exIndex; i < existingControls.length; i++) { @@ -531,17 +530,17 @@ public class DetailedProgressViewer extends AbstractProgressViewer { /** * Set the virtual items to be visible or not depending on the displayed area. */ - private void updateVisibleItems() { + void updateVisibleItems() { updateVisibleProgressItems(control.getChildren()); } private void updateVisibleProgressItems(Control... progressInfoItems) { - int top = scrolled.getOrigin().y; - int bottom = top + scrolled.getParent().getBounds().height; - for (Control control : progressInfoItems) { - ProgressInfoItem item = (ProgressInfoItem) control; - item.setDisplayed(top, bottom); - } +// int top = scrolled.getOrigin().y; +// int bottom = top + scrolled.getParent().getBounds().height; +// for (Control control : progressInfoItems) { +// ProgressInfoItem item = (ProgressInfoItem) control; +// item.setDisplayed(top, bottom); +// } } /** diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/ProgressInfoItem.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/ProgressInfoItem.java index 39066765e0..54a6a110e5 100644 --- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/ProgressInfoItem.java +++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/ProgressInfoItem.java @@ -19,7 +19,6 @@ package org.eclipse.ui.internal.progress; import static org.eclipse.swt.events.SelectionListener.widgetSelectedAdapter; -import java.net.URL; import java.text.DateFormat; import java.util.ArrayList; import java.util.Date; @@ -31,7 +30,6 @@ import org.eclipse.core.commands.NotEnabledException; import org.eclipse.core.commands.NotHandledException; import org.eclipse.core.commands.ParameterizedCommand; import org.eclipse.core.commands.common.NotDefinedException; -import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Status; import org.eclipse.core.runtime.jobs.Job; @@ -43,17 +41,11 @@ import org.eclipse.e4.ui.css.swt.theme.IThemeEngine; import org.eclipse.jface.action.IAction; import org.eclipse.jface.dialogs.Dialog; import org.eclipse.jface.dialogs.IDialogConstants; -import org.eclipse.jface.resource.ImageDescriptor; import org.eclipse.jface.resource.JFaceResources; -import org.eclipse.jface.resource.LocalResourceManager; -import org.eclipse.jface.resource.ResourceManager; import org.eclipse.jface.util.Util; import org.eclipse.osgi.util.NLS; import org.eclipse.swt.SWT; -import org.eclipse.swt.events.MouseAdapter; -import org.eclipse.swt.events.MouseEvent; import org.eclipse.swt.graphics.Color; -import org.eclipse.swt.graphics.Image; import org.eclipse.swt.graphics.RGB; import org.eclipse.swt.layout.FormAttachment; import org.eclipse.swt.layout.FormData; @@ -65,7 +57,6 @@ import org.eclipse.swt.widgets.Label; import org.eclipse.swt.widgets.Link; import org.eclipse.swt.widgets.ProgressBar; import org.eclipse.swt.widgets.ToolBar; -import org.eclipse.swt.widgets.ToolItem; import org.eclipse.ui.IWorkbench; import org.eclipse.ui.PlatformUI; import org.eclipse.ui.handlers.IHandlerService; @@ -98,17 +89,17 @@ public class ProgressInfoItem extends Composite { JobTreeElement info; - Label progressLabel; + private Label progressLabel; ToolBar actionBar; - ToolItem actionButton; + //ToolItem actionButton; List<Link> taskEntries = new ArrayList<>(0); private ProgressBar progressBar; - private Label jobImageLabel; + //private Label jobImageLabel; static final int MAX_PROGRESS_HEIGHT = 12; @@ -135,17 +126,17 @@ public class ProgressInfoItem extends Composite { void select(); } - IndexListener indexListener; + //IndexListener indexListener; private int currentIndex; private boolean selected; - private MouseAdapter mouseListener; + //private MouseAdapter mouseListener; private boolean isShowing = true; - private ResourceManager resourceManager; + //private ResourceManager resourceManager; private Link link; @@ -205,21 +196,21 @@ public class ProgressInfoItem extends Composite { FormLayout layout = new FormLayout(); setLayout(layout); - jobImageLabel = new Label(this, SWT.NONE); - Image infoImage = getInfoImage(); - jobImageLabel.setImage(infoImage); - FormData imageData = new FormData(); - if (infoImage != null) { - // position it in the center - imageData.top = new FormAttachment(50, -infoImage.getBounds().height / 2); - } else { - imageData.top = new FormAttachment(0, IDialogConstants.VERTICAL_SPACING); - } - imageData.left = new FormAttachment(0, IDialogConstants.HORIZONTAL_SPACING / 2); - jobImageLabel.setLayoutData(imageData); - - progressLabel = new Label(this, SWT.NONE); - progressLabel.addListener(SWT.Resize, event -> setMainText()); +// jobImageLabel = new Label(this, SWT.NONE); +// Image infoImage = getInfoImage(); +// jobImageLabel.setImage(infoImage); +// FormData imageData = new FormData(); +// if (infoImage != null) { +// // position it in the center +// imageData.top = new FormAttachment(50, -infoImage.getBounds().height / 2); +// } else { +// imageData.top = new FormAttachment(0, IDialogConstants.VERTICAL_SPACING); +// } +// imageData.left = new FormAttachment(0, IDialogConstants.HORIZONTAL_SPACING / 2); +// jobImageLabel.setLayoutData(imageData); + +// progressLabel = new Label(this, SWT.NONE); +// progressLabel.addListener(SWT.Resize, event -> setMainText()); setMainText(); actionBar = new ToolBar(this, SWT.FLAT); @@ -227,44 +218,44 @@ public class ProgressInfoItem extends Composite { // set cursor to overwrite any busy cursor we might have - actionButton = new ToolItem(actionBar, SWT.NONE); - actionButton.setToolTipText(ProgressMessages.NewProgressView_CancelJobToolTip); - actionButton.addSelectionListener(widgetSelectedAdapter(e -> { - actionButton.setEnabled(false); - cancelOrRemove(); - })); +// actionButton = new ToolItem(actionBar, SWT.NONE); +// actionButton.setToolTipText(ProgressMessages.NewProgressView_CancelJobToolTip); +// actionButton.addSelectionListener(widgetSelectedAdapter(e -> { +// actionButton.setEnabled(false); +// cancelOrRemove(); +// })); actionBar.addListener(SWT.Traverse, event -> { - if (indexListener == null) { - return; - } +// if (indexListener == null) { +// return; +// } int detail = event.detail; if (detail == SWT.TRAVERSE_ARROW_NEXT) { - indexListener.selectNext(); + //indexListener.selectNext(); } if (detail == SWT.TRAVERSE_ARROW_PREVIOUS) { - indexListener.selectPrevious(); + //indexListener.selectPrevious(); } }); - updateToolBarValues(); - - FormData progressData = new FormData(); - progressData.top = new FormAttachment(0, IDialogConstants.VERTICAL_SPACING); - progressData.left = new FormAttachment(jobImageLabel, IDialogConstants.HORIZONTAL_SPACING / 2); - progressData.right = new FormAttachment(actionBar, IDialogConstants.HORIZONTAL_SPACING * -1); - progressLabel.setLayoutData(progressData); - - mouseListener = new MouseAdapter() { - @Override - public void mouseDown(MouseEvent e) { - if (indexListener != null) { - indexListener.select(); - } - } - }; - addMouseListener(mouseListener); - jobImageLabel.addMouseListener(mouseListener); - progressLabel.addMouseListener(mouseListener); + //updateToolBarValues(); + +// FormData progressData = new FormData(); +// progressData.top = new FormAttachment(0, IDialogConstants.VERTICAL_SPACING); +// progressData.left = new FormAttachment(jobImageLabel, IDialogConstants.HORIZONTAL_SPACING / 2); +// progressData.right = new FormAttachment(actionBar, IDialogConstants.HORIZONTAL_SPACING * -1); +// progressLabel.setLayoutData(progressData); +// +// mouseListener = new MouseAdapter() { +// @Override +// public void mouseDown(MouseEvent e) { +// if (indexListener != null) { +// indexListener.select(); +// } +// } +// }; +// addMouseListener(mouseListener); +// jobImageLabel.addMouseListener(mouseListener); +// progressLabel.addMouseListener(mouseListener); setLayoutsForNoProgress(); @@ -275,7 +266,7 @@ public class ProgressInfoItem extends Composite { * Set the main text of the receiver. Truncate to fit the available space. */ private void setMainText() { - progressLabel.setText(Dialog.shortenText(getMainTitle(), progressLabel)); + // progressLabel.setText(Dialog.shortenText(getMainTitle(), progressLabel)); } /** @@ -285,14 +276,15 @@ public class ProgressInfoItem extends Composite { private void setLayoutsForNoProgress() { FormData buttonData = new FormData(); - buttonData.top = new FormAttachment(progressLabel, 0, SWT.TOP); + // buttonData.top = new FormAttachment(progressLabel, 0, SWT.TOP); buttonData.right = new FormAttachment(100, IDialogConstants.HORIZONTAL_SPACING * -1); actionBar.setLayoutData(buttonData); if (taskEntries.size() > 0) { FormData linkData = new FormData(); - linkData.top = new FormAttachment(progressLabel, IDialogConstants.VERTICAL_SPACING); - linkData.left = new FormAttachment(progressLabel, 0, SWT.LEFT); + // linkData.top = new FormAttachment(progressLabel, + // IDialogConstants.VERTICAL_SPACING); + // linkData.left = new FormAttachment(progressLabel, 0, SWT.LEFT); linkData.right = new FormAttachment(actionBar, 0, SWT.LEFT); taskEntries.get(0).setLayoutData(linkData); @@ -318,46 +310,46 @@ public class ProgressInfoItem extends Composite { * * @return Image */ - private Image getInfoImage() { - - if (!info.isJobInfo()) { - return JFaceResources.getImage(DEFAULT_JOB_KEY); - } - - JobInfo jobInfo = (JobInfo) info; - - ImageDescriptor descriptor = null; - Object property = jobInfo.getJob().getProperty(IProgressConstants.ICON_PROPERTY); - - if (property instanceof ImageDescriptor) { - descriptor = (ImageDescriptor) property; - } else if (property instanceof URL) { - descriptor = ImageDescriptor.createFromURL((URL) property); - } - - Image image = null; - if (descriptor == null) { - image = ProgressManager.getInstance().getIconFor(jobInfo.getJob()); - } else { - image = getResourceManager().createImageWithDefault(descriptor); - } - - if (image == null) - image = jobInfo.getDisplayImage(); - - return image; - } - - /** - * Return a resource manager for the receiver. - * - * @return {@link ResourceManager} - */ - private ResourceManager getResourceManager() { - if (resourceManager == null) - resourceManager = new LocalResourceManager(JFaceResources.getResources()); - return resourceManager; - } +// private Image getInfoImage() { +// +// if (!info.isJobInfo()) { +// return JFaceResources.getImage(DEFAULT_JOB_KEY); +// } +// +// JobInfo jobInfo = (JobInfo) info; +// +// ImageDescriptor descriptor = null; +// Object property = jobInfo.getJob().getProperty(IProgressConstants.ICON_PROPERTY); +// +// if (property instanceof ImageDescriptor) { +// descriptor = (ImageDescriptor) property; +// } else if (property instanceof URL) { +// descriptor = ImageDescriptor.createFromURL((URL) property); +// } +// +// Image image = null; +// if (descriptor == null) { +// image = ProgressManager.getInstance().getIconFor(jobInfo.getJob()); +// } else { +// image = getResourceManager().createImageWithDefault(descriptor); +// } +// +// if (image == null) +// image = jobInfo.getDisplayImage(); +// +// return image; +// } + +// /** +// * Return a resource manager for the receiver. +// * +// * @return {@link ResourceManager} +// */ +// private ResourceManager getResourceManager() { +// if (resourceManager == null) +// resourceManager = new LocalResourceManager(JFaceResources.getResources()); +// return resourceManager; +// } /** * Get the main title for the receiver. @@ -457,46 +449,46 @@ public class ProgressInfoItem extends Composite { if (isDisposed() || !isShowing) return; - jobImageLabel.setImage(getInfoImage()); - int percentDone = getPercentDone(); - ProgressBar currentProgressBar = progressBar; - +// jobImageLabel.setImage(getInfoImage()); +// int percentDone = getPercentDone(); +// ProgressBar currentProgressBar = progressBar; +// JobInfo[] infos = getJobInfos(); - if (isRunning()) { - if (progressBar == null) { - if (percentDone == IProgressMonitor.UNKNOWN) { - // Only do it if there is an indeterminate task - // There may be no task so we don't want to create it - // until we know for sure - for (JobInfo jobInfo : infos) { - Optional<TaskInfo> optionalInfo = jobInfo.getTaskInfo(); - if (optionalInfo.isPresent() && optionalInfo.get().totalWork == IProgressMonitor.UNKNOWN) { - createProgressBar(SWT.INDETERMINATE); - break; - } - } - } else { - createProgressBar(SWT.NONE); - progressBar.setMinimum(0); - progressBar.setMaximum(100); - } - } - - // Protect against bad counters - if (percentDone >= 0 && percentDone <= 100 && percentDone != progressBar.getSelection()) { - progressBar.setSelection(percentDone); - } - } - - else if (isCompleted()) { - - if (progressBar != null) { - progressBar.dispose(); - progressBar = null; - } - setLayoutsForNoProgress(); - - } +// if (isRunning()) { +// if (progressBar == null) { +// if (percentDone == IProgressMonitor.UNKNOWN) { +// // Only do it if there is an indeterminate task +// // There may be no task so we don't want to create it +// // until we know for sure +// for (JobInfo jobInfo : infos) { +// Optional<TaskInfo> optionalInfo = jobInfo.getTaskInfo(); +// if (optionalInfo.isPresent() && optionalInfo.get().totalWork == IProgressMonitor.UNKNOWN) { +// createProgressBar(SWT.INDETERMINATE); +// break; +// } +// } +// } else { +// createProgressBar(SWT.NONE); +// progressBar.setMinimum(0); +// progressBar.setMaximum(100); +// } +// } +// +// // Protect against bad counters +// if (percentDone >= 0 && percentDone <= 100 && percentDone != progressBar.getSelection()) { +// progressBar.setSelection(percentDone); +// } +// } +// +// else if (isCompleted()) { +// +// if (progressBar != null) { +// progressBar.dispose(); +// progressBar = null; +// } +// setLayoutsForNoProgress(); +// +// } for (int i = 0; i < infos.length; i++) { JobInfo jobInfo = infos[i]; @@ -549,12 +541,12 @@ public class ProgressInfoItem extends Composite { taskEntries.clear(); } - updateToolBarValues(); + //updateToolBarValues(); setMainText(); - if (currentProgressBar != progressBar) { +// if (currentProgressBar != progressBar) { requestLayout(); - } +// } } /** @@ -589,69 +581,69 @@ public class ProgressInfoItem extends Composite { return infos; } - /** - * Return whether or not the receiver is being displayed as running. - * - * @return boolean - */ - private boolean isRunning() { - - for (JobInfo jobInfo : getJobInfos()) { - int state = jobInfo.getJob().getState(); - if (state == Job.WAITING || state == Job.RUNNING) - return true; - } - return false; - } - - /** - * Get the current percent done. - * - * @return int - */ - private int getPercentDone() { - if (info.isJobInfo()) { - return ((JobInfo) info).getPercentDone(); - } - - if (info.hasChildren()) { - Object[] roots = ((GroupInfo) info).getChildren(); - if (roots.length == 1 && roots[0] instanceof JobTreeElement) { - Optional<TaskInfo> optionalInfo = ((JobInfo) roots[0]).getTaskInfo(); - if (optionalInfo.isPresent()) { - return optionalInfo.get().getPercentDone(); - } - } - return ((GroupInfo) info).getPercentDone(); - } - return 0; - } - - /** - * Set the images in the toolbar based on whether the receiver is finished or - * not. Also update tooltips if required. - * - */ - private void updateToolBarValues() { - if (isCompleted()) { - actionButton.setImage(JFaceResources.getImage(CLEAR_FINISHED_JOB_KEY)); - actionButton.setDisabledImage(JFaceResources.getImage(DISABLED_CLEAR_FINISHED_JOB_KEY)); - actionButton.setToolTipText(ProgressMessages.NewProgressView_ClearJobToolTip); - } else { - actionButton.setImage(JFaceResources.getImage(STOP_IMAGE_KEY)); - actionButton.setDisabledImage(JFaceResources.getImage(DISABLED_STOP_IMAGE_KEY)); - - } - - for (JobInfo jobInfo : getJobInfos()) { - // Only disable if there is an unresponsive operation - if (jobInfo.isCanceled() && !isCompleted()) { - actionButton.setEnabled(false); - return; - } - } - actionButton.setEnabled(true); - } +// /** +// * Return whether or not the receiver is being displayed as running. +// * +// * @return boolean +// */ +// private boolean isRunning() { +// +// for (JobInfo jobInfo : getJobInfos()) { +// int state = jobInfo.getJob().getState(); +// if (state == Job.WAITING || state == Job.RUNNING) +// return true; +// } +// return false; +// } +// +// /** +// * Get the current percent done. +// * +// * @return int +// */ +// private int getPercentDone() { +// if (info.isJobInfo()) { +// return ((JobInfo) info).getPercentDone(); +// } +// +// if (info.hasChildren()) { +// Object[] roots = ((GroupInfo) info).getChildren(); +// if (roots.length == 1 && roots[0] instanceof JobTreeElement) { +// Optional<TaskInfo> optionalInfo = ((JobInfo) roots[0]).getTaskInfo(); +// if (optionalInfo.isPresent()) { +// return optionalInfo.get().getPercentDone(); +// } +// } +// return ((GroupInfo) info).getPercentDone(); +// } +// return 0; +// } +// +// /** +// * Set the images in the toolbar based on whether the receiver is finished or +// * not. Also update tooltips if required. +// * +// */ +// private void updateToolBarValues() { +// if (isCompleted()) { +// actionButton.setImage(JFaceResources.getImage(CLEAR_FINISHED_JOB_KEY)); +// actionButton.setDisabledImage(JFaceResources.getImage(DISABLED_CLEAR_FINISHED_JOB_KEY)); +// actionButton.setToolTipText(ProgressMessages.NewProgressView_ClearJobToolTip); +// } else { +// actionButton.setImage(JFaceResources.getImage(STOP_IMAGE_KEY)); +// actionButton.setDisabledImage(JFaceResources.getImage(DISABLED_STOP_IMAGE_KEY)); +// +// } +// +// for (JobInfo jobInfo : getJobInfos()) { +// // Only disable if there is an unresponsive operation +// if (jobInfo.isCanceled() && !isCompleted()) { +// actionButton.setEnabled(false); +// return; +// } +// } +// actionButton.setEnabled(true); +// } /** * Create the progress bar and apply any style bits from style. @@ -668,7 +660,7 @@ public class ProgressInfoItem extends Composite { progressBar = new ProgressBar(this, SWT.HORIZONTAL | style); FormData barData = new FormData(); barData.top = new FormAttachment(actionBar, IDialogConstants.VERTICAL_SPACING, SWT.TOP); - barData.left = new FormAttachment(progressLabel, 0, SWT.LEFT); + // barData.left = new FormAttachment(progressLabel, 0, SWT.LEFT); barData.right = new FormAttachment(actionBar, IDialogConstants.HORIZONTAL_SPACING * -1); barData.height = MAX_PROGRESS_HEIGHT; barData.width = 0;// default is too large @@ -694,12 +686,10 @@ public class ProgressInfoItem extends Composite { if (index >= taskEntries.size()) {// Is it new? link = new Link(this, SWT.NONE); + FormData linkData = new FormData(); if (index == 0 || taskEntries.isEmpty()) { Control top = progressBar; - if (top == null) { - top = progressLabel; - } linkData.top = new FormAttachment(top, IDialogConstants.VERTICAL_SPACING); linkData.left = new FormAttachment(top, 0, SWT.LEFT); linkData.right = new FormAttachment(top, 0, SWT.RIGHT); @@ -864,7 +854,7 @@ public class ProgressInfoItem extends Composite { */ private void setAllForegrounds(Color color) { setForeground(color); - progressLabel.setForeground(color); + // progressLabel.setForeground(color); Iterator<Link> taskEntryIterator = taskEntries.iterator(); while (taskEntryIterator.hasNext()) { @@ -880,9 +870,9 @@ public class ProgressInfoItem extends Composite { */ private void setAllBackgrounds(Color color) { setBackground(color); - progressLabel.setBackground(color); + // progressLabel.setBackground(color); actionBar.setBackground(color); - jobImageLabel.setBackground(color); + //jobImageLabel.setBackground(color); Iterator<Link> taskEntryIterator = taskEntries.iterator(); while (taskEntryIterator.hasNext()) { @@ -917,7 +907,7 @@ public class ProgressInfoItem extends Composite { * @param indexListener */ void setIndexListener(IndexListener indexListener) { - this.indexListener = indexListener; + //this.indexListener = indexListener; } /** @@ -959,8 +949,8 @@ public class ProgressInfoItem extends Composite { @Override public void dispose() { super.dispose(); - if (resourceManager != null) - resourceManager.dispose(); +// if (resourceManager != null) +// resourceManager.dispose(); } /** If I "enable" the progress label, I do see the leak (ran for 3 days and a bit, 2.4 GB RES): --- ProgressInfoItem_good.txt 2021-01-20 09:06:20.138705000 +0100 +++ ProgressInfoItem_bad.txt 2021-01-20 09:05:45.400755000 +0100 @@ -209,8 +209,8 @@ // imageData.left = new FormAttachment(0, IDialogConstants.HORIZONTAL_SPACING / 2); // jobImageLabel.setLayoutData(imageData); -// progressLabel = new Label(this, SWT.NONE); -// progressLabel.addListener(SWT.Resize, event -> setMainText()); + progressLabel = new Label(this, SWT.NONE); + progressLabel.addListener(SWT.Resize, event -> setMainText()); setMainText(); actionBar = new ToolBar(this, SWT.FLAT); @@ -239,11 +239,12 @@ }); //updateToolBarValues(); -// FormData progressData = new FormData(); -// progressData.top = new FormAttachment(0, IDialogConstants.VERTICAL_SPACING); -// progressData.left = new FormAttachment(jobImageLabel, IDialogConstants.HORIZONTAL_SPACING / 2); -// progressData.right = new FormAttachment(actionBar, IDialogConstants.HORIZONTAL_SPACING * -1); -// progressLabel.setLayoutData(progressData); + FormData progressData = new FormData(); + progressData.top = new FormAttachment(0, IDialogConstants.VERTICAL_SPACING); + // progressData.left = new FormAttachment(jobImageLabel, + // IDialogConstants.HORIZONTAL_SPACING / 2); + progressData.right = new FormAttachment(actionBar, IDialogConstants.HORIZONTAL_SPACING * -1); + progressLabel.setLayoutData(progressData); // // mouseListener = new MouseAdapter() { // @Override @@ -266,7 +267,7 @@ * Set the main text of the receiver. Truncate to fit the available space. */ private void setMainText() { - // progressLabel.setText(Dialog.shortenText(getMainTitle(), progressLabel)); + progressLabel.setText(Dialog.shortenText(getMainTitle(), progressLabel)); } /** @@ -276,15 +277,14 @@ private void setLayoutsForNoProgress() { FormData buttonData = new FormData(); - // buttonData.top = new FormAttachment(progressLabel, 0, SWT.TOP); + buttonData.top = new FormAttachment(progressLabel, 0, SWT.TOP); buttonData.right = new FormAttachment(100, IDialogConstants.HORIZONTAL_SPACING * -1); actionBar.setLayoutData(buttonData); if (taskEntries.size() > 0) { FormData linkData = new FormData(); - // linkData.top = new FormAttachment(progressLabel, - // IDialogConstants.VERTICAL_SPACING); - // linkData.left = new FormAttachment(progressLabel, 0, SWT.LEFT); + linkData.top = new FormAttachment(progressLabel, IDialogConstants.VERTICAL_SPACING); + linkData.left = new FormAttachment(progressLabel, 0, SWT.LEFT); linkData.right = new FormAttachment(actionBar, 0, SWT.LEFT); taskEntries.get(0).setLayoutData(linkData); @@ -660,7 +660,7 @@ progressBar = new ProgressBar(this, SWT.HORIZONTAL | style); FormData barData = new FormData(); barData.top = new FormAttachment(actionBar, IDialogConstants.VERTICAL_SPACING, SWT.TOP); - // barData.left = new FormAttachment(progressLabel, 0, SWT.LEFT); + barData.left = new FormAttachment(progressLabel, 0, SWT.LEFT); barData.right = new FormAttachment(actionBar, IDialogConstants.HORIZONTAL_SPACING * -1); barData.height = MAX_PROGRESS_HEIGHT; barData.width = 0;// default is too large @@ -854,7 +854,7 @@ */ private void setAllForegrounds(Color color) { setForeground(color); - // progressLabel.setForeground(color); + progressLabel.setForeground(color); Iterator<Link> taskEntryIterator = taskEntries.iterator(); while (taskEntryIterator.hasNext()) { @@ -870,7 +870,7 @@ */ private void setAllBackgrounds(Color color) { setBackground(color); - // progressLabel.setBackground(color); + progressLabel.setBackground(color); actionBar.setBackground(color); //jobImageLabel.setBackground(color); I'll try to write an SWT snippet with this in mind.
Created attachment 285335 [details] SWT snippet that approximates what the Progress view is doing. For a (non-minimal) SWT snippet, this does what the Progress view does to a lesser degree (no dynamic changes to jobs, no re-ordering of controls, no buttons for cancel). I *think* I see a leak when running this, but I'll need to leave it running for more time. Possibly just spamming the progress info item and disposing it creates a bigger leak, I'll check after I'm sure this snippet actually leaks something. Run with -Xmx100m to prevent the heap from growing "too much" (and so making it unclear whether the snippet is leaking memory).
Thanks, that was helpful. The result of 'Control.getFontDescription()' leaks via 'Label.computeSizeInPixels()'. I'll make a patch.
(In reply to Alexandr Miloslavskiy from comment #14) > Thanks, that was helpful. > > The result of 'Control.getFontDescription()' leaks via > 'Label.computeSizeInPixels()'. Could you share how you found this?
I'll give you a longer answer. I'm a fan of all kinds of verification tools. I have a background low-priority project where I'm trying to run SWT JUnit under ElectricFence, which is a memory checking tool. It works by forcing every native heap block at the end of the virtual memory page, with a guard page after it. This way, it reliably catches these problems: * Reading/writing past block * Reading/writing to deallocated block * Double deallocation Don't start googling ElectricFence, unfortunately it's quite broken, so I had to rewrite it almost completely to fix bugs. On Windows, there's a comparable tool from Microsoft, called Application Verifier. It "just works" and I already ran SWT-win32 under it, and already fixed a couple bugs I found. However, because every heap block goes to its own page, it increases the amount of memory needed by rounding all sizes up to next 4096 boundary. On Windows, that's fine, but GTK loves to allocate thousands of small blocks, which makes the problem a lot worse. When I try to run SWT JUnit, it just eats all available RAM and then dies. So I started fixing memory leaks to be able to run the tool. For that, again on Windows there are great tools that just work, but on Linux it's more complicated. I took Valgrind and then had a few iterations where I ran it and then fixed things in Valgrind itself to make it compatible with Java and Ubuntu. If you're really interested to use it, I can give you my stuff, but it's going to take time to explain everything, so I'd rather not if you're not dedicated. So I made this patched Valgrind and custom script to run it and also custom list of suppressions to ignore "expected" leaks in JVM and GTK. Next, I run the snippet that Simeon provided and got a series of leaks, all of them having 'Java_org_eclipse_swt_internal_gtk_GTK_gtk_1style_1context_1get__JI_3B_3JJ' on native stack. I then put a Java breakpoint on 'gtk_style_context_get' and ran the snippet again. Feel free to ask questions, I'm totally into convincing more people to use tools.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/175189
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/175189 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=786dc7209b71746c6aaa0efda4a9d86f040ffaba
Thanks Alexandr!
You're welcome! I take it, leak is no longer observable in your original environment?
(In reply to Alexandr Miloslavskiy from comment #20) > You're welcome! I take it, leak is no longer observable in your original > environment? Yes.
(In reply to Alexandr Miloslavskiy from comment #16) > I'll give you a longer answer. > ... > Feel free to ask questions, I'm totally into convincing more people to use > tools. Hi Alexandr, where can we start with your improved tooling? Is there a repository with some minimal documentation that we can take a look at (or something similar)? We would like to bother you as little as possible, since its unclear how much time we can spend on even checking if the tools work in our environment. The tools you use do seem very useful though; we would like to try them out (likely at a slow pace), documenting the use process as we go. Best regards and thanks, Simeon
I'm currently on a longer vacation. My expectations is that somewhere in the next 4 weeks I will be able to find time to polish and extract my scripts in a standalone repo so that you can try them.
