Very slow handling of SWT messages when millions of messages queued

[jukzi]

During eclipse-pde/eclipse.pde#53 is see terrible hotspots in

• org.eclipse.swt.widgets.Synchronizer.addLast ()
• org.eclipse.swt.graphics.Device.isDisposed ()
• org.eclipse.swt.widgets.Synchronizer.asyncExec ()
• Display.findDisplay ()

while the amount of messages in org.eclipse.swt.widgets.Synchronizer.addLast(RunnableLock) was > 1million.

All those functions show "synchronized" blocks, which could be probably avoided with the usage of appropriate classes of java.util.concurrent.

Sampling of SWT Thread:

And asyncExec() seen in a sampling of a background thread:

[merks]

Good finding! My sense is that org.eclipse.pde.internal.ui.editor.context.InputContextManager.resourceChanged(IResourceChangeEvent) should just build sets of added and removed resources and then do a single asyncExec in which it calls org.eclipse.pde.internal.ui.editor.context.InputContextManager.structureChanged(IFile, boolean) for each of those added/removed things. Then likely all the other methods will not be a significant performance concern anymore.

[vogella]

PR request are welcome to replace the sync blocks, I assume that would benefit all parts of SWT.

[iloveeclipse]

Most parts of SWT don't use any sync blocks, because SWT is single threaded. There are only few that contact with the "outer" world :-)

[merks]

@vogella

Of course improvements are always welcome. :-) But no amount of improvement will make doing asyncExec per changed resource when processing a resource delta as reasonable design! :-P That's definitely just bad design and should be fixed.

@jukzi Are you planning on fixing the "too many asyncExecs" problem?"

[vogella]

@vogella

Of course improvements are always welcome. :-) But no amount of improvement will make doing asyncExec per changed resource when processing a resource delta as reasonable design! :-P That's definitely just bad design and should be fixed.

Of course but this discovery is still good as it may speed up SWT in general.

[jukzi]

@jukzi Are you planning on fixing the "too many asyncExecs" problem?"

Not at the moment. I submitted an Issue on PDE for that so that hopefully PDE team could take a look. I will take a short look if i find a quick improvement for SWT meanwhile (my build is still running dequeuing the events after hours 😒 ).

[merks]

@vogella

I have a fix but should I hold off? I recall there is a merge that should happen soon.

I'll paste the changes in org.eclipse.pde.internal.ui.editor.context.InputContextManager.resourceChanged(IResourceChangeEvent) here so I don't loose them:

	@Override
	public void resourceChanged(IResourceChangeEvent event) {
		IResourceDelta delta = event.getDelta();

		try {
			List<IFile> added = new ArrayList<>();
			List<IFile> removed = new ArrayList<>();
			delta.accept(delta1 -> {
				IResource resource = delta1.getResource();
				if (resource instanceof IFile) {
					int kind = delta1.getKind();
					if (kind == IResourceDelta.ADDED)
						added.add((IFile) resource);
					else if (kind == IResourceDelta.REMOVED)
						removed.add((IFile) resource);
					return false;
				}
				return true;
			});
			if (!added.isEmpty() || !removed.isEmpty()) {
				asyncStructureChanged(added, removed);
			}
		} catch (CoreException e) {
			PDEPlugin.logException(e);
		}
	}

	private void asyncStructureChanged(List<IFile> added, List<IFile> removed) {
		if (editor == null || editor.getEditorSite() == null)
			return;
		Shell shell = editor.getEditorSite().getShell();
		Display display = shell != null ? shell.getDisplay() : Display.getDefault();

		display.asyncExec(() -> {
			for (IFile file : added) {
				structureChanged(file, true);
			}
			for (IFile file : removed) {
				structureChanged(file, false);
			}
		});
	}

[merks]

Sorry, I see the merge already happened!

[jukzi]

@merks i propose you create a PDE PR for eclipse-pde/eclipse.pde#53
thanks for working on it!

[vogella]

Sorry, I see the merge already happened!

PDE merge is still pending but please do not hold of any commits to PDE, we plan to merge pde build into pde next Monday and will be able to handle any work, so do not worry.