On Jun 15, 2013, at 2:22 PM, Remi Forax wrote: > On 06/15/2013 08:33 PM, Nick Williams wrote: >> Bug 4851444 [1] was filed just over 10 years ago and has languished without >> so much as a comment ever since. I hate to revive something so old, but I'd >> like to see what everyone thinks. > > Hi Nick, > first 4851444 should be closed in my opinion, given that 1.6 introduces the > ServiceLoader class which is the way to load implementation of a service. > JAXP and JAXB now use the ServiceLoader.
There are really two parts to this bug: what the user needed to accomplish (which ServiceLoader accomplishes), and the suggestion for exposing getCallerClass() (which has broader implications than just his particular use-case). > > 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). > > And about adding a way to store the class in the StackTraceElement, one issue > is that stack trace element are Serializable, so if an error occurs on a > server, the exception can be serialized to the client which is a really nice > feature, but if a stack trace element embed the class object, then it means > that all the class that are available at server side should also be available > at client side. > I have no idea how Microsoft solve this issue. Classes are Serializable in Java, so it shouldn't cause a problem with serialization. The elementClass property will be properly serialized and deserialized. If I understand you correctly, you're expressing concern that this could actually be a problem; that we don't want Classes transferred to the client and deserialized. The Class class has a special serialized form that is unlike other classes and is very efficient. Only a descriptor for the class (including its serialVersionUID value) is transmitted, just enough to give the endpoint enough information to locate the class. There won't be a performance issue here, as far as I can tell. However, I can foresee one potential problem when deserializing the StackTraceElement: if the Class isn't on the class path on the deserializing JVM. This could be handled two possible ways: 1) Mark the elementClass property on StackTraceElement transient so that it isn't serialized. 2) Add a special deserialization rule (not unprecedented by any means) for StackTraceElement that says that StackTraceElement will not fail to deserialize if the elementClass property can't be deserialized because it couldn't be loaded. (This would be my suggestion.) I'm not sure how .NET handles it either, but it clearly does, as its StackFrame class is the equivalent of Serializable. Nick > > cheers, > Rémi > >> >> I'm working on the Apache project Log4j2. Our problem is we need to get the >> Class object that belongs to a particular element on the stack trace. We use >> three different methods, failing over from one to the next, in order to get >> the Class object. >> >> 1. If sun.reflect.Reflection#getCallerClass(int) exists, we repeatedly call >> it--increasing the depth each time--to fill up a java.util.Stack, then >> compare it to the exception stack trace and match as many of the elements as >> we can to Classes returned by getCallerClass. This is surprisingly fast, but >> some elements don't match and we have to fail over to 3. >> >> 2. If sun.reflect.Reflection.getCallerClass(int) does not exist, we create a >> SecurityManager and call getClassContext(), use it to fill up a >> java.util.Stack, and do the same thing we would normally do with 1. This is >> about two times slower than 1--not terrible. >> >> 3. If the element in a stack trace doesn't match a class found in methods 1 >> or 2, we load the class with the ClassLoader using the class name in the >> StackTraceElement. This is more than TEN TIMES SLOWER than 1--terrible. >> >> As you can see, it greatly benefits us to have some kind of efficient >> mechanism to match a StackTraceElement to a Class, so that's why we resort >> to using a non-public API (with the understanding that we must have backup >> options since this class will not always be present). >> >> Where I came in is when the change was made in 1.8.0-ea-b87 to remove the >> parameter from sun.reflect.Reflection#getCallerClass(). Now getCallerClass() >> can ONLY be used to retrieve the immediate caller, not callers at various >> depth, making method 1 unusable in Java 8. This led me to look for an >> alternative. >> >> Looking at Throwable.java and Throwable.c, getStackTrace() ultimately calls >> JVM_GetStackTraceElement. Looking at jvm.c from Java 6 (I can't find >> JVM_GetStackTraceElement in newer versions of Java), >> JVM_GetStackTraceElement calls CVMgetStackTraceElement. >> CVMgetStackTraceElement, in turn, then gets a copy of the native version of >> the Class object and uses it to populate the StackTraceElement's data. This >> leads me to believe that the task of adding a getElementClass() method to >> StackTraceElement would be as simple as: >> >> 1) Adding a new constructor to StackTraceElement to preserve backwards >> compatibility, with the Class being null by default. >> 2) Add the getStackTraceElement() method and backing private field. >> 3) Add a CVMID_fieldWriteRef call to set the class to the backing private >> field. >> >> I'm sure there are some idiosyncrasies I'm missing here, but overall this >> seems like a pretty trivial change. What do people think? Can this be worked >> in? .NET has had this ability since .NET 1.1 with the StackFrame class [2] >> and the StackTrace [3], which contains StackFrames. It provides more than >> just the Class (Type in .NET), it also provides the Method (MethodBase in >> .NET). It would greatly improve the efficiency of certain tasks not only in >> Log4j, but also in many other projects that I have found using >> getCallerClass() for the same purpose. >> >> [1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4851444 >> [2] >> http://msdn.microsoft.com/en-us/library/system.diagnostics.stackframe.aspx >> [3] >> http://msdn.microsoft.com/en-us/library/system.diagnostics.stacktrace.aspx >> >
