Thoughts on adding getElementClass() method to StackTraceElement?
nicholas+openjdk at nicholaswilliams.net
Mon Jun 17 07:20:35 UTC 2013
On Jun 17, 2013, at 1:55 AM, Peter Levart wrote:
> On 06/15/2013 09:59 PM, Nick Williams wrote:
>> On Jun 15, 2013, at 2:22 PM, Remi Forax wrote:
>>> Could you be a little more clear why you need a class for your logging API, exactly, why the class name is not enough for logging purpose ?
>> Certainly. Log4j 2 offers "extended stack trace" patterns. When exceptions or stack traces are logged, or when displaying the location that the logging statement originated from, the stack trace/log also reveals the resource (JAR file, directory, etc.) that the class was loaded from and the implementation version from the package the class is in. This helps reveal possible class loading issues (among other things), which can be an extremely valuable tool when troubleshooting problems with an application.
>> Log4j 2 accomplishes this by getting the class associated with a StackTraceElement (as outlined below), and then 1) getting the Package from the Class and the version from the Package, and 2) getting the ProtectionDomain from the Class, getting the CodeSource from the ProtectionDomain, and getting the JAR/URL from the CodeSource. The last two parts are easy. It's getting the Class from the StackTrace that is extremely inefficient if not done right. Java 8 took away the best way we had found to do that (which, admittedly, was because we were relying on a non-standard class, which isn't recommended).
> On 06/17/2013 08:12 AM, Peter Levart wrote:
>> New API could be entirely unrelated to Throwable, if there was support for it in native code. Since there would have to be changes to the native code anyway to support this, why not create a separate API?
> If I understand this correctly, one part of your code is displaying the "resource location" of each method's declaring class in the stack trace when such "extended stack trace" is configured. Therefore you need this API to capture the class-enriched-stack-trace at the time the Throwable instance is constructed. No special Throwable-unrelated API to return the stack trace would be suitable for this purpose then.
Yes, this is correct. This is the primary use case that needs satisfying.
> But for the purpose of displaying the "resource location" of the class invoking the logging API, you don't need the entire stack trace, just the immediate caller class, right? When @CallerSensitive internal annotation was introduced (and Reflection.getCallerClass(int frames) was dropped), there was some debate about possible introduction of public API that could be used to reveal the caller's class. Perhaps there's still time to create such API for JDK8? At least one part of your logging functionality would be covered with such API.
I would love for this to be part of the public API, but since it's annotation-based we wouldn't be able to use it for probably six or seven years. For the foreseeable future Log4j must compile on Java 6.
> Suppose the API to obtain the immediate caller class (j.l.Class object) was public and fast, so you could do that for each call to the logging API eagerly (after initial check for logging level). Now, could you delay the evaluation of "extended stack trace" pattern to the latest time possible, when you must format the stack-trace? Would calling Class.forName(name, false, classLoader) for each individual StackTraceElement, using the ClassLoader of the caller class, still present a performance problem?
Yes. Our _primary_ concern here is locating the source information for Throwable stack traces. (In fact, if you want, just forget about the source info for the calling class. That's easy. Let's focus JUST on Throwables.) Class.forName() is orders of magnitude slower that SecurityManager#getClassContext() and Reflection#getCallerClass(). And that's not even considering the fact that getClassContext() and getCallerClass() can only fill in _part_ of the stack trace (because they don't line up; one is from Throwable, the other is from current stack). If we could actually obtain the entire stack trace's source info without Class.forName(), it would be even faster. That's why I thought the _simplest_ approach would be to add the getElementClass() method to StackTraceElement ... it works everywhere.
If the proposed solution is a StackFrame class instead, that's fine by me. I just need to be able to get the StackFrame for a Throwable.
More information about the core-libs-dev