RFR 8150778: Reduce Throwable.getStackTrace() calls to the JVM

Coleen Phillimore coleen.phillimore at oracle.com
Thu Mar 3 02:55:09 UTC 2016


Mandy, thank you for reviewing this.

On 3/2/16 9:18 PM, Mandy Chung wrote:
>> On Mar 2, 2016, at 4:03 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
>>
>> Freshly tested changes with jck tests, with missing checks and other changes to use the depth field, as pointed out by Aleksey.  I've kept the StackTraceElement[] allocation in Java to match the new API that was approved.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8150778.02_hotspot/
>> open webrev at http://cr.openjdk.java.net/~coleenp/8150778.02_jdk/
> typo in your link:
>    http://cr.openjdk.java.net/~coleenp/8150778.02_jck/
>
> StackTraceElement.java
>   80      * @since 1.9

Okay, good because it's probably 9.0 anyway.
>
> This is not needed. Simply take this out.
>
> Throwable.java
>
> 215      * Native code sets the depth of the backtrace for later retrieval
>
> s/Native code/VM/
I changed it to "The JVM sets the depth..."  There was another sentence 
just like this for the backtrace field, which I also changed.
> since VM is setting the depth field.
>
>
> 896     private native void getStackTraceElements(StackTraceElement[] elements);
>
> Can you add the method description
>     “Gets the stack trace elements."

Fixed.
> I only skimmed on the hotspot change.  Looks okay to me.
>
> TestThrowable.java
>
>    43     int getDepth(Throwable t) throws Exception {
>    44       for (Field f : Throwable.class.getDeclaredFields()) {
>    45         if (f.getName().equals("depth")) {
>   
>
> You can replace the above with Throwable.class.getDeclaredField(“depth”)

Yes, that's better.
> Otherwise, looks okay.

Thanks!
Coleen
> Mandy




More information about the core-libs-dev mailing list