Bugzilla – Full Text Bug Listing
| Summary: | Reduce the amount of Strings created when tracing is disabled | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [WebTools] WTP ServerTools | Reporter: | Troy Bishop <tjbishop> | ||||||||||
| Component: | wst.server | Assignee: | Angel Vera <arvera> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | Angel Vera <arvera> | ||||||||||
| Severity: | enhancement | ||||||||||||
| Priority: | P3 | ||||||||||||
| Version: | 3.2 | ||||||||||||
| Target Milestone: | 3.2.4 | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Windows XP | ||||||||||||
| See Also: | https://git.eclipse.org/r/109041 | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
This patch also includes the update from bug 337592. Created attachment 189547 [details]
initial patch
Created attachment 189561 [details]
second patch - removes a lot of duplicate (historical) strings in favor of using the option value in the trace output
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? 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
changes committed to 32M Changes released to 32M. (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. (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. Created attachment 190905 [details]
equilvalent patch for HEAD
changes committed to HEAD(3.3) Changes released to HEAD(3.3) New Gerrit change created: https://git.eclipse.org/r/109041 |
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