Community
Participate
Working Groups
I'm all for instrumenting plugin code with trace statements using Eclipse's DebugTrace facility. Every line identifies the time, thread, and what line of code is generating the trace. This is very useful for troubleshooting timing and performance issues. I'm glad to see EDC plugins using it. However, I think the way the trace statements are coded need improvement. First, trace statements should have as little overhead as possible. Ideally, they'd have zero overhead when tracing is turned off. That's not possible without jvm runtime support, but we should strive for as close to zero overhead as possible. Checking a static boolean is extremely fast, and that is what you see a lot of plugins do. E.g., see: org.eclipse.cdt.dsf.internal.DsfPlugin.start(BundleContext) then, trace statements are coded as follows: if (TRACE_RUNCONTROL) { <trace statement> } EDC, however, is relying on DebugTrace to determine whether or not to trace. While this is convenient, look at what it involves: EDCDebugger.getDefault().getTrace().traceEntry(option, ...); ...three method calls and a possible string creation for every trace statement *even when tracing is turned off*. I've taken some measurements and that's 500 X more expensive than checking a static field. There are quite a few CDT plugins tracing, and none but EDC rely on DebugTrace class for checking the option. I recommend we use static field checks to reduce overhead when tracing is off. We lose the ability to dynamically toggle tracing, but is that being used? Also, the traceExit() call in EDC are unreliable since an exception throw will bypass them. Also, any methods with multiple return statements would have to be equipped with multiple exit traces statements. This is not only onerous and code-cluttering, but error prone. traceExit's should be done in a finally block. I've done some measurements here, too, and adding a finally block doesn't add any runtime overhead. To summarize, my recommendations are: 1. Use static field checks before calling into DebugTrace methods (unless someone is relying on dynamically toggling on/off trace). 2. Use finally sections for exit trace statements Objections?
No objections, tracing in EDC was created with good intentions but neglected. The last time I attempted to turn on tracing I got NPEs from some of the trace statements... The use case I wanted to support was this: user reports bug, we tell them to turn on tracing, reproduce the bug, then send us the log. But currently it's difficult to turn on tracing and I can never remember where the log files get created. We may need to create some UI for this and make the trace files more accessible.
(In reply to comment #1) > The use case I wanted to support was this: user reports bug, we tell them to > turn on tracing, reproduce the bug, then send us the log. Yep. That's exactly the sort of thing the Eclipse tracing mechanism is supposed to allow. > But currently it's difficult to turn on tracing and I can never remember where > the log files get created. We may need to create some UI for this and make the > trace files more accessible. {workspace}/.metadata/trace.log Yeah. It requires specifying switches when launching eclipse. The GUI developers have to turn these things on are in the PDE launch configuration. Equivalent GUI is not available for our end users. We could add it to the EDC launch configuration. I guess the question there is whether the tracing is used frequently enough to add the GUI. And if that is even the right approach--a more general GUI (as in in PDE launch configuration) may be a better solution, but then that becomes a platform thing. For now, my concern is only the performance and the missing of function-exit statements. I'll address those.
(In reply to comment #0) > Also, the traceExit() call in EDC are unreliable since an exception throw will > bypass them. Also, any methods with multiple return statements would have to be > equipped with multiple exit traces statements. This is not only onerous and > code-cluttering, but error prone. traceExit's should be done in a finally > block. I've done some measurements here, too, and adding a finally block > doesn't add any runtime overhead. Once I started trying to adjust these things, it occurred to me that perhaps the missed trace statements in the case of an exception is a good thing. It at least provides a hint that an exception was thrown whereas an exit trace put in a finally clause can make a successful and unsuccessful execution of the method indistinguishable. Also, consolidating multiple exit statements into one a finally clause can be messy since the result logged in the trace sometimes varies based on the flow. I'm going to address only the performance aspect of this bug.
Created attachment 180202 [details] Solution
Committed to HEAD.
*** cdt cvs genie on behalf of jcortell *** Bug 326706: EDC code tracing has (relatively) high overhead [+] EDCTrace.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/edc/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/EDCTrace.java?root=Tools_Project&revision=1.1&view=markup [*] EDCDebugger.java 1.5 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/edc/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/EDCDebugger.java?root=Tools_Project&r1=1.4&r2=1.5 [-] IEDCTraceOptions.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/edc/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/IEDCTraceOptions.java?root=Tools_Project&view=markup [*] MemoryCache.java 1.8 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/edc/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/services/dsf/MemoryCache.java?root=Tools_Project&r1=1.7&r2=1.8 [*] Breakpoints.java 1.22 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/edc/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/services/dsf/Breakpoints.java?root=Tools_Project&r1=1.21&r2=1.22 [*] Expressions.java 1.19 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/edc/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/services/dsf/Expressions.java?root=Tools_Project&r1=1.18&r2=1.19 [*] Memory.java 1.9 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/edc/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/services/dsf/Memory.java?root=Tools_Project&r1=1.8&r2=1.9 [*] RunControl.java 1.39 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/edc/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/services/dsf/RunControl.java?root=Tools_Project&r1=1.38&r2=1.39 [*] OperandValue.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/edc/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/eval/ast/engine/instructions/OperandValue.java?root=Tools_Project&r1=1.3&r2=1.4 [*] DwarfInfoReader.java 1.18 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/edc/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/symbols/dwarf/DwarfInfoReader.java?root=Tools_Project&r1=1.17&r2=1.18 [*] ASTInstructionCompiler.java 1.9 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/edc/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/eval/ast/engine/ASTInstructionCompiler.java?root=Tools_Project&r1=1.8&r2=1.9 [*] Stack.java 1.38 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/edc/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/services/Stack.java?root=Tools_Project&r1=1.37&r2=1.38
Created attachment 180209 [details] patch to make link to "how to" page clickable in javadoc pop-up hey, john … if you format the javadoc in EDCTrace.java as shown in the attached patch, it will cause the link to be clickable from the javadoc window, and will take the user straight an eclipse editor window with the page at the URL you cite.
(In reply to comment #7) > Created an attachment (id=180209) [details] > patch to make link to "how to" page clickable in javadoc pop-up > > hey, john … if you format the javadoc in EDCTrace.java as shown in the attached > patch, it will cause the link to be clickable from the javadoc window, and will > take the user straight an eclipse editor window with the page at the URL you > cite. Thanks. Applied.
