> 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 three additional commits since the last revision: - re-fix to exclude only popframe/forceearlyreturn - Revert "8383879" This reverts commit 437bf25c9b53308b1b236ca1863677b478e8fd49. - Revert "removed unused constant" This reverts commit 0ae016428f5f810aaa3c1e061125e9e6d9069fa9. ------------- Changes: - all: https://git.openjdk.org/jdk/pull/31389/files - new: https://git.openjdk.org/jdk/pull/31389/files/0ae01642..58102854 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=31389&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=31389&range=00-01 Stats: 104 lines in 8 files changed: 99 ins; 0 del; 5 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
