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.phillim...@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
