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