On Sat, 13 Jun 2026 00:41:13 GMT, Leonid Mesnik <[email protected]> wrote:

>> The cur_stack_depth is the cache that is maintained in interponly mode to 
>> optimize getting number of thread in the debugger operations.
>> 
>> Historically, it cause a lot of issues. Most oftenly, related with wrong 
>> calculation in the case if stack depth changed during yield or stack 
>> debugger operations. Also, there is a gep between decrement of 
>> cur_stack_depth and actual stack change where cur_stack_depth is invalid. 
>> 
>> The current problem happens when operations like PopFrame  or 
>> ForceEarlyReturn happen when MethodExit is completed and cur_stack_depth is 
>> decremented while physical stack is not removed yet.  It is reproduced by 
>> internal stress test.
>> 
>> We discussed privately with @sspitsyn and there is an idea that this cache 
>> is not worth to maintain. It is used in interp-only mode which is slow 
>> already. Really this cur_stack_depth() is not so often used and quite often 
>> invalidated.
>> 
>> Currently, the function is uses in 3 places:
>> 
>> 1. JvmtiExport::continuation_yield_cleanup, where 
>> invalidate_cur_stack_depth() is called right before cur_stack_depth(), 
>> making the caching useless. So no performance degradation here is expected. 
>> 2. In the post_method_exit_inner when 'ets->has_frame_pops()' is true. And 
>> actually it might crash in this place. This happens when NotifyFramePop 
>> event is requested. It doesn't require the interponly mode after fix of 
>> [JDK-6960970](https://bugs.openjdk.org/browse/JDK-6960970) Debugger very 
>> slow during stepping 
>> 3. JvmtiThreadState::process_pending_step_for_popframe which is called in 
>> UpdateForPopTopFrameClosure during PopFrame when performance is not 
>> important. Also, the crash happens when 'process_pending_step_for_popframe' 
>> is executed while cache doesn't correspond the actual state.
>> 
>> 
>> So I wonder if there are any scenario when the impact of the removal of 
>> cur_stack_depth is so significant that it makes sense to still maintain it?
>> 
>> i am running tier1-8 and some internal stress testing to verify this fix.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   typo fixed

This looks good. Thank you for fixing it!
We still need to remove this mechanism in the future. But need to find out how 
to avoid the performance impact. I'm thinking if it is possible to allocate one 
bit in the frame mark it for a `FramePop` event. This will remove any 
performance impact. But it needs to be discussed with the Runtime team first. 
I'll follow up on this soon.

-------------

Marked as reviewed by sspitsyn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/31389#pullrequestreview-4511061975

Reply via email to