This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
refactoring build from May10, jdk1.4.2_04, linux P3/800MHz, 512MB Two projects with 40+42 Java files opened. Selecting 20 Java files takes 3-5 seconds. If you select one file and extend selection with CTRL-<down arrrow> couple of times it is better visible that there is a lot of repetead activity. BTW: actually it is not worse that trunk build on the same dataset
> BTW: actually it is not worse that trunk build on the same dataset Radime, please reassign this issue to correct component. As you mentioned before, it has nothing to do with refactoring nor refactoring build. The same problem is in trunk, right? Thanks
I do not know what is right component and we were asked to file bugs found in this build against refactoring module. Feel free to assign to proper category. MDR vs trunk: trunk is much worse for the first usage (may take 30-60 seconds to select 20 nodes). Next attempts are comparable.
From your comments it is obvious that this issue has nothing to do with the new java langauge infrastructure, so I am reassingning this to java module.
Upgrading to P2. This is highly visible problem. BTW, you don't have to have anything open in editor. Just expand some package and select several nodes, then try adding more nodes to the selection. The more nodes you have selected or add to the selection, the more time it takes. The dependency of the time on number of nodes definitely is not linear.
I would like to add a comment: This problem is relating to the number of node selection, not how to select it. Another way to reproduce this is to Click the first node, then hold the shift key, then click the last node (about 100 nodes for example). It will take forever to select all of them.
*** Issue 45248 has been marked as a duplicate of this issue. ***
Created attachment 15964 [details] First part of the problem is recursive refreshing of LookupSensitiveAction(s)
Fixed the recursive refreshing problem: http://www.netbeans.org/source/browse/projects/projectui/src/org/netbeans/modules/project/ui/actions/LookupSensitiveAction.java.diff?r1=text&tr1=1.1&r2=text&tr2=1.2&f=h
Created attachment 15965 [details] Second part of the problem is refreshing of FileCommandAction(s)
Enhancing the fix of LookupSensitiveAction to not refresh other actions from within refreshing of one action: http://www.netbeans.org/source/browse/projects/projectui/src/org/netbeans/modules/project/ui/actions/LookupSensitiveAction.java.diff?r1=text&tr1=1.2&r2=text&tr2=1.3&f=h
The problem with refreshing FileCommandAction(s) is that all such actions are notified about the selection change and refreshed independently of each other and all these action refreshes need to call J2SEActionProvider.isActionEnabled which calls findSources() and findTestSources() and these methods call ActionUtils.findSelectedFiles(). The method findSelectedFiles() is thus called very many times and always returns the same array of FileObjects, but it takes very long time to create the array. (Calling FileUtil.toFile() and FileUtil.isParentOf() very many times is very expensive). I see one way to get out of this problem - cache the array of selected files and return it unxpensively for all the actions being refreshed during one cycle of notifying them about the selection change. So, I suggest the following patch consisting of two changes. Let me know if you agree and I will integrate it. 1) modify ExplorerManager to be able to ask it if it is currently notifying listeners about selection change - attached patch-part1.txt 2) modify ActionUtils.findSelectedFiles() to cache the array of selected files for all the calls within one selection change notification cycle - attached patch-part2.txt
Created attachment 16108 [details] Modified ExplorerManager to be able to ask it if it is currently notifying listeners about selection change
Created attachment 16109 [details] Modified ActionUtils.findSelectedFiles() to cache the array of selected files for all the calls within one selection change notification cycle
As an API change this would need to go through fast-track apireview. Maybe there is some non-API-visible solution that would have a similar effect. (I have not studied this problem in any detail; if you want one of the responsible developers to work on it seriously, assign to them.)
OK, assigning to projects module. This is your area. Please, find a solution without the need of API change or let me know and I'll ask for the API fastraq change and integrate my solution...
Seems to me that most of the talk of how fast ActionUtils etc. is, is missing the point completely. Why is enablement of all these actions even being tested during node selection change at all? The only reason why action enablement should ever be tested during changes in node selection in the Explorer is for actions which appear in visible toolbar buttons, since these may need to turn on or off dynamically. All other actions, living only in KB shortcuts and menu items, do not need to listen to every change; they only need to check enablement when they are displayed. NodeAction behaves this way, or at least it is supposed to, and there are detailed unit tests to check it. But it seems that LookupSensitiveAction, in projects/projectui, does not do this. That is a bug - Petr's code. LSA and its variants should detach all listeners when not visible (which can be checked by whether or not the Action currently has any PropertyChangeListener's); while an action is "visible" (i.e. has >= 1 listener) it should cache its enablement status and fire enablement changes; while not visible, it should keep no cache, and respond to each call to isEnabled() individually. (NodeAction is able to keep some enablement cache based on the last-used node selection, but for a Lookup this will not work well, since the same Lookup object is probably passed each time, but with different contents.) Tim, adding you to CC since we have discussed this kind of thing before. In the future it would be preferable to have a real infrastructure to manage this kind of action enablement logic centrally. For actions extending NodeAction and not doing anything too weird, the logic is handled for you, but if you do not use NodeAction you have to do it from scratch.
Good time to restart our work on actions again (issue 17597)? The LookupSensitiveAction is probably the basic building block of any actions rewrite and it would be reasonable to have it as an API (in a separate module) and write it correctly and not force all the module writers to copy/paste from projectui module and spread all the mistakes around the codebase. As far as I can imagine adding (basic) actions infrastructure module is a compatible addition and might be doable in any promotion.
I agree completely that it would be better to have a general, well-tested infrastructure for "modern" action handling (i.e. not SystemAction), based on Lookup contexts and javax.swing.Action and declarative registration where possible etc.; and projectui should use that when it is available. I think it is unreasonable to start such a thing for D, though; we are past feature freeze. Should talk to Trung about possibly working on it for E.
Agree with Jesse. Adding it for D is probably to late. All teams would have to start using it to get to the desired state. Anyway, the major issue with the performance was not about not refreshing the actions when there is no listener (although it helped as well), it was about my pretty stupid implementation of getProjects/FilesFromLookup() methods, which caused the refresh to be something like to m x n. Where m was number of actions and n number of selected nodes. I'm about to fix it soon. Including some tests for the sensitive actions. It still won't be perfect but should be much better. Some of the problems which won't be fixed: 1) SystemAction wrapers listening on some action. 2) Listening on more than one Lookup.Result and thus reacting more than once to one change in Lookup.
Should no longer depend on number of selected nodes. Checking in projectui/src/org/netbeans/modules/project/ui/actions/ActionsUtil.java; /cvs/projects/projectui/src/org/netbeans/modules/project/ui/actions/ActionsUtil.java,v <-- ActionsUtil.java new revision: 1.2; previous revision: 1.1 done Checking in projectui/src/org/netbeans/modules/project/ui/actions/FileCommandAction.java; /cvs/projects/projectui/src/org/netbeans/modules/project/ui/actions/FileCommandAction.java,v <-- FileCommandAction.java new revision: 1.4; previous revision: 1.3 done Checking in projectui/src/org/netbeans/modules/project/ui/actions/LookupSensitiveAction.java; /cvs/projects/projectui/src/org/netbeans/modules/project/ui/actions/LookupSensitiveAction.java,v <-- LookupSensitiveAction.java new revision: 1.4; previous revision: 1.3 done Checking in projectui/src/org/netbeans/modules/project/ui/actions/ProjectAction.java; /cvs/projects/projectui/src/org/netbeans/modules/project/ui/actions/ProjectAction.java,v <-- ProjectAction.java new revision: 1.2; previous revision: 1.1 done
Cool. There is not the brutal dependency on number of selected nodes anymore which produced times of many seconds for just some 30 nodes. The results on my machine (Dell Precision 220, PIII 800MHz, 512MB RAM, Windows 2000, JDK 1.4.2_04) are now: 10 java nodes = 235ms 20 java nodes = 250ms 30 java nodes = 296ms Anyway, reopening this issue because we should get under 100ms.
But no longer P2 I suppose.
FWIW, there's a proof of concept in performance/actionsframework which, while not perfectly what we'd need, may point in a positive direction: The idea is some kind of hash or bitmask, which represents all objects in the current lookup (say, by class not by actual object). An action can do a very fast test to determine if anything has actually changed that is relevant to it. If so, it can then optionally do a more thorough test (in other words, since hashing algorithms are imperfect, it can be implemented such that there can be false "something has changed"'s but no false "nothing has changed"'s) and change its state as needed. The point being, that the IDE is going through a lot of effort to figure out that nothing has changed when, say, the caret is moved in the editor. The other semi interesting thing in performance/actionsframework is that in fact you can use frequent asynchronous polling to update the state of actions, rather than doing it synchronously on any change, with no perceptible degredation in performance (and this also provides some coalescing of rapid changes for free).
Removed the listeners from SystemAction wrappers in projects and java Checking in projects/projectui/src/org/netbeans/modules/project/ui/actions/Actions.java; /cvs/projects/projectui/src/org/netbeans/modules/project/ui/actions/Actions.java,v <-- Actions.java new revision: 1.16; previous revision: 1.15 done
Same thng needs to be done in webapps. Thus moving to you guys. Will attach a patch file. After you apply the patch please close the bug.
Created attachment 17944 [details] Proposed patch for webapps
Target milestone to TBD, so we don't forget.
proposed patch integrated into web/project: Checking in org/netbeans/modules/web/project/WebProjectModule.java; /cvs/web/project/src/org/netbeans/modules/web/project/WebProjectModule.java,v <-- WebProjectModule.java new revision: 1.9; previous revision: 1.8 done
Remeasured. The results on my machine (Dell Precision 220, PIII 800MHz, 512MB RAM, Windows 2000, JDK 1.4.2_04) for regular java nodes are now: 10 java nodes = 94ms 20 java nodes = 125ms 30 java nodes = 156ms (see the numbers above for the times before Hrebejk's fix)