Bug 117349 - [performance] Viewer config for JSP needs to dispose JavaTextTools
Summary: [performance] Viewer config for JSP needs to dispose JavaTextTools
Status: CLOSED FIXED
Alias: None
Product: WTP Source Editing
Classification: WebTools
Component: jst.jsp (show other bugs)
Version: 1.0   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 1.5.1 M151   Edit
Assignee: Amy Wu CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
: 151910 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-11-21 14:30 EST by Phillip Avery CLA
Modified: 2007-02-02 17:42 EST (History)
2 users (show)

See Also:

Attachments
org.eclipse.jst.jsp.ui.patch (2.65 KB, patch)
2006-07-31 20:03 EDT, Amy Wu CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Phillip Avery CLA 2005-11-21 14:30:56 EST 
JavaTextTools adds a preference store listener when created (in constructor).
It needs to have dipose() called somewhere to remove the listener.
Comment 1 David Williams CLA 2005-11-21 23:57:46 EST 
Amy ... you're still our "config" expert, right?
Comment 2 Amy Wu CLA 2005-11-28 14:46:59 EST 
StructuredTextViewerConfiguration does not have a dispose method in the API so I'm not sure when to get rid of JavaTextTools.

Option #1: add a dispose method to StructuredTextViewerConfiguration so that StructuredTextViewerConfigurationJSP can override it and call dispose on JavaTextTools

Option B: create 1 instance of JavaTextTools for the entire JSP UI plugin and add something like the following to JSPUIPlugin:
public synchronized JavaTextTools getJavaTextTools() {
	if (fJavaTextTools == null)
		fJavaTextTools= new JavaTextTools(getPreferenceStore(), JavaCore.getPlugin().getPluginPreferences());
	return fJavaTextTools;
}

This is what JDT does.  and then when the JDT UI plugin/bundle stops, the java text tools is disposed.

B seems safer/better because it keeps everything internal and does not create API.  I would only go for #1 if there are other reasons why we want a dispose method on the viewerconfig interface.
Comment 3 Nitin Dahyabhai CLA 2006-07-26 16:05:06 EDT 
*** Bug 151910 has been marked as a duplicate of this bug. ***
Comment 4 Amy Wu CLA 2006-07-26 19:55:40 EDT 
According to bug 151910 this sounds rather bad, and the fix (Option B) sounds like a relatively safe fix, so targetting to 1.5.1.
Comment 5 Amy Wu CLA 2006-07-31 20:02:37 EDT 
Actually, I found a solution that is 50x better.  Since all our viewer configuration really needs is the colormanager from the JavaTextTools, I discovered that there is a public API to get the color manager from the jdt plugin.  JavaUI.getColorManager() We can just call that, and we will no longer need to create an instance of JavaTextTools.
Comment 6 Amy Wu CLA 2006-07-31 20:03:04 EDT 
Created attachment 47121 [details]
org.eclipse.jst.jsp.ui.patch
Comment 7 Amy Wu CLA 2006-08-01 14:22:22 EDT 
Fix released to 1.5.1 M1 & head.

Jeffrey, since you opened a duplicate and Phil is no longer able to verify bugs, do you mind verifying and marking as verified when you have done so?  Thanks.
Comment 8 Jeffrey Liu CLA 2006-08-01 14:25:49 EDT 
Sure, I'll verify it in this week's I-build.
Comment 9 Amy Wu CLA 2007-02-02 17:42:18 EST 
verified WTP1.5.3 SDK 200701311336