Community
Participate
Working Groups
Build ID: I20070625-1500 Steps To Reproduce: 1. Create a Draw2D chart with one parent figure 2. Add to this parent, a lot of children (10000) 3. Observe the time spend in FocuseTraverseManager when a focuse event occurs in the SWT canvas containing the Draw2D chart More information: After several discussions on the Gef-Draw2d mailling list, Randy Hudson told me that this is a known issue: "There's an n2 performance issue in the way that lightweight system handle focus gained" A simple workaround for this consists in telling the parent figure to be focusable. This way the FocuseTraverseManager stops to find a focusable focus on the first figure (the parent).
Hi all, I looked the preorder tree traverse implementation of getNextFocusableFigure(). Randy told me about recursion but the implementation is fully iterative so i didn't understand ... The bottleneck in my case is the following line : int parentIndex = gp.getChildren().indexOf(p); This line is used when there is no sibling to the right of the current node to go up the tree until a node with un-traversed siblings is found. The indexOf method on array list is O(n) with n = list.size() (in the worst case, the looked for element index is the last one, so n comparisons are necessary) I wrote a simple patch, that store for each level of the tree the index of the last visited node. This way instead of find again this index using indexOf we just look inside a map that directly gives the index of the last visited node. This works since the preorder algorithm traverse the tree from left to right. What do you think of such idea ? PS : I will submit my patch soon
Created attachment 75192 [details] Diff between FocusTraverseManager and proposed FocusTraverseManager Proposed patch using a hashmap to sotre for each level the index of last visited node.
Any comments about the supplied patch ? Does any body already encountered this bug , in my case this is quite blocking ? Manuel
Hi all, I am reviewing all the bug i opened in Bugzilla, and of course trying to close them ;o) So, sorry for being repetitive, but does anybody already encountered this bug ? What do you think about the solution ?? Thanks in advance Manu
I haven't looked at the patch closely, but it should be possible to fix it without introducing new fields to the manager. Maybe just a new parameter added to a method somewhere or eliminating recursion and using some local variable holding onto a stack of interesting data.
(In reply to comment #5) > I haven't looked at the patch closely, but it should be possible to fix it > without introducing new fields to the manager. Maybe just a new parameter added > to a method somewhere or eliminating recursion and using some local variable > holding onto a stack of interesting data. > Manuel, are you able to create a new patch for this. We would like a real patch to apply as well, rather than just a diff.
Created attachment 131283 [details] Patch using Team Synchronizing perspective
This patch uses a List instead of a map. Let me know if all is ok. Manuel (In reply to comment #7) > Created an attachment (id=131283) [details] > Patch using Team Synchronizing perspective >
Any news about this patch ? Thanks Manu
Just a ping ;o)
Can you give some light to the performance gain here?
(In reply to comment #11) > Can you give some light to the performance gain here? Hi Anthony, The following line int parentIndex = gp.getChildren().indexOf(p); is replaced by int parentIndex = ((Integer)siblingsPositions.remove(siblingsPositions.size() - 2)).intValue(); Storing for each level of the tree the index of the last visited node allows to faster "go up the tree until a node with un-traversed siblings is found " (comment taken from the documentation in the code) Instead of computing again the index using indexOf we get it directly from the list. Manu