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).

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

Commit messages:
 - removed unused constant
 - 8383879

Changes: https://git.openjdk.org/jdk/pull/31389/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=31389&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8383879
  Stats: 103 lines in 8 files changed: 0 ins; 97 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/31389.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/31389/head:pull/31389

PR: https://git.openjdk.org/jdk/pull/31389

Reply via email to