On Thu, 4 Jun 2026 20:06:16 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).

Overall, it looks good. It is my long time dream to get rid of this mechanism. 
:)  However, I still have a performance concern which I'd prefere to discuss 
offline. This potential performance issue held me of this simplification before.
My concern is about `PopFrame` related fragment in the function 
`post_method_exit_inner()`. If stack is big and the `interp_only_mode` has been 
already enforced for target thread which has and a `FramePop` request then 
recalculation of current stack depth can give a significant performance 
overhead. At least, we need to somehow mitigate this issue and measure the 
performance impact with a specific benchmark test.

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

PR Comment: https://git.openjdk.org/jdk/pull/31389#issuecomment-4630673319

Reply via email to