On 3/2/16 1:58 PM, Aleksey Shipilev wrote:
Hi Coleen,

On 03/02/2016 09:44 PM, Coleen Phillimore wrote:
Summary: replace JVM_GetStackTraceDepth and JVM_GetStackTraceElement,
with JVM_GetStackTraceElements that gets all the elements in the
StackTraceElement[]

These improvements were found during the investigation for replacing
Throwable with the StackWalkAPI.   This change also adds iterator for
BacktraceBuilder to make changing format of backtrace easier.

Tested with -testset core, RBT nightly hotspot nightly tests on all
platforms, and jck tests on linux x64.  Compatibility request is approved.

open webrev at http://cr.openjdk.java.net/~coleenp/8150778_jdk/
open webrev at http://cr.openjdk.java.net/~coleenp/8150778_hotspot
bug link https://bugs.openjdk.java.net/browse/JDK-8150778
Looks interesting!

Is there an underlying reason why we can't return the pre-filled
StackTraceElements[] array from the JVM_GetStackTraceElements to begin
with? This will avoid leaking StackTraceElement constructor into
standard library, *and* allows to make StackTraceElement fields final.
Taking stuff back from the standard library is hard, if not impossible,
so we better expose as little as possible.

We measured that it's faster to allocate the StackTraceElement array in Java and it seems cleaner to the Java guys.
It came from similar code we've been prototyping for StackFrameInfo.

Other minor nits:

  * Initializing fields to their default values is a code smell in Java:
      private transient int depth = 0;

ok, not sure what "code smell" means but it doesn't have to be initialized like this. It's set in the native code.

  * Passing a null array to getStackTraceElement probably SEGVs? I don't
see the null checks in native parts.

Yes, it would SEGV. I'll add some checks for null and make sure it's an array of StackTraceElement.

Thanks,
Coleen

Thanks,
-Aleksey


Reply via email to