Code Review for JEP 259: Stack-Walking API

Brent Christian brent.christian at
Wed Nov 18 19:00:56 UTC 2015

Hi, Mandy

I looked through the jdk code.  I think it's looking quite good.  I just 
noticed some small details to consider fixing up before pushing.


the "binary name" link seems to be broken
( line 100)

StackWalker.getInstance(Set<StackWalker.Option> options):
StackWalker.getInstance(Set<StackWalker.Option> options, int estimateDepth):

A couple missing words:

"Returns a StackWalker instance with the given *options* specifying..."

"If the given *["options"|"set"]* is empty, ..."

(Looks like a typo ("@ocde") on lines 278 & 307)



The comment for get() states that it returns a StackTraceElement

hotspot/src/share/vm/prims/jvm.h (219)
jdk/src/java.base/share/native/include/jvm.h (196)

FWIW, the start_index argument of
JVM_FillStackFrames is an int in the hotspot file, and a jint in the jdk 


The comment for get() states that it returns a StackTraceElement


  137         // VM writes to these arrays to fill the stack frame 
  138         // for each batch

I think this comment applies to a previous version of the code.

  195          * Subclass should override this method to change the 
batch size
  196          * or invoke the function passed to the StackWalker::walk 

I believe the function in question is the batchSizeMapper function, no 
longer being passed to walk(); line 196 can be removed.


Perhaps this.memberName does not need to be allocated in the case of 
-XX:-MemberNameInStackFrame ?


On 11/16/15 4:13 PM, Mandy Chung wrote:
> I’d like to get the code review done by this week.
> I renamed the static factory method from create to getInstance since “create” implies to create a new instance but the method returns a cached instance instead.  I also changed the spec of getCallerClass per [1].  There is not much change since webrev.01.
> Webrev:
> javadoc:
> Mandy
> [1]

More information about the core-libs-dev mailing list