Interesting. I can access HTTPS in all of my browsers. OT, to help my figure out what's up with my HTTPS, what browsers and versions are y'all using, Jörn and Alan?
N On Sep 1, 2013, at 10:40 AM, Jörn Huxhorn wrote: > This looks pretty awesome! > > There are just two small things I'd suggest: > > Rename > StackTraceFrame[] getCurrentStackTrace() > to > StackTraceFrame[] getStackFrames() (I'd prefer this one…) > or > StackTraceFrame[] getStackTraceFrames() (…while this one is probably more > accurate.) > or > StackTraceFrame[] getCurrentStackFrames() > or > StackTraceFrame[] getCurrentStackTraceFrames() > > Rename > StackTraceFrame[] getStackTrace(Throwable throwable) > to > StackTraceFrame[] getStackFrames(Throwable throwable) (I'd prefer this one…) > or > StackTraceFrame[] getStackTraceFrames(Throwable throwable) (…while this one > is probably more accurate.) > > > The rationale for this is that previous get[..]StackTrace methods always > returned StackTraceElement[] while get[..]StackFrames methods would then > always return a StackTraceFrame[]. So essentially I'd like to establish the > notion of either StackFrames or StackTraceFrames. > > I did a quick visual review and I didn't find anything horrific or > questionable - which obviously wasn't a security audit, just a sanity check. > > And, yes, https is broken. I got the patches by using http instead. > > Cheer, > Jörn. > > On 1. September 2013 at 10:17:51, Nick Williams > (nicholas+open...@nicholaswilliams.net) wrote: > I have completed and am proposing a patch for replacing > sun.reflect.Reflection#getCallerClass(...) with a public API in Java 8. I saw > no point in duplicating an issue, even though it's over 10 years old, so I'm > indicating that this is a patch for 4851444 > (http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4851444). > > I welcome and solicit support/+1s and a sponsor. Someone about a month ago > had mentioned that they would be willing to be a sponsor if the patch looked > good, but I can't remember who it was and I can't find the email. I want to > say it was someone with RedHat, but my memory could be faulty, so please > don't hold it against me if I'm wrong. > > *Summary of Changes* > Added the new class java.lang.StackTraceFrame. It's a virtual mirror of > StackTraceElement, except that it contains a Class<?> declaringClass property > instead of a String className property. Since the list members expressed > reluctance to add methods to Thread and Throwable, StackTraceFrame also > contains several static methods for retrieving Classes and StackTraceFrames > from the stack. These methods are as follows: > > Class<?> getCallerClass(): Retrieves the class of the caller of the method > calling getCallerClass(). This is identical to the new > Reflection#getCallerClass() added in Java 7u25/8. > > Class<?> getCallerClass(int): Retrieves the class of the caller n frames down > the stack. This is identical to the old Reflection#getCallerClass(int) that > was deprecated in Java 7u25 and removed in Java 8. > > StackTraceFrame getCallerFrame(): Retrieves the StackTraceFrame of the line > of code that called the method calling getCallerClass(). This is similar to > the new Reflection#getCallerClass() added in Java 7u25/8, except that it > returns a StackTraceFrame. > > StackTraceFrame getCallerFrame(int): Retrieves the StackTraceFrame of the > caller n frames down the stack. This is similar to the old > Reflection#getCallerClass(int), except that it returns a StackTraceFrame. > > StackTraceFrame[] getCurrentStackTrace(): Functionally equivalent to > Thread#getStackTrace(), except that it returns an array of StackTraceFrames. > > StackTraceFrame[] getStackTrace(Throwable throwable): Functionally equivalent > to Throwable#getStackTrace(), except that it returns an array of > StackTraceFrames. It uses the same save point (backtrace) created when the > Throwable is created that Throwable#getStackTrace() uses when it's first > called. It caches the array of StackTraceFrames in the Throwable just like > the array of StackTraceElements are cached, so that multiple calls for the > same Throwable are fast. > > As a result of this change, sun.reflect.CallerSensitive has been moved to > java.lang.CallerSensitive. > > I spent considerable time reviewing, revising, considering, and testing these > changes. I included several unit tests that establish the proper behavior. I > also spent considerable time benchmarking the changes. While benchmarking, I > noticed some things. First, getCallerClass() and getCallerClass(int) are as > fast as their counterparts in sun.reflect.Reflection, and they easily inline > when appropriate. Second, getCallerFrame() and getCallerFrame(int) are > /almost/ as fast as the Class versions, but there is some additional overhead > for the construction of the StackTraceFrame. This is minuscule (1,000,000 > invocations consume around 500 ms total on my machine). At this point, all of > the benchmarking was as expected. > > However, I then encountered a surprise. The getCurrentStackTrace() and > getStackTrace(Throwable) methods executed (again, 1,000,000 times) in about > 70% of the time that Thread#getStackTrace() and Throwable#getStackTrace() > did, respectively. Theoretically, they should have executed in the same > amount of time, not faster. After extensive analysis, I discovered (what I > considered to be) a serious flaw in how the stack trace is filled in within > Throwable (which also affects how Thread#getStackTrace() works). > > Instead of simply iterating over the entire save point and filling in the > Throwable stack trace in native code (which is what I did when I implemented > the StackTraceFrame methods), the Java code in Throwable first called a > native method to figure out how deep the stack was, then called another > native method once for every frame in the stack to retrieve each element > individually. This native method that is called repeatedly iterates over the > entire backtrace once for each call, stopping only when it finds the matching > element (so it's O(1) for the first call, O(2) for the second call, O(3) for > the third call, and so on). While my StackTraceFrame methods were iterating > over the backtrace exactly 1 time (O(n)), the Throwable methods were > iterating over the backtrace 1+(n/2) times (worse than O(nlogn) but not as > bad as O(n^2)). This problem would not have been extremely apparent over > small stack traces (the 30% improvement was a stack trace of 6 elements), but > over a large (200+ elements) stack traces the performance difference would be > huge and noticeable. Seeing a room for improvement, I refactored the code > that fills in the stack trace for Throwable, improving its performance > accordingly to match the performance of the StackTraceFrame methods. > > I'm very pleased with how this turned out, and both the unit tests and my > extensive functional testing show that the new class and its methods are > working great. I just need someone willing to review and sponsor my patch! > > *The Code Changes* > I couldn't get WebRev to run without all kinds of errors. So I ran `hg diff > -g` on every repository in the forest with changes. Here are the four patch > files for the four repositories that had changes (no other repositories had > changes): > > https://java.nicholaswilliams.net/Java8GetCallerClass/jdk8.patch > https://java.nicholaswilliams.net/Java8GetCallerClass/jdk8_jdk.patch > https://java.nicholaswilliams.net/Java8GetCallerClass/jdk8_hotspot.patch > https://java.nicholaswilliams.net/Java8GetCallerClass/jdk8_nashorn.patch > > I believe I have followed all of the procedures as closely as possible. I > await feedback and hope for some support on this, so that we can get a public > replacement for this method in Java 8. Let me know if you have any questions. > > > Thanks! > > Nick
