Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 337907 - Reduce the amount of Strings created when tracing is disabled
Summary: Reduce the amount of Strings created when tracing is disabled
Status: RESOLVED FIXED
Alias: None
Product: WTP ServerTools
Classification: WebTools
Component: wst.server (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 3.2.4   Edit
Assignee: Angel Vera CLA
QA Contact: Angel Vera CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-22 16:39 EST by Troy Bishop CLA
Modified: 2017-10-11 16:36 EDT (History)
0 users

See Also:


Attachments
initial patch (441.15 KB, patch)
2011-02-22 16:41 EST, Troy Bishop CLA
no flags Details | Diff
second patch - removes a lot of duplicate (historical) strings in favor of using the option value in the trace output (439.55 KB, patch)
2011-02-22 18:12 EST, Troy Bishop CLA
no flags Details | Diff
v3.0 (394.21 KB, patch)
2011-03-09 21:24 EST, Angel Vera CLA
arvera: iplog+
Details | Diff
equilvalent patch for HEAD (437.09 KB, patch)
2011-03-10 14:29 EST, Troy Bishop CLA
arvera: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Troy Bishop CLA 2011-02-22 16:39:50 EST
Build Identifier: 

Currently the Trace class for the various server tools bundles has the following (internal) API:

Trace.trace(level, message)
Trace.trace(level, message, Throwable)

In these methods a check is done to see if tracing is enabled and, if so, the corresponding message and Throwable are printed the the System.out stream.

The problem with this approach is that the various String and (sometimes, but fairly rare) the Throwable are constructed and end up doing nothing if tracing is disabled.

The attached patch updates all of the calls to this internal API so that they are wrapped in a boolean check, i.e.

if(flag) {
  Trace.trace(...);
}

Reproducible: Always
Comment 1 Troy Bishop CLA 2011-02-22 16:40:39 EST
This patch also includes the update from bug 337592.
Comment 2 Troy Bishop CLA 2011-02-22 16:41:27 EST
Created attachment 189547 [details]
initial patch
Comment 3 Troy Bishop CLA 2011-02-22 18:12:27 EST
Created attachment 189561 [details]
second patch - removes a lot of duplicate (historical) strings in favor of using the option value in the trace output
Comment 4 Angel Vera CLA 2011-03-09 21:06:41 EST
Troy, Thank you for contributing this enhancement. This will help reduce the number of strings in memory.

I do have a question about the following, : 
   	// register the debug options listener
		final Hashtable<String, String> props = new Hashtable<String, String>(4);
		props.put(DebugOptions.LISTENER_SYMBOLICNAME, JavaServerPlugin.PLUGIN_ID);
		context.registerService(DebugOptionsListener.class.getName(), new Trace(), props);

Would you able to explain its purpose?
Comment 5 Angel Vera CLA 2011-03-09 21:24:20 EST
Created attachment 190811 [details]
v3.0

To continue with our convention I have made some slight modifications to your patch.

Things like:
- Static variables declared at the top of the class instead of the bottom
- .options files having org.eclipse.jst.server.ui/debug=true instead of false
Comment 6 Angel Vera CLA 2011-03-09 21:24:51 EST
changes committed to 32M
Comment 7 Angel Vera CLA 2011-03-09 21:52:17 EST
Changes released to 32M.
Comment 8 Troy Bishop CLA 2011-03-10 10:02:32 EST
(In reply to comment #4)
> Troy, Thank you for contributing this enhancement. This will help reduce the
> number of strings in memory.
> 
> I do have a question about the following, : 
>        // register the debug options listener
>         final Hashtable<String, String> props = new Hashtable<String,
> String>(4);
>         props.put(DebugOptions.LISTENER_SYMBOLICNAME,
> JavaServerPlugin.PLUGIN_ID);
>         context.registerService(DebugOptionsListener.class.getName(), new
> Trace(), props);
> 
> Would you able to explain its purpose?

Angel,

This is the way to register the bundle-specific Trace class to the OSGi DebugOptions service so that the various boolean flags in that Trace class can be dynamically changed when their corresponding debug-option value changes. See bug 296631 for the possibility of a UI to exist which will allow for dynamically changing these values via a UI in the future without restarting the product.
Comment 9 Troy Bishop CLA 2011-03-10 10:06:59 EST
(In reply to comment #5)
> Created attachment 190811 [details]
> v3.0
> 
> To continue with our convention I have made some slight modifications to your
> patch.
> 
> Things like:
> - Static variables declared at the top of the class instead of the bottom
> - .options files having org.eclipse.jst.server.ui/debug=true instead of false

FYI, my changing of the .options entries to 'false' was just in preparation for the UI (bug 296631) so that the 'default' state of the tracing is disabled.  This UI will read the .options files and provide a check-box (for boolean option-path entries) and check them automatically if their value is true.  Having your option-path entry default to 'true' would mean that it would automatically enabling this tracing even if the end-user did not want this tracing enabled.
Comment 10 Troy Bishop CLA 2011-03-10 14:29:42 EST
Created attachment 190905 [details]
equilvalent patch for HEAD
Comment 11 Angel Vera CLA 2011-04-19 13:30:34 EDT
changes committed to HEAD(3.3)
Comment 12 Angel Vera CLA 2011-04-19 15:35:42 EDT
Changes released to HEAD(3.3)
Comment 13 Eclipse Genie CLA 2017-10-11 16:36:36 EDT
New Gerrit change created: https://git.eclipse.org/r/109041