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

Move outline tree computation to background thread #2320

Closed
lwrage opened this issue May 8, 2020 · 11 comments · Fixed by #2454
Closed

Move outline tree computation to background thread #2320

lwrage opened this issue May 8, 2020 · 11 comments · Fixed by #2454

Comments

@lwrage
Copy link
Contributor

lwrage commented May 8, 2020

Summary

Outline tree computation is currently done in a foreground thread, which can block the UI. Xtext has support for updating the outline in the background.

See https://www.eclipse.org/forums/index.php/t/1103639/

related to #1832

Environment

  • OSATE Version: 2.7.1
  • Operating System: all
@lwrage
Copy link
Contributor Author

lwrage commented May 8, 2020

There seems to be no official documentation. Get help on the TMF forum if needed.

@AaronGreenhouse
Copy link
Contributor

AaronGreenhouse commented Sep 21, 2020

So actually, are talking about the AADL textual outline or the AAXL instance model outline or both?

@AaronGreenhouse
Copy link
Contributor

AaronGreenhouse commented Sep 21, 2020

Okay comments on DefaultOutlineTreeProvider say this:

/**
 * The default implementation of an {@link IOutlineTreeProvider}.
 * If you would like to run your outline in Background, please have a look at {@link BackgroundOutlineTreeProvider}.
 * Please note that this would have implications to you LabelProvider implementation. Please read the JavaDoc in the mentioned class carefully.
 * 
 * @author Jan Koehnlein - Initial contribution and API
 */

and BackgroundOutlineTreeProvider says this:

 * It is essential that the {@link ILabelProvider} implements {@link ILabelProviderImageDescriptorExtension} and that
 * the method {@link ILabelProviderImageDescriptorExtension#getImageDescriptor(Object)} does not need to be executed in
 * the {@link Display} thread, e.g. does not create {@link Image} objects internally.

This matches up with the comments in https://www.eclipse.org/forums/index.php/t/1103639/ that stress the importance of using ImageDescriptor.

@lwrage
Copy link
Contributor Author

lwrage commented Sep 21, 2020

@AaronGreenhouse
Copy link
Contributor

AaronGreenhouse commented Sep 21, 2020

So need to change Aadl2OutlineTreeProvider to extend BackgroundOutlineTreeProvider, but first I need to fix Aadl2LabelProvider to also extend ILabelProviderImageDescriptorExtension.

@AaronGreenhouse
Copy link
Contributor

AaronGreenhouse commented Sep 21, 2020

Ugh. SO despite what the comments on DefaultOutlineTreeProvider suggest, you cannot just swap BackgroundOutlineTreeProvider for DefaultOutlineTreeProvider (modulo the image descriptor issue) because of this nonsense:


	protected PolymorphicDispatcher<Void> createChildrenDispatcher = PolymorphicDispatcher.createForSingleTarget(
			"_createChildren", 2, 2, this);

	protected PolymorphicDispatcher<Void> createNodeDispatcher = PolymorphicDispatcher.createForSingleTarget(
			"_createNode", 2, 2, this);

	protected PolymorphicDispatcher<Object> textDispatcher = PolymorphicDispatcher.createForSingleTarget("_text", 1, 1,
			this);

	protected PolymorphicDispatcher<Image> imageDispatcher = PolymorphicDispatcher.createForSingleTarget("_image", 1,
			1, this);

	protected PolymorphicDispatcher<Boolean> isLeafDispatcher = PolymorphicDispatcher.createForSingleTarget("_isLeaf",
			1, 1, this);

And the background tree wants to inject an OutlineNodeFactory

@AaronGreenhouse
Copy link
Contributor

AaronGreenhouse commented Sep 22, 2020

Good news: Aadl2LabelProvider already extends ILabelProviderImageDescriptorExtension!

@AaronGreenhouse
Copy link
Contributor

AaronGreenhouse commented Sep 22, 2020

I think I have this working. Certainly it doesn't seem to be breaking anything, but I'm having a hard time telling if it makes a difference.

Some strange stuff I noticed though: The original Aadl2OutlineTreeProvider has

	protected boolean _isLeaf(IntegerLiteral bpa) {

		if (bpa.eContainer() instanceof RecordValue) {
			return false;
		}
		return false;
	}

In particular this method always returns false. I am wondering if this is an error. Should the return value of the conditional really be true? There are two other methods that do:

	protected boolean _isLeaf(BasicPropertyAssociation bpa) {

		if (bpa.eContainer() instanceof RecordValue) {
			return true;
		}
		return false;
	}

	protected boolean _isLeaf(ReferenceValue bpa) {

		if (bpa.eContainer() instanceof RecordValue) {
			return true;
		}
		return false;
	}

@AaronGreenhouse
Copy link
Contributor

AaronGreenhouse commented Sep 22, 2020

The workspace has the following additional extensions of DefaultOutlineTreeProvider

  • AlisaOutlineTreeProvider
  • CategoriesOutlineTreeProvider
  • CodetemplatesOutlineTreeProvider
  • CommonOutlineTreeProvider
  • ErrorModelOutlineTreeProvider
  • InstanceOutlineTreeProvider
  • ModeAwareOutlineTreeProvider
  • OrganizationOutlineTreeProvider
  • PropertiesOutlineTreeProvider
  • ReqSpecOutlineTreeProvider
  • SingleCodestemplateOutlineTreeProvider
  • TemplatesOutlineTreeProvider
  • VerifyOutlineTreeProvider
  • XbaseOutlineTreeProvider
  • XbaseWithAnnotationsOutlineTreeProvider

I see one annex: ErrorModelOutlineTreeProvider

Some are clearly from the mini languages.

Some I have no idea: xbase, mode aware

The following are trivial extensions and can be just as well trivial extensions of BackgroundOutlineTreeProvider:

  • AlisaOutlineTreeProvider
  • CategoriesOutlineTreeProvider
  • CodetemplatesOutlineTreeProvider
  • CommonOutlineTreeProvider
  • InstanceOutlineTreeProvider
  • OrganizationOutlineTreeProvider
  • PropertiesOutlineTreeProvider
  • TemplatesOutlineTreeProvider
  • VerifyOutlineTreeProvider
  • XbaseOutlineTreeProvider
  • XbaseWithAnnotationsOutlineTreeProvider

ReqSpecOutlineTreeProvider doesn't have a lot going on and should be easy to update.

ErrorModelOutlineTreeProvider shouldn't be too challenging to update either.

@AaronGreenhouse
Copy link
Contributor

AaronGreenhouse commented Sep 23, 2020

Just make that ErrorModelOutlineTreeProvider can run in the background, and also make sure that the AADL outline checks if the annex outline provider is backgroundable.

@AaronGreenhouse
Copy link
Contributor

AaronGreenhouse commented Sep 23, 2020

Updated the ErrorModelOutlineTreeProvider.

Replaced the old Aadl2OutlineTreeProvider with the contents of Aadl2BackgroundOutlineTreeProvider. This better preserves the history of the situation.

@lwrage lwrage added this to the 2.9.0 milestone Sep 24, 2020
@lwrage lwrage closed this as completed Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants