On Fri, 16 Jan 2026 04:02:23 GMT, Chris Plummer <[email protected]> wrote:
>> Yasumasa Suenaga has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Update copyright year in ConstantPool.java
>> - Update ClhsdbDumpheap to verify HPROF_GC_ROOT_JAVA_FRAME
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/InterpretedVFrame.java
> line 86:
>
>> 84: int length = maskLen - maxLocals;
>> 85:
>> 86: StackValueCollection result = new StackValueCollection(length);
>
> Can you explain why the expression stack is not always empty for native
> methods? I would think we would only care about parameters, and they would be
> returned by getLocals(), not getExpressions().
In HotSpot, local stack and expression stack would be handled in same function.
I just followed its manner in this patch. Stack values would be collected at
`interpretedVFrame::stack_data` in both case, and it considers native methods
of course.
heapDumper.cpp
if (vf->is_java_frame()) {
javaVFrame* jvf = javaVFrame::cast(vf);
if (!(jvf->method()->is_native())) {
java_ref_dumper.set_frame_number(depth);
java_ref_dumper.dump_java_stack_refs(jvf->locals());
java_ref_dumper.dump_java_stack_refs(jvf->expressions());
} else {
vframe.cpp
StackValueCollection* interpretedVFrame::locals() const {
return stack_data(false);
}
StackValueCollection* interpretedVFrame::expressions() const {
return stack_data(true);
}
/*
* Worker routine for fetching references and/or values
* for a particular bci in the interpretedVFrame.
*
* Returns data for either "locals" or "expressions",
* using bci relative oop_map (oop_mask) information.
*
* @param expressions bool switch controlling what data to return
(false == locals / true == expression)
*
*/
StackValueCollection* interpretedVFrame::stack_data(bool expressions) const {
InterpreterOopMap oop_mask;
method()->mask_for(bci(), &oop_mask);
const int mask_len = oop_mask.number_of_entries();
// If the method is native, method()->max_locals() is not telling the truth.
// For our purposes, max locals instead equals the size of parameters.
const int max_locals = method()->is_native() ?
method()->size_of_parameters() : method()->max_locals();
But current SA and this PR are not aligned structure of HotSpot - SA does not
have `stack_data` in `InterpretedVFrame`. Point of view of maintainance, it
might be better to follow HotSpot. Should we refactor to follow HotSpot
implementation?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29058#discussion_r2697455521
