Community
Participate
Working Groups
Just a couple of impression that I got when implementing an outline structure for a language: The overall implementation of the outline has become far too complex and limited. The API is to complicated for the problem that has to be solved. A redesign of the outline API should keep the following constraints in mind: - There may be nodes without an associated EObject - There may be nodes that point to an EStructuralFeature.Setting (list of imports, name attribute) - It should be possible to compute a region for a node from its children or set it explicitly, regions may overlap. - We should have an expression interpreter that allows to write something in the plugin.xml that is similar to <test property="outlineNode.eclass" value="org.eclipse.xtext.Grammar" /> - It should be straight forward to implement the adapter pattern for the outline nodes of one particular language without any influence on other outlines - QuickOutline and Outline may show different trees - It should be easy to register actions for the outline itself (sort, filter, viewpoints, whatever) and for the actual nodes. - The outline may not require an eager computation of every node. - It has to be easy to implement label decorations such as adding error annotations to the outline nodes. We have to keep all the stuff in mind, that has already been solved, e.g. preserving the expansion state after editing the model, styled string support.
Performance on big models is also an issue. While an delayed update of the outline is tolerable the current implementation blocks the Display thread for too long. I tested a virtual tree with an ILazyTreeContentProvider. Expanding a node with many children is fast enough for interactivity as children are computed on demand when they become visible. But resetting the expansion state after a model change can be a lot slower than with a plain ITreeContentProvider. Especially if many expanded nodes have to be created, e.g. when the current selection is the last item of a huge list. All preceding siblings of an expanded tree have to be materialized, and invisible siblings are never virtualized again. So we might gain interactivity, but not scalability by using virtual trees. Summing up, a virtual lazy tree seems to be for endless, lazy fetched data, such as database query results, rather than for big models. The ITreeContentProvider also creates items on demand, but all children of an expanded node at once. This should be sufficient for most cases. Profiling on a simplified outline tree reveals that refreshing the viewer can be neglected compared to the restoration of the expanded nodes (setExpandedElements) as this is where everything is created. About half the time there is spent on SWT stuff and half on the creation of IOutlineNodes (our implementation of the outline model). All SWT related stuff has to run in the Display thread and there is not much we can improve here. I'll try to move outline node model creation into a non-UI Job. We can also improve on label providers, especially on image resolution. Profiling also revealed that Xtext opening and closing transactions don't really deteriorate execution time. So transactions will be used for consistency, but we should try to encapsulate them to keep the API as easy as possible.
Sounds good to me.
First shot committed. The outline nodes now follow a more object oriented pattern. Outline nodes are pre-computed in a job before updating the UI in the Display thread, reducing display blocking times by around 50%. Sorting and filtering is still done on the JFace level (ViewerSorter and ViewerFilter). Sorting a big outline takes rather long. We should experiment with doing that on the model level, too. Or at least in the viewer's content provider. API has changed quite a bit. The main component a user has to implement is called the IOutlineTreeProvider. It has a method to create the root node and another one to create child nodes of an existing node. All of this is called from within a transaction that is started/committed from the outside, so we can access the model resource safely within these methods. We can now create any number of nodes for any element in any position of the tree. The default implementation uses polymorphic dispatch create nodes and children. IOutlineNode can now have two text regions to support "link with editor": The small one gets selected when the outline node is selected, the large one selects the outline node if the cursor in the editor is inside. I also updated the outline implementation for Xtext, MWE2, and the examples. I will write some tests now, getting a better feeling for the API.
Second version committed 7a8880da5fa598c1eec4cd3897c81888429a768e Refactored IOutlineTreeProvider to make it easier to customize. IOutlineNodes are ow a more directly related to EObjects. The DefaultOutlineTreeProvider is now also responsible for calculating images and texts, making the annotation qualifier @OutlineLableProvider obsolete. We have four dispatchers in the default impl: - doCreateChildren(IOutlineNode parent, EObject modelElement) - doCreateNode(IOutlineNode parent, EObject modelElement) - text(EObject) - image(EObject) What still bothers me is that defining a leaf currently requires two changes, implementing an empty doCreateChildren() and setting isLeaf in doCreateNode(). We could add another dispatcher for isLeaf(EObject) and not call the dispatcher in createChildren if it returns true. I am also not too happy with having another interface IOutlineTreeStructureProvider (and the duplicate binding of the DefaultOutlineTreeProvider) but I don't want to code against the DefaultOutlineTreeProvider as users might want to replace that with a non dispatching implementation to improve performance. I tried to implement a sensible filter for the Xtext grammar language, but expansion / selection recovery clearly needs more work.
Added an isLeaf() dispatcher.
Moved filtering and sorting into the OutlineNodeContentProvider and added a bunch of tests. I guess that's enough for this ticket. Nearly all issues have been solved. Closing this ticket.
Closing all bugs that were set to RESOLVED before Neon.0
