Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Modeling » Graphiti » deletePictogramElement Performance
deletePictogramElement Performance [message #873175] Thu, 17 May 2012 16:23 Go to next message
Craig Petre is currently offline Craig Petre
Messages: 32
Registered: April 2011
Member
I'm seeing some performance problems with deleting elements from, what I would consider, a reasonably large diagram for my application. I've noticed that much of the time seems to be eaten up in PeService.deletePictogramElement, so I took a quick look at it and from what I can understand it seems to be unnecessarily using recursion to iterate over the children of ContainerShapes to call EcoreUtil.delete on each individual node. It seems unnecessary because it seems like EcoreUtil.delete is already taking care of the children. So I did an experiment, I deleted all pictogram elements in a large diagram within my application and it took in the neighborhood of 4 1/2 minutes before the operation completed. On the next run I commented out the lines in PeService.deletePictogramElement which iterated over the children of ContainerShapes and also the part that deletes anchors, so it looks like this:

	public void deletePictogramElement(PictogramElement pe) {
		/*
		 * if (pe instanceof ContainerShape) { ContainerShape cs =
		 * (ContainerShape) pe; List<Shape> childList = cs.getChildren();
		 * List<Shape> l = new ArrayList<Shape>(); for (Shape shape : childList)
		 * { l.add(shape); } for (Shape shape : l) {
		 * deletePictogramElement(shape); } } if (pe instanceof AnchorContainer)
		 * { AnchorContainer ac = (AnchorContainer) pe; Collection<Anchor>
		 * anchorList = ac.getAnchors(); List<Anchor> l = new
		 * ArrayList<Anchor>(); for (Anchor anchor : anchorList) {
		 * l.add(anchor); } for (Anchor anchor : l) {
		 * deletePictogramElement(anchor); } }
		 */
		GraphicsAlgorithm graphicsAlgorithm = pe.getGraphicsAlgorithm();
		if (graphicsAlgorithm != null) {
			EcoreUtil.delete(graphicsAlgorithm, false);
		}
		PictogramLink linkForPictogramElement = Graphiti.getLinkService().getLinkForPictogramElement(pe);
		if (linkForPictogramElement != null) {
			EcoreUtil.delete(linkForPictogramElement, false);
		}
		EcoreUtil.delete(pe, false);
	}


Then reran my test case and the operation took 1 minute and 20 seconds this time and the serialization of the diagram looked correct, so it appears everything was deleted correctly. This solution is probably ignoring some reason why this recursion is there to begin with, which is why I bring it up here. I can go ahead and open up a bug report if this is indeed something that looks like it is worth checking out. I should also note that the PictogramElements in my application usually contain a few dozen children, which is probably why this issue is particularly painful for me.

Thanks,
Craig

[Updated on: Thu, 17 May 2012 16:38]

Report message to a moderator

Re: deletePictogramElement Performance [message #873638 is a reply to message #873175] Fri, 18 May 2012 18:25 Go to previous messageGo to next message
Hernan Gonzalez is currently offline Hernan Gonzalez
Messages: 181
Registered: October 2010
Location: Buenos Aires, Argentina
Senior Member
Ironically, I've just post a probable bug caused by a missing recursion in the same scenario.
In your case, the recursion does not looks really necesary to me (it seems implicit by the PE EMF model), but I'm not sure.
Re: deletePictogramElement Performance [message #875112 is a reply to message #873175] Tue, 22 May 2012 06:59 Go to previous messageGo to next message
Michael Wenz is currently offline Michael Wenz
Messages: 1618
Registered: July 2009
Location: Walldorf, Germany
Senior Member
The numbers you present are really dramatic, so investigating here is
definitely worth some effort...

Since Graphiti needs to generically support all scenarios, we unfortunatly
need both loops because they clean up the links that might exist as a
separate object tree besides the shape hierarchy. These links would
otherwise not be removed by EcoreUtil.delete. Besides, I think most of the
time gain comes from changing the second EcoreUtil.delete parameter to false
not from removing the loops. Did you investigate on that?

In your case you don't seem to need all of the generic handling and it seems
to cost much time. Maybe it would make sense to provide your own delete
feature that is custom-tailored to your pictogram model usage, instead of
relying on the generic functionality?

Michael
Re: deletePictogramElement Performance [message #875342 is a reply to message #875112] Tue, 22 May 2012 14:57 Go to previous messageGo to next message
Craig Petre is currently offline Craig Petre
Messages: 32
Registered: April 2011
Member
Thanks for the feedback. I inadvertently posted the wrong source code, commenting out the recursion AND changing the EcoreUtil to false, breaks the delete. Changing the EcoreUtil to false OR commenting out the recursion results in a functional delete (in my case).

I currently have my own delete feature in which I take care of deleting the underlying business objects and I defer to the generic functionality of the DefaultRemoveFeature for removing the graphical representation of the PE. I kind of like that hierarchy of the delete feature calling the remove feature to clean up the graphical representation. I think for now I will try to take care of both inside my delete feature, but it would be nice if the DefaultRemoveFeature.remove weren't final so I would be able to preserve the approach I'm currently using. Is that something you would consider doing if I were to file a feature request?

Thanks again,
Craig
Re: deletePictogramElement Performance [message #875695 is a reply to message #875342] Wed, 23 May 2012 07:39 Go to previous messageGo to next message
Michael Wenz is currently offline Michael Wenz
Messages: 1618
Registered: July 2009
Location: Walldorf, Germany
Senior Member
Yes, that should be opened up. Please file a Bugzilla for this so we can
still get this into Juno RC2.

Michael


"Craig Petre" schrieb im Newsbeitrag news:jpg9h8$9ii$1@xxxxxxxxe.org...

I think for now I will try to take care of both inside my delete feature,
but it would be nice if the DefaultRemoveFeature.remove weren't final so I
would be able to preserve the approach I'm currently using. Is that
something you would consider doing if I were to file a feature request?

Thanks again,
Craig
Re: deletePictogramElement Performance [message #875877 is a reply to message #875695] Wed, 23 May 2012 13:40 Go to previous message
Craig Petre is currently offline Craig Petre
Messages: 32
Registered: April 2011
Member
BugID: 380400

Thanks.
Previous Topic:DiagramEditorFactory changed to what in 0.9.0?
Next Topic:How do I override equals() of diagram editor input
Goto Forum:
  


Current Time: Sun Oct 26 03:15:56 GMT 2014

Powered by FUDForum. Page generated in 0.02137 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.2.
Copyright ©2001-2010 FUDforum Bulletin Board Software