Bugzilla – Full Text Bug Listing
| Summary: | CompositeNode.getTotalOffset too slow | ||
|---|---|---|---|
| Product: | [Modeling] TMF | Reporter: | Mark Christiaens <mark.g.j.christiaens> |
| Component: | Xtext | Assignee: | Project Inbox <tmf.xtext-inbox> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | mark.g.j.christiaens, sebastian.zarnekow, sven.efftinge |
| Version: | 2.0.0 | Flags: | sven.efftinge:
indigo+
|
| Target Milestone: | M5 | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Whiteboard: | |||
Thanks Mark. Your detailed feedback is very helpful. Just to make sure that you guys are not off on a wild goose chase. I've instrumented the getTotalOffset function and it seems that most of the time it ends up in the expensive fall back code. That probably means that it's something else than handling the root node that is causing the problem. Don't know what though. Adding the return 0; line does break some of the navigation (F3) so that's definitely no fix. Hi Mark, the root node will return 0 immediatly (see RootNode#getTotalOffset). I'd assume that your model has siblings but is the last sibling in many cases so basicHasNextSibling() will return false. I'd hoped that there is no real need to check for basicHasPreviousSibling, too because it requires some more code - one has to ensure that two adjacent nodes do not overflow the stack by subsequent calls to nextSibling and previousSibling. I'll definitely look into this. Thanks for the valuable report. (In reply to comment #3) > Hi Mark, > > the root node will return 0 immediatly (see RootNode#getTotalOffset). I'd > assume that your model has siblings but is the last sibling in many cases so > basicHasNextSibling() will return false. > I'd hoped that there is no real need to check for basicHasPreviousSibling, too > because it requires some more code - one has to ensure that two adjacent nodes > do not overflow the stack by subsequent calls to nextSibling and > previousSibling. > I'll definitely look into this. Thanks for the valuable report. I was looking at this again and I wonder if it's just a question of fixing one if statement (the one that climbs to a node with siblings): public int getTotalOffset() { if (firstChild != null) return firstChild.getTotalOffset(); CompositeNode compositeWithSiblings = this; while(!compositeWithSiblings.basicHasNextSibling() && compositeWithSiblings.basicGetParent() != null) { compositeWithSiblings = compositeWithSiblings.basicGetParent(); } if (compositeWithSiblings.basicHasNextSibling()) { AbstractNode sibling = compositeWithSiblings.basicGetNextSibling(); return sibling.getTotalOffset(); } // expensive fallback - should never happen in a valid node model BidiTreeIterator<INode> iter = getRootNode().getAsTreeIterable().iterator(); ILeafNode lastSeen = null; while(iter.hasNext()) { INode next = iter.next(); if (next == this) { if (lastSeen == null) return 0; else return lastSeen.getTotalEndOffset(); } if (next instanceof ILeafNode) { lastSeen = (ILeafNode) next; } } return 0; } Mark, that should do the trick. I'll have a second look at it but at a first glance it looks good to me. Thanks a lot, Sebastian Thanks for the patch. I've pushed it to master. Closing all bugs that were set to RESOLVED before Neon.0 Closing all bugs that were set to RESOLVED before Neon.0 |
Build Identifier: I20101028-1441 Working with the daily build of Xtext 2.0. When opening a big VHDL file (> 2 MB) and working around a number of performance issues already reported, my file does not open. The Xtext editor is busy in CompositeNode.getTotalOffset Name Time (ms) Invocation Count ... org.eclipse.xtext.ui.editor.model.XtextDocument.readOnly(IUnitOfWork) 264052 1 org.eclipse.xtext.util.concurrent.AbstractReadWriteAcces.readOnly(IUnitOfWork) 264052 1 org.eclipse.xtext.ui.editor.folding.DefaultFoldingRegionProvider$1.exec(Object) 264052 1 org.eclipse.xtext.ui.editor.folding.DefaultFoldingRegionProvider$1.exec(XtextResource) 264052 1 org.eclipse.xtext.ui.editor.folding.DefaultFoldingRegionProvider.doGetFoldingRegions(IXtextDocument, XtextResource) 264052 1 org.eclipse.xtext.ui.editor.folding.DefaultFoldingRegionProvider.addFoldingRegions(IXtextDocument, EObject, List) 264047 258 org.eclipse.xtext.ui.editor.folding.DefaultFoldingRegionProvider.getPosition(IXtextDocument, ICompositeNode) 264044 258 org.eclipse.xtext.nodemodel.impl.AbstractNode.getOffset() 132059 516 org.eclipse.xtext.nodemodel.impl.CompositeNode.getTotalOffset() 132055 22 org.eclipse.xtext.nodemodel.impl.CompositeNode.getTotalOffset() 132055 22 org.eclipse.xtext.nodemodel.util.NodeTreeIterator.hasNext() 99855 25857356 org.eclipse.xtext.nodemodel.util.NodeTreeIterator.next() 32199 25857356 ... The code seems to be executing in the code after the "should never happen" comment. public int getTotalOffset() { if (firstChild != null) return firstChild.getTotalOffset(); CompositeNode compositeWithSiblings = this; while(!compositeWithSiblings.hasSiblings() && compositeWithSiblings.basicGetParent() != null) { compositeWithSiblings = compositeWithSiblings.basicGetParent(); } if (compositeWithSiblings.basicHasNextSibling()) { AbstractNode sibling = compositeWithSiblings.basicGetNextSibling(); return sibling.getTotalOffset(); } // expensive fallback - should never happen in a valid node model BidiTreeIterator<INode> iter = getRootNode().getAsTreeIterable().iterator(); ILeafNode lastSeen = null; while(iter.hasNext()) { INode next = iter.next(); if (next == this) { if (lastSeen == null) return 0; else return lastSeen.getTotalEndOffset(); } if (next instanceof ILeafNode) { lastSeen = (ILeafNode) next; } } return 0; } I suspect that the getTotalOffset method was invoked on the root node of the tree (but I'm not sure)? I would expect that node not to have siblings or parents which goes into the expensive fallback code. I put a "return 0;" before the expensive callback and I could open my big file and the code folds seem to have been constructed correctly. It still takes a while but that's better than nothing :) Reproducible: Always