Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opening big Trees is slow #123

Open
2 tasks
fedejeanne opened this issue Sep 23, 2024 · 6 comments
Open
2 tasks

Opening big Trees is slow #123

fedejeanne opened this issue Sep 23, 2024 · 6 comments
Assignees
Labels
Performance A Request for a Performance Improvement Platform UI Issue for Platform UI Student An Issue to be Addressed By a Student SWT Issue for SWT

Comments

@fedejeanne
Copy link

fedejeanne commented Sep 23, 2024

Problem

Related to vi-eclipse/Eclipse-JDT#3

Opening big/deep Trees can be quite slow e.g. when showing deep hierarchies the Hierarchy View. For example when opening the hierarchy of MECU (meta-model class), it can take some minutes to do it and the UI is frozen.

Goal

  • No UI freeze when opening big trees (hierarchies)

Steps

  • Reproduce the problem and provide a Visual-VM sample that shows where the problem lies
  • Evaluate possible approaches to deal with the issue e.g. doing some processing outside of the UI thread or making the whole thing cancelable (like when one opens the call hierarchy of a method)

Hint

  • There are hints of it in the SWT Book by Northover and Wilson (Chapter 9, section "Using SWT.Expand to Fill a Tree Lazily").
  • Look at the method org.eclipse.jface.viewers.AbstractTreeViewer.internalConditionalExpandToLevel(Widget, int, Function<Widget, Boolean>), there's currently a hint in the code that points to that but sadly a better implementation can not be provided because it wouldn't work on Linux:
Method
	private void internalConditionalExpandToLevel(Widget widget, int level,
			Function<Widget, Boolean> shouldChildrenExpand) {
		if (level == ALL_LEVELS || level > 0) {
			Object data = widget.getData();
			if (widget instanceof Item it && data != null && !isExpandable(it, null, data)) {
				return;
			}
			createChildren(widget, false);
			// XXX for performance widget should be expanded after expanding children:
			if (widget instanceof Item it) {
				setExpanded(it, true);
			}
			if (level == ALL_LEVELS || level > 1) {
				Item[] children = getChildren(widget);
				if (children != null) {
					int newLevel = (level == ALL_LEVELS ? ALL_LEVELS
							: level - 1);
					for (Item element : children) {
						if (shouldChildrenExpand.apply(widget).booleanValue()) {
							internalConditionalExpandToLevel(element, newLevel, shouldChildrenExpand);
						}
					}
				}
			}
			// XXX expanding here fails on linux
		}
	}

You can change the implementation (move the if (widget instanceof Item it) {...} to be executed after the other if) and use the test snippet below to see how the change breaks the implementation on Linux

Test snippet
public class Main {

	static interface I1 {}
	static interface I2 extends I1 {}
	static interface I3 extends I2 {}
	static interface I4 extends I3 {}
	static interface I5 extends I4 {}
	
	static class C2 implements I2 {}
	static class C3 implements I3 {}
}

You'll see for example that showing the hierarchy of I3 produces different results.

This is with the current (slow) implementation:

Image

This is with the modified (fast) implementation, which wouldn't work properly on Linux:

Image

@fedejeanne fedejeanne converted this from a draft issue Sep 23, 2024
@fedejeanne fedejeanne added Platform UI Issue for Platform UI SWT Issue for SWT Performance A Request for a Performance Improvement Student An Issue to be Addressed By a Student labels Sep 23, 2024
@jannisCode jannisCode self-assigned this Oct 11, 2024
@christianstaib christianstaib self-assigned this Oct 25, 2024
@jannisCode
Copy link

I think I got a solution for the Problem. In the class TypeHierarchyViewPart.java and Method updateViewers() you have to put the updateHierarchyViewer(true) after updateMethodViewer(root) because otherwise it updates the wrong thing.
Then expanding the children later should not be a Problem anymore (I tested it on linux and windows)

@jannisCode
Copy link

I need help for testing the changes. I tried using VisualVM, but it didn´t work the way I wanted, because I can´t see any differences between the old and new implementation.

Also I tried measuring the time t takes to run the method, but I didn´t really see any difference between the old and new implementation.

@fedejeanne
Copy link
Author

@jannisCode testing for performance might be trickier than one expects because this problem is only visible when opening big hierarchies in the Type Hierarchy view. You could try opening the hierarchy of say AutoCloseable or Serializable. If you don't see much of a performance improvement there then you can ping me and we can try to hack the change into our own Eclipse and test it with our meta-classes.

@jannisCode
Copy link

jannisCode commented Nov 19, 2024

@fedejeanne @christianstaib So i tested the changes with the help of a modified version of Snippet002TreeViewer and with the old implementation, the average runtime was 4384 ms and with the new implementation, the average was 4370 ms so overall a constant but not that huge difference.
Below is the test snippet I used

Snippet

package org.eclipse.jface.snippets.viewers;

import java.util.ArrayList;
import java.util.List;

import org.eclipse.jface.viewers.ITreeContentProvider;
import org.eclipse.jface.viewers.LabelProvider;
import org.eclipse.jface.viewers.TreeViewer;
import org.eclipse.swt.SWT;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.Tree;
import org.eclipse.swt.widgets.TreeColumn;

public class Snippet002TreeViewer {
	private static class MyContentProvider implements ITreeContentProvider {

		@Override
		public Object[] getElements(Object inputElement) {
			return ((MyModel)inputElement).child.toArray();
		}

		@Override
		public Object[] getChildren(Object parentElement) {
			return getElements(parentElement);
		}

		@Override
		public Object getParent(Object element) {
			if( element == null) {
				return null;
			}

			return ((MyModel)element).parent;
		}

		@Override
		public boolean hasChildren(Object element) {
			return ((MyModel)element).child.size() > 0;
		}

	}

	public static class MyModel {
		public MyModel parent;
		public List<MyModel> child = new ArrayList<>();
		public int counter;

		public MyModel(int counter, MyModel parent) {
			this.parent = parent;
			this.counter = counter;
		}

		@Override
		public String toString() {
			String rv = "Item ";
			if( parent != null ) {
				rv = parent + ".";
			}

			rv += counter;

			return rv;
		}
	}

	public Snippet002TreeViewer(Shell shell) {
		final TreeViewer v = new TreeViewer(shell);
		v.setLabelProvider(new LabelProvider());
		v.setContentProvider(new MyContentProvider());
		createColumn(v.getTree(), "Values");
		v.setInput(createModel());
	}

	public void createColumn(Tree tr, String text) {
		TreeColumn column = new TreeColumn(tr, SWT.NONE);
		column.setWidth(100);
		column.setText(text);
		tr.setHeaderVisible(true);
	}

	private MyModel createModel() {

		MyModel root = new MyModel(0,null);
		root.counter = 0;

		MyModel tmp;
		System.out.println("here");
		for (int i = 1; i < 15000; i++) {
			tmp = new MyModel(i, root);
			root.child.add(tmp);
			for( int j = 1; j < i; j++ ) {
				tmp.child.add(new MyModel(j,tmp));

			}
		}

		return root;
	}

	public static void main(String[] args) {
		long time = 0;
		System.out.print(time);
		for (int i = 0; i < 50; i++) {
			long startTime = System.currentTimeMillis();
			Display display = new Display();
			Shell shell = new Shell(display);
			shell.setLayout(new FillLayout());
			new Snippet002TreeViewer(shell);
			shell.open();

			display.dispose();
			long endTime = System.currentTimeMillis();
			time = time + (endTime - startTime);
			System.out.println("Durchlauf " + i + ": " + (endTime - startTime));
		}
		System.out.println(time / 50);
	}
}

@fedejeanne
Copy link
Author

@jannisCode Thank you for the analysis.

I sampled the latest I-BUILD (20241118-1800) and opened the type hierarchy of our meta-class MECU. It took ~4 minutes to do it and the UI froze.

I-BUILD_20241118-1800_Open TH of MECU.zip

I can try again with your changes if you provide me a patch or you tell me were to edit the code. Who knows, maybe there is some improvement.

@HeikoKlare
Copy link
Contributor

I sampled the latest I-BUILD (20241118-1800) and opened the type hierarchy of our meta-class MECU. It took ~4 minutes to do it and the UI froze.

Slow performance of quick outline and type hierarchy is als a JDT issue, see vi-eclipse/Eclipse-JDT#2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Performance A Request for a Performance Improvement Platform UI Issue for Platform UI Student An Issue to be Addressed By a Student SWT Issue for SWT
Projects
Status: 🏗 In Work: Short
Development

No branches or pull requests

4 participants