Community
Participate
Eclipse IDE
We're using the CodeGeneration class to get stub method body. It appears that every time we make the call (thousands of times), the ListenerList would grow in size. Eventually this list grows so large that it becomes a performance issue for us while memory keeps increasing. In the ScopedPreferenceStore constructor, a new listener is added to the ProjecPreferences but I don't see it ever being removed. While the ListenerList attempts to check for duplicate listeners, the listener doesn't implement equals.
Tod, let's look at this early next week.
Fixed in build >20051116. There is now an explicit call to dispose() required when diposing of a preference store.
Problem area seems to be in the class: org.eclipse.jdt.internal.corext.codemanipulation.StubUtility.getCodeTemplate(...) It calls: new ProjectTemplateStore(...) ProjectTemplateStore constructor calls new ScopedPreferenceStore(...)
I did a quick search in my workspace and JDT is not the only one using the ScopedPreferenceStore in this way. May be there's a flaw in the preferences API. There needs to be a simpler way to get a value of a preference. It appears that ScopedPreferenceStore is designed to be a very heavy weight object and is meant to be a long living object. It reads, modifies, listens/updates/notifies and writes the preferences. You create one and hold on to it when you start up, and destroy it when you exit. In most cases, we only need to look up the value of a preference and that's all.
Duong can you give us the list? We need to determine how much these are used and if the performance gain is good enough to introduce new API into 3.1.2. Adding Dirk as he can let us know how much help this will be to JDT.
CCing Martin since he is our preference store guru. Martin, can you please answer Tod's question.
I think there's a better solution that adding a method 'dispose()' to the ScopedPreferenceStore: Don't install the listeners in the constructor but at the moment when when the first IPropertyChangeListener is added to the ScopedPreferenceStore itself. You can remove the listeners when the last IPropertyChangeListener is deregistered again. Preference stores don't have a dispose so far. Adding one to the ScopedPreferenceStore should be avoided, as clients are likely to forget to call.
I think this is certainly a good optimization but I did some testing and this is not a reliable method as it relies on plug-ins to clean up (org.eclipse.jdt.ui.PreferenceConstants leaks a listener for instance - I'll log a seperate report for that). Having said that I am all for leaving the dispose out for a 3.1.2 implementation and leaving it in for 3.2 with a description of what it is required. Duong would doing this in 3.1.2 be suffecient? I'll attach a potential 3.1.2 patch for you to try out.
This is reliable. Is is a severe bug not to deregister a listener! We have to fix these cases! Don't introduce new lifecyle if not really required. It just makes the system more complicated.
Point taken Martin - the potential to accidentally recreate preference nodes by referencing a removed store is pretty serious and should be treated as such. I am going to write a performance test to check for this and add some leak detection for debug mode. I think you might be right that adding a dispose is an invitation to bad code style.
One cent from my side: if somebody forgets to deregister a listener then he will forget call dispose as well.
Duong can you give a test case to replicate this please - I want to see if the 3.2 fix will handle your case in 3.1.2.
I just checked with Duong and this issue is the ScopedPreferenceStore created in ProjectTemplateStore which does not register any listeners. I'll backport the fix to 3.1.2 as it will solve the issue with no JDT code changes required.
Released in 3.1.2 build >20051118
Verified by code inspection in 20050109 maintenance build
verified on Linux Motif for Version: 3.1.2 Build id: M20060109-1200
oops, ignore previous comment that was meant for another bug
