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.
[dev-200404191800, JDK 1.4.2_04, Linux RH8] After couple minutes of working with IDE, it became almost unusable due to responsivness problems. See thread dumps taken when doing actions such as: - selecting another file in editor - switching between editor and explorer - Alt G - opening file Restart fixes the problem for some time.
Created attachment 14496 [details] thread dumps
I had opened 3-4 projects and was working with them.
Mentioned actions took between 5-10 sec.
Raising to P1, since it's reproducible and the IDE is really unusable.
Really strange - responsivness of Main Window is excelent. It seems to be somehow related to files or filesystems.
Created attachment 14498 [details] thread dumps
Petre, Yarda is on training. Please do something about this. Looks pretty scary to me. Thanks
Always related to firing PROP_ACTIVATED from TopComponent.Registry, Always computing inside a Lookup provided by org.netbeans.modules.openide.windows.GlobalActionContextImpl. The SimpleProxyLooup updates the results after delegate change. The problem is either in the number of active results or in some badly behaving TC that creates new Lookup instance for each query. Still investigating...
*** Issue 42256 has been marked as a duplicate of this issue. ***
Cannot reproduce on yesterday's sources, easily reproduce on current sources. The problem is growing number of Lookup.Results that gets updated. Yesterday: constantly 2 entries. Today: growing number - hundreds of entries. Going to identify the culprit (who keeps them)
OK, so here we go: The problem is triggerred by phrebejk's integration of projects/projectui/src/org/netbeans/modules/project/ui/actions/LookupSensitiveAction.java but the actual problem lies in implementation of SimpleProxyLookup. For each query, the SPL creates new instance of ProxyResult. Once the PR is asked for content (always just after creation), it attaches itself as a strong listener to the delegate's result. It also keeps strong reference to the delegate. If there is a single strong reference to a single SPL$PR instance, all ever created results are strong referenced as the instance references the delegate, delegate references (through EventListenerList) all other results, and even if the original result (or even the original lookup) changes, each (now probably useless) SPL$SR re-attaches itself to the new delegate.
I've implemented a hotfix that improves the situation using WeakListener, but it is just a hotfix and needs to be changed, because it still allows (in extreme case of only switching TCs) the buildup of several hundred results between GC runs. Keeping this open, P2, reassigned to Lookup owner for correct fix. Correct fix: either reuse the same instance of Lookup.Result for the same template or use another technique to prevent buildup of results (even weakly) attached to another result. Note: The javadoc for SPL$PR talks about empty add/removeLookupListener, which is both not true and suggests that the original use case for the SPL was meant to be different....
The actual problem is in implementation of the action in projects module. I have filled issue 42436 to rewrite that.
Missing tests written. Checking in openide/test/unit/src/org/openide/util/lookup/AbstractLookupArrayStorageTest.java; /cvs/openide/test/unit/src/org/openide/util/lookup/AbstractLookupArrayStorageTest.java,v <-- AbstractLookupArrayStorageTest.java new revision: 1.3; previous revision: 1.2 done Checking in openide/test/unit/src/org/openide/util/lookup/AbstractLookupBaseHid.java; /cvs/openide/test/unit/src/org/openide/util/lookup/AbstractLookupBaseHid.java,v <-- AbstractLookupBaseHid.java new revision: 1.8; previous revision: 1.7 done Checking in openide/test/unit/src/org/openide/util/lookup/AbstractLookupTest.java; /cvs/openide/test/unit/src/org/openide/util/lookup/AbstractLookupTest.java,v <-- AbstractLookupTest.java new revision: 1.26; previous revision: 1.25 done Checking in openide/test/unit/src/org/openide/util/lookup/LookupsProxyTest.java; /cvs/openide/test/unit/src/org/openide/util/lookup/LookupsProxyTest.java,v <-- LookupsProxyTest.java new revision: 1.5; previous revision: 1.4 done Checking in openide/test/unit/src/org/openide/util/lookup/ProxyLookupTest.java; /cvs/openide/test/unit/src/org/openide/util/lookup/ProxyLookupTest.java,v <-- ProxyLookupTest.java new revision: 1.6; previous revision: 1.5 done RCS file: /cvs/openide/test/unit/src/org/openide/util/lookup/SimpleProxyLookupIssue42244Test.java,v done Checking in openide/test/unit/src/org/openide/util/lookup/SimpleProxyLookupIssue42244Test.java; /cvs/openide/test/unit/src/org/openide/util/lookup/SimpleProxyLookupIssue42244Test.java,v <-- SimpleProxyLookupIssue42244Test.java initial revision: 1.1
this is not fixed. Only a few tests were written. Yarda, you should add your comment here instead of filing a new bug and perhaps reassign to Hrebejk or whoever. Why fork the bug report? Reopen. This is Yarda's comment from issue 42436 which I am going to close <Yarda> During investigation of issue 42244 I concluded that the Actions.SystemNewFile is not implemented optimally. It creates createContextAware action everytime a selection changes. That is wrong as cloning of action is not cheap operation. To improve - extend CallableSystemAction and always delegete to your delegate. In createContextAware method again just delegate. </Yarda>
> To improve - extend CallableSystemAction and I thought we're moving away from SystemActions. Wrong?
During the weekend I have simulated the issue with SimpleProxyLookup rev. 1.4 and ensured that Petr's hotfix (rev. 1.5) is the right one. I am not sure why the fix is called hot, when it is perfectly valid. Maybe due to comment about GC, which as far as I know has not been observed after Petr's fix. This all lead me to observation that the UI responsiveness is ok with SimpleProxyLookup rev. 1.5 and this issue can be closed. I did that but created issue 42436 to fix the obscure use that is the original cause of this bug. I deeply appologize if this is seen wrong - but now the problem from issue 42436 needs to be closed and that is why I am reassigning to Hrebejk. "we're moving away from SystemActions. Wrong?" - we are. There is only one place that requires SystemActions we know about - DataLoader.get/setActions. But Hrebejk needs to use it to replace folder's "New File" action. The dicussed options included - fix DataLoader to work swing actions or write wrapping SystemAction. Hrebejk has choosen the second solution. Given all the problem it caused it might be time for reconsidering it!?
Yardo: I can't agree. My "fix" was really a hotfix, as it has (and still have) following behaviour: If I create a SPL, set a delegate, do one query on SPL and keep the result, then do hundreds queries on SPL without keeping reference to newly created results, and then change the delegate, all these hundreds of garbage results get expansive reregistration to the new instance of the delegate's result. Current implementation of SPL (with my hotfix) is not likely to cause >1s lasting UI stall during TC switch, but regularly slows down TC switching by several tens of ms. My idea was to share the instance of result for given template (as there is probably only one template used)
Ok. So do I understand that the problem is: Lookup.Template t = ...; res1 = lookup.lookup (t); res2 = lookup.lookup (t); and res1 != res2? Ok, that should be prevented. All I am saying is that it should be prevented on the callers side (Hrebejk) and not implemented in lookup as this has some not so desired behaviours attached as well. I can implement it, but rather as part of another issue, as this does not cause UI problems.
Sorry, I'd better not wrote the "My idea" section to not distract you. So again, the problem is: The speed of SPL.lookup(t).allInstances() after the change in the Lookup.Provider does not scale and is highly dependent on the number of previous lookup().allInstances() calls. Please note: "dependent on the number of calls", not "dependent on the number of actively used results".
Created attachment 14556 [details] Measurement demo source
BTW: Why does the SPL explicit refresh (updateLookup) when the underlaying lookup is not replaced? If the underlaying data has changed, it should get resultChanged, right? (Ok, maybe this is about nondetectable asynchronous changes and things like beforeLookup to get them...)
*** Issue 42420 has been marked as a duplicate of this issue. ***
Created attachment 14680 [details] Test rewritten to junit form
Hrebejku, if you have private copy of NewTemplateAction consider update to newer fixed version. "#42244: Sharing results for the same templates. Fixing a GC related bug in NewTemplateAction discovered due to the changes in SimpleProxyLookup" Checking in loaders/src/org/openide/actions/NewTemplateAction.java; /cvs/openide/loaders/src/org/openide/actions/NewTemplateAction.java,v <-- NewTemplateAction.java new revision: 1.17; previous revision: 1.16 done Processing log script arguments... More commits to come... Checking in src/org/openide/util/lookup/SimpleProxyLookup.java; /cvs/openide/src/org/openide/util/lookup/SimpleProxyLookup.java,v <-- SimpleProxyLookup.java new revision: 1.6; previous revision: 1.5 done Processing log script arguments... More commits to come... Checking in test/unit/src/org/openide/actions/NewTemplateActionTest.java; /cvs/openide/test/unit/src/org/openide/actions/NewTemplateActionTest.java,v <-- NewTemplateActionTest.java new revision: 1.3; previous revision: 1.2 done Processing log script arguments... More commits to come... RCS file: /cvs/openide/test/unit/src/org/openide/util/lookup/SimpleProxyLookupSpeedIssue42244Test.java,v done Checking in test/unit/src/org/openide/util/lookup/SimpleProxyLookupSpeedIssue42244Test.java; /cvs/openide/test/unit/src/org/openide/util/lookup/SimpleProxyLookupSpeedIssue42244Test.java,v <-- SimpleProxyLookupSpeedIssue42244Test.java initial revision: 1.1 done
Thanks. It does not cause TODO navigation problems anymore.
Originaly reported problem is no more reproducible - verified in dev-200407131800.