From 4a039c6e6bfcee7e935e41fc4f9c09d8d718bbff Mon Sep 17 00:00:00 2001 From: Tom Rodriguez Date: Tue, 12 Nov 2024 19:15:24 -0800 Subject: [PATCH] Store deferred locals information in frame --- src/hotspot/cpu/aarch64/frame_aarch64.cpp | 9 +-- src/hotspot/cpu/aarch64/frame_aarch64.hpp | 2 +- src/hotspot/cpu/arm/frame_arm.cpp | 9 +-- src/hotspot/cpu/arm/frame_arm.hpp | 4 +- src/hotspot/cpu/riscv/frame_riscv.cpp | 9 +-- src/hotspot/cpu/riscv/frame_riscv.hpp | 2 +- src/hotspot/cpu/x86/frame_x86.cpp | 9 +-- src/hotspot/cpu/x86/frame_x86.hpp | 2 +- src/hotspot/os/posix/signals_posix.cpp | 2 +- src/hotspot/os/windows/os_windows.cpp | 2 +- src/hotspot/share/code/nmethod.cpp | 4 - src/hotspot/share/code/nmethod.hpp | 8 +- .../leakprofiler/checkpoint/rootResolver.cpp | 7 -- .../share/prims/jvmtiDeferredUpdates.cpp | 52 ------------- .../share/prims/jvmtiDeferredUpdates.hpp | 44 ++--------- src/hotspot/share/runtime/deoptimization.cpp | 9 +-- src/hotspot/share/runtime/escapeBarrier.cpp | 39 +++++----- src/hotspot/share/runtime/escapeBarrier.hpp | 8 +- src/hotspot/share/runtime/frame.cpp | 74 ++++++++++++++++++- src/hotspot/share/runtime/frame.hpp | 10 +++ src/hotspot/share/runtime/frame.inline.hpp | 2 +- src/hotspot/share/runtime/javaThread.cpp | 20 +---- src/hotspot/share/runtime/javaThread.hpp | 20 +++-- src/hotspot/share/runtime/objectMonitor.cpp | 2 +- src/hotspot/share/runtime/vframe_hp.cpp | 13 ++-- .../hotspot/HotSpotStackFrameReference.java | 3 - .../ci/hotspot/HotSpotStackIntrospection.java | 3 +- .../MaterializeVirtualObjectTest.java | 24 +----- 28 files changed, 163 insertions(+), 229 deletions(-) diff --git a/src/hotspot/cpu/aarch64/frame_aarch64.cpp b/src/hotspot/cpu/aarch64/frame_aarch64.cpp index 361b913fd2ea2..92fbda23a35aa 100644 --- a/src/hotspot/cpu/aarch64/frame_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/frame_aarch64.cpp @@ -457,14 +457,9 @@ JavaThread** frame::saved_thread_address(const frame& f) { // given unextended SP. #ifdef ASSERT void frame::verify_deopt_original_pc(nmethod* nm, intptr_t* unextended_sp) { - frame fr; + assert(unextended_sp == _unextended_sp, "expected to be the same frame"); - // This is ugly but it's better than to change {get,set}_original_pc - // to take an SP value as argument. And it's only a debugging - // method anyway. - fr._unextended_sp = unextended_sp; - - address original_pc = nm->get_original_pc(&fr); + address original_pc = get_original_pc(nm); assert(nm->insts_contains_inclusive(original_pc), "original PC must be in the main code section of the compiled method (or must be immediately following it)"); } diff --git a/src/hotspot/cpu/aarch64/frame_aarch64.hpp b/src/hotspot/cpu/aarch64/frame_aarch64.hpp index da020b4234d10..f7eaec9ddecee 100644 --- a/src/hotspot/cpu/aarch64/frame_aarch64.hpp +++ b/src/hotspot/cpu/aarch64/frame_aarch64.hpp @@ -154,7 +154,7 @@ #ifdef ASSERT // Used in frame::sender_for_{interpreter,compiled}_frame - static void verify_deopt_original_pc(nmethod* nm, intptr_t* unextended_sp); + void verify_deopt_original_pc(nmethod* nm, intptr_t* unextended_sp); #endif public: diff --git a/src/hotspot/cpu/arm/frame_arm.cpp b/src/hotspot/cpu/arm/frame_arm.cpp index 13a5c471c6fea..221b0bea3ed96 100644 --- a/src/hotspot/cpu/arm/frame_arm.cpp +++ b/src/hotspot/cpu/arm/frame_arm.cpp @@ -338,14 +338,9 @@ JavaThread** frame::saved_thread_address(const frame& f) { // for MethodHandle call sites. #ifdef ASSERT void frame::verify_deopt_original_pc(nmethod* nm, intptr_t* unextended_sp, bool is_method_handle_return) { - frame fr; + assert(unextended_sp == _unextended_sp, "expected to be the same frame"); - // This is ugly but it's better than to change {get,set}_original_pc - // to take an SP value as argument. And it's only a debugging - // method anyway. - fr._unextended_sp = unextended_sp; - - address original_pc = nm->get_original_pc(&fr); + address original_pc = get_original_pc(nm); assert(nm->insts_contains_inclusive(original_pc), "original PC must be in the main code section of the compiled method (or must be immediately following it)"); assert(nm->is_method_handle_return(original_pc) == is_method_handle_return, "must be"); diff --git a/src/hotspot/cpu/arm/frame_arm.hpp b/src/hotspot/cpu/arm/frame_arm.hpp index dee005b8d75de..68a1ca5cb2b1b 100644 --- a/src/hotspot/cpu/arm/frame_arm.hpp +++ b/src/hotspot/cpu/arm/frame_arm.hpp @@ -93,8 +93,8 @@ #ifdef ASSERT // Used in frame::sender_for_{interpreter,compiled}_frame - static void verify_deopt_original_pc(nmethod* nm, intptr_t* unextended_sp, bool is_method_handle_return = false); - static void verify_deopt_mh_original_pc(nmethod* nm, intptr_t* unextended_sp) { + void verify_deopt_original_pc(nmethod* nm, intptr_t* unextended_sp, bool is_method_handle_return = false); + void verify_deopt_mh_original_pc(nmethod* nm, intptr_t* unextended_sp) { verify_deopt_original_pc(nm, unextended_sp, true); } #endif diff --git a/src/hotspot/cpu/riscv/frame_riscv.cpp b/src/hotspot/cpu/riscv/frame_riscv.cpp index ecc450bd6b254..73cbefc84af82 100644 --- a/src/hotspot/cpu/riscv/frame_riscv.cpp +++ b/src/hotspot/cpu/riscv/frame_riscv.cpp @@ -430,15 +430,10 @@ JavaThread** frame::saved_thread_address(const frame& f) { // given unextended SP. #ifdef ASSERT void frame::verify_deopt_original_pc(nmethod* nm, intptr_t* unextended_sp) { - frame fr; - - // This is ugly but it's better than to change {get,set}_original_pc - // to take an SP value as argument. And it's only a debugging - // method anyway. - fr._unextended_sp = unextended_sp; + assert(unextended_sp == _unextended_sp, "expected to be the same frame"); assert_cond(nm != nullptr); - address original_pc = nm->get_original_pc(&fr); + address original_pc = get_original_pc(nm); assert(nm->insts_contains_inclusive(original_pc), "original PC must be in the main code section of the compiled method (or must be immediately following it)"); } diff --git a/src/hotspot/cpu/riscv/frame_riscv.hpp b/src/hotspot/cpu/riscv/frame_riscv.hpp index b4540c45ab8f5..6b8051bfcd772 100644 --- a/src/hotspot/cpu/riscv/frame_riscv.hpp +++ b/src/hotspot/cpu/riscv/frame_riscv.hpp @@ -187,7 +187,7 @@ #ifdef ASSERT // Used in frame::sender_for_{interpreter,compiled}_frame - static void verify_deopt_original_pc(nmethod* nm, intptr_t* unextended_sp); + void verify_deopt_original_pc(nmethod* nm, intptr_t* unextended_sp); #endif public: diff --git a/src/hotspot/cpu/x86/frame_x86.cpp b/src/hotspot/cpu/x86/frame_x86.cpp index 4e28dc125341a..ebc9f49b6d95e 100644 --- a/src/hotspot/cpu/x86/frame_x86.cpp +++ b/src/hotspot/cpu/x86/frame_x86.cpp @@ -446,14 +446,9 @@ JavaThread** frame::saved_thread_address(const frame& f) { // given unextended SP. #ifdef ASSERT void frame::verify_deopt_original_pc(nmethod* nm, intptr_t* unextended_sp) { - frame fr; + assert(unextended_sp == _unextended_sp, "expected to be the same frame"); - // This is ugly but it's better than to change {get,set}_original_pc - // to take an SP value as argument. And it's only a debugging - // method anyway. - fr._unextended_sp = unextended_sp; - - address original_pc = nm->get_original_pc(&fr); + address original_pc = get_original_pc(nm); assert(nm->insts_contains_inclusive(original_pc), "original PC must be in the main code section of the compiled method (or must be immediately following it) original_pc: " INTPTR_FORMAT " unextended_sp: " INTPTR_FORMAT " name: %s", p2i(original_pc), p2i(unextended_sp), nm->name()); } diff --git a/src/hotspot/cpu/x86/frame_x86.hpp b/src/hotspot/cpu/x86/frame_x86.hpp index f3034ee9263a5..a046639c571e5 100644 --- a/src/hotspot/cpu/x86/frame_x86.hpp +++ b/src/hotspot/cpu/x86/frame_x86.hpp @@ -146,7 +146,7 @@ #ifdef ASSERT // Used in frame::sender_for_{interpreter,compiled}_frame - static void verify_deopt_original_pc(nmethod* nm, intptr_t* unextended_sp); + void verify_deopt_original_pc(nmethod* nm, intptr_t* unextended_sp); #endif public: diff --git a/src/hotspot/os/posix/signals_posix.cpp b/src/hotspot/os/posix/signals_posix.cpp index 6a14d0a485643..59ebb9b9a8232 100644 --- a/src/hotspot/os/posix/signals_posix.cpp +++ b/src/hotspot/os/posix/signals_posix.cpp @@ -622,7 +622,7 @@ int JVM_HANDLE_XXX_SIGNAL(int sig, siginfo_t* info, assert(deopt != nullptr, ""); frame fr = os::fetch_frame_from_context(uc); - nm->set_original_pc(&fr, pc); + fr.set_original_pc(nm, pc); os::Posix::ucontext_set_pc(uc, deopt); signal_was_handled = true; diff --git a/src/hotspot/os/windows/os_windows.cpp b/src/hotspot/os/windows/os_windows.cpp index fd857c2cd95eb..c251d1c4072ea 100644 --- a/src/hotspot/os/windows/os_windows.cpp +++ b/src/hotspot/os/windows/os_windows.cpp @@ -2778,7 +2778,7 @@ LONG WINAPI topLevelExceptionFilter(struct _EXCEPTION_POINTERS* exceptionInfo) { nm->deopt_mh_handler_begin() : nm->deopt_handler_begin(); assert(nm->insts_contains_inclusive(pc), ""); - nm->set_original_pc(&fr, pc); + fr.set_original_pc(nm, pc); // Set pc to handler exceptionInfo->ContextRecord->PC_NAME = (DWORD64)deopt; return EXCEPTION_CONTINUE_EXECUTION; diff --git a/src/hotspot/share/code/nmethod.cpp b/src/hotspot/share/code/nmethod.cpp index 754a7786605ff..7b658f20cbbf4 100644 --- a/src/hotspot/share/code/nmethod.cpp +++ b/src/hotspot/share/code/nmethod.cpp @@ -857,10 +857,6 @@ void nmethod::cleanup_inline_caches_whitebox() { cleanup_inline_caches_impl(false /* unloading_occurred */, true /* clean_all */); } -address* nmethod::orig_pc_addr(const frame* fr) { - return (address*) ((address)fr->unextended_sp() + orig_pc_offset()); -} - // Called to clean up after class unloading for live nmethods void nmethod::cleanup_inline_caches_impl(bool unloading_occurred, bool clean_all) { assert(CompiledICLocker::is_safe(this), "mt unsafe call"); diff --git a/src/hotspot/share/code/nmethod.hpp b/src/hotspot/share/code/nmethod.hpp index 095c0ba4a26a4..183d3694cf9dc 100644 --- a/src/hotspot/share/code/nmethod.hpp +++ b/src/hotspot/share/code/nmethod.hpp @@ -749,9 +749,9 @@ class nmethod : public CodeBlob { inline bool is_deopt_mh_entry(address pc); inline bool is_deopt_entry(address pc); - // Accessor/mutator for the original pc of a frame before a frame was deopted. - address get_original_pc(const frame* fr) { return *orig_pc_addr(fr); } - void set_original_pc(const frame* fr, address pc) { *orig_pc_addr(fr) = pc; } + address* orig_pc_addr(const frame* fr) const { + return (address*) ((address)fr->unextended_sp() + _orig_pc_offset); + } const char* state() const; @@ -871,8 +871,6 @@ class nmethod : public CodeBlob { private: ScopeDesc* scope_desc_in(address begin, address end); - address* orig_pc_addr(const frame* fr); - // used by jvmti to track if the load events has been reported bool load_reported() const { return _load_reported; } void set_load_reported() { _load_reported = true; } diff --git a/src/hotspot/share/jfr/leakprofiler/checkpoint/rootResolver.cpp b/src/hotspot/share/jfr/leakprofiler/checkpoint/rootResolver.cpp index 57b29a09d016f..685b0f3b98670 100644 --- a/src/hotspot/share/jfr/leakprofiler/checkpoint/rootResolver.cpp +++ b/src/hotspot/share/jfr/leakprofiler/checkpoint/rootResolver.cpp @@ -263,13 +263,6 @@ bool ReferenceToThreadRootClosure::do_thread_stack_detailed(JavaThread* jt) { return true; } - GrowableArrayView* const list = JvmtiDeferredUpdates::deferred_locals(jt); - if (list != nullptr) { - for (int i = 0; i < list->length(); i++) { - list->at(i)->oops_do(&rcl); - } - } - if (rcl.complete()) { return true; } diff --git a/src/hotspot/share/prims/jvmtiDeferredUpdates.cpp b/src/hotspot/share/prims/jvmtiDeferredUpdates.cpp index bddbdd4f9ffef..8502b102bd3b5 100644 --- a/src/hotspot/share/prims/jvmtiDeferredUpdates.cpp +++ b/src/hotspot/share/prims/jvmtiDeferredUpdates.cpp @@ -26,11 +26,6 @@ #include "precompiled.hpp" #include "prims/jvmtiDeferredUpdates.hpp" -void JvmtiDeferredUpdates::create_for(JavaThread* thread) { - assert(thread->deferred_updates() == nullptr, "already allocated"); - thread->set_deferred_updates(new JvmtiDeferredUpdates()); -} - JvmtiDeferredUpdates::~JvmtiDeferredUpdates() { while (_deferred_locals_updates.length() != 0) { jvmtiDeferredLocalVariableSet* dlv = _deferred_locals_updates.pop(); @@ -38,50 +33,3 @@ JvmtiDeferredUpdates::~JvmtiDeferredUpdates() { delete dlv; } } - -void JvmtiDeferredUpdates::inc_relock_count_after_wait(JavaThread* thread) { - if (thread->deferred_updates() == nullptr) { - create_for(thread); - } - thread->deferred_updates()->inc_relock_count_after_wait(); -} - -int JvmtiDeferredUpdates::get_and_reset_relock_count_after_wait(JavaThread* jt) { - JvmtiDeferredUpdates* updates = jt->deferred_updates(); - int result = 0; - if (updates != nullptr) { - result = updates->get_and_reset_relock_count_after_wait(); - if (updates->count() == 0) { - delete updates; - jt->set_deferred_updates(nullptr); - } - } - return result; -} - -void JvmtiDeferredUpdates::delete_updates_for_frame(JavaThread* jt, intptr_t* frame_id) { - JvmtiDeferredUpdates* updates = jt->deferred_updates(); - if (updates != nullptr) { - GrowableArray* list = updates->deferred_locals(); - assert(list->length() > 0, "Updates holder not deleted"); - int i = 0; - do { - // Because of inlining we could have multiple vframes for a single frame - // and several of the vframes could have deferred writes. Find them all. - jvmtiDeferredLocalVariableSet* dlv = list->at(i); - if (dlv->id() == frame_id) { - list->remove_at(i); - // individual jvmtiDeferredLocalVariableSet are CHeapObj's - delete dlv; - } else { - i++; - } - } while ( i < list->length() ); - if (updates->count() == 0) { - jt->set_deferred_updates(nullptr); - // Free deferred updates. - // Note, the 'list' of local variable updates is embedded in 'updates'. - delete updates; - } - } -} diff --git a/src/hotspot/share/prims/jvmtiDeferredUpdates.hpp b/src/hotspot/share/prims/jvmtiDeferredUpdates.hpp index f23f942780aaf..391393bd17156 100644 --- a/src/hotspot/share/prims/jvmtiDeferredUpdates.hpp +++ b/src/hotspot/share/prims/jvmtiDeferredUpdates.hpp @@ -112,51 +112,21 @@ class jvmtiDeferredLocalVariableSet : public CHeapObj { class JvmtiDeferredUpdates : public CHeapObj { - // Relocking has to be deferred if the lock owning thread is currently waiting on the monitor. - int _relock_count_after_wait; + address _original_pc; // Deferred updates of locals, expressions, and monitors GrowableArray _deferred_locals_updates; - void inc_relock_count_after_wait() { - _relock_count_after_wait++; - } - - int get_and_reset_relock_count_after_wait() { - int result = _relock_count_after_wait; - _relock_count_after_wait = 0; - return result; - } - - GrowableArray* deferred_locals() { return &_deferred_locals_updates; } - - JvmtiDeferredUpdates() : - _relock_count_after_wait(0), +public: + JvmtiDeferredUpdates(address original_pc) : + _original_pc(original_pc), _deferred_locals_updates((AnyObj::set_allocation_type((address) &_deferred_locals_updates, - AnyObj::C_HEAP), 1), mtCompiler) { } + AnyObj::C_HEAP), 1), mtCompiler) { } -public: ~JvmtiDeferredUpdates(); - static void create_for(JavaThread* thread); - - static GrowableArray* deferred_locals(JavaThread* jt) { - return jt->deferred_updates() == nullptr ? nullptr : jt->deferred_updates()->deferred_locals(); - } - - // Relocking has to be deferred if the lock owning thread is currently waiting on the monitor. - static int get_and_reset_relock_count_after_wait(JavaThread* jt); - static void inc_relock_count_after_wait(JavaThread* thread); - - // Delete deferred updates for the compiled frame with id 'frame_id' on the - // given thread's stack. The thread's JvmtiDeferredUpdates instance will be - // deleted too if no updates remain. - static void delete_updates_for_frame(JavaThread* jt, intptr_t* frame_id); - - // Number of deferred updates - int count() const { - return _deferred_locals_updates.length() + (_relock_count_after_wait > 0 ? 1 : 0); - } + address original_pc() const { return _original_pc; } + GrowableArray* deferred_locals() { return &_deferred_locals_updates; } }; #endif // SHARE_PRIMS_JVMTIDEFERREDUPDATES_HPP diff --git a/src/hotspot/share/runtime/deoptimization.cpp b/src/hotspot/share/runtime/deoptimization.cpp index 9992d42622f65..45ac259fafb7d 100644 --- a/src/hotspot/share/runtime/deoptimization.cpp +++ b/src/hotspot/share/runtime/deoptimization.cpp @@ -393,7 +393,7 @@ static bool rematerialize_objects(JavaThread* thread, int exec_mode, nmethod* co static void restore_eliminated_locks(JavaThread* thread, GrowableArray* chunk, bool realloc_failures, frame& deoptee, int exec_mode, bool& deoptimized_objects) { JavaThread* deoptee_thread = chunk->at(0)->thread(); - assert(!EscapeBarrier::objs_are_deoptimized(deoptee_thread, deoptee.id()), "must relock just once"); + assert(!EscapeBarrier::objs_are_deoptimized(deoptee_thread, deoptee), "must relock just once"); assert(thread == Thread::current(), "should be"); HandleMark hm(thread); #ifndef PRODUCT @@ -542,7 +542,7 @@ Deoptimization::UnrollBlock* Deoptimization::fetch_unroll_info_helper(JavaThread #if COMPILER2_OR_JVMCI if ((jvmci_enabled COMPILER2_PRESENT( || ((DoEscapeAnalysis || EliminateNestedLocks) && EliminateLocks) )) - && !EscapeBarrier::objs_are_deoptimized(current, deoptee.id())) { + && !EscapeBarrier::objs_are_deoptimized(current, deoptee)) { bool unused = false; restore_eliminated_locks(current, chunk, realloc_failures, deoptee, exec_mode, unused); } @@ -579,8 +579,7 @@ Deoptimization::UnrollBlock* Deoptimization::fetch_unroll_info_helper(JavaThread // Now that the vframeArray has been created if we have any deferred local writes // added by jvmti then we can free up that structure as the data is now in the // vframeArray - - JvmtiDeferredUpdates::delete_updates_for_frame(current, array->original().id()); + deoptee.clear_deferred_locals(); // Compute the caller frame based on the sender sp of stub_frame and stored frame sizes info. CodeBlob* cb = stub_frame.cb(); @@ -1652,7 +1651,7 @@ bool Deoptimization::relock_objects(JavaThread* thread, GrowableArraylock()->set_bad_metadata_deopt(); } #endif - JvmtiDeferredUpdates::inc_relock_count_after_wait(deoptee_thread); + deoptee_thread->inc_relock_count_after_wait(); continue; } } diff --git a/src/hotspot/share/runtime/escapeBarrier.cpp b/src/hotspot/share/runtime/escapeBarrier.cpp index 410609beb9541..385b817023777 100644 --- a/src/hotspot/share/runtime/escapeBarrier.cpp +++ b/src/hotspot/share/runtime/escapeBarrier.cpp @@ -50,16 +50,14 @@ #if COMPILER2_OR_JVMCI // Returns true iff objects were reallocated and relocked because of access through JVMTI -bool EscapeBarrier::objs_are_deoptimized(JavaThread* thread, intptr_t* fr_id) { +bool EscapeBarrier::objs_are_deoptimized(JavaThread* thread, frame fr) { // first/oldest update holds the flag - GrowableArrayView* list = JvmtiDeferredUpdates::deferred_locals(thread); + GrowableArrayView* list = fr.deferred_locals(); bool result = false; if (list != nullptr) { for (int i = 0; i < list->length(); i++) { - if (list->at(i)->matches(fr_id)) { - result = list->at(i)->objects_are_deoptimized(); - break; - } + guarantee(list->at(i)->matches(fr.id()), "must match"); + result = list->at(i)->objects_are_deoptimized(); } } return result; @@ -100,7 +98,7 @@ bool EscapeBarrier::deoptimize_objects(int d1, int d2) { // Deoptimize frame and local objects if any exist. // If cvf is deeper than depth, then we deoptimize iff local objects are passed as args. bool should_deopt = cur_depth <= d2 ? cvf->has_ea_local_in_scope() : cvf->arg_escape(); - if (should_deopt && !deoptimize_objects(cvf->fr().id())) { + if (should_deopt && !deoptimize_objects(cvf->fr())) { // reallocation of scalar replaced objects failed because heap is exhausted return false; } @@ -147,7 +145,7 @@ bool EscapeBarrier::deoptimize_objects_all_threads() { if (vf->is_compiled_frame()) { compiledVFrame* cvf = compiledVFrame::cast(vf); if ((cvf->has_ea_local_in_scope() || cvf->arg_escape()) && - !deoptimize_objects_internal(jt, cvf->fr().id())) { + !deoptimize_objects_internal(jt, cvf->fr())) { return false; // reallocation failure } // move to top frame @@ -301,18 +299,16 @@ void EscapeBarrier::thread_removed(JavaThread* jt) { } // Remember that objects were reallocated and relocked for the compiled frame with the given id -static void set_objs_are_deoptimized(JavaThread* thread, intptr_t* fr_id) { +static void set_objs_are_deoptimized(JavaThread* thread, frame fr) { // set in first/oldest update - GrowableArrayView* list = - JvmtiDeferredUpdates::deferred_locals(thread); + GrowableArrayView* list = fr.deferred_locals(); + DEBUG_ONLY(bool found = false); if (list != nullptr) { for (int i = 0; i < list->length(); i++) { - if (list->at(i)->matches(fr_id)) { - DEBUG_ONLY(found = true); - list->at(i)->set_objs_are_deoptimized(); - break; - } + guarantee (list->at(i)->matches(fr.id()), "must match"); + DEBUG_ONLY(found = true); + list->at(i)->set_objs_are_deoptimized(); } } assert(found, "variable set should exist at least for one vframe"); @@ -324,13 +320,14 @@ static void set_objs_are_deoptimized(JavaThread* thread, intptr_t* fr_id) { // Deoptimized objects are kept as JVMTI deferred updates until the compiled // frame is replaced with interpreter frames. Returns false iff at least one // reallocation failed. -bool EscapeBarrier::deoptimize_objects_internal(JavaThread* deoptee, intptr_t* fr_id) { +bool EscapeBarrier::deoptimize_objects_internal(JavaThread* deoptee, frame fr) { assert(barrier_active(), "should not call"); JavaThread* ct = calling_thread(); bool realloc_failures = false; + intptr_t* fr_id = fr.id(); - if (!objs_are_deoptimized(deoptee, fr_id)) { + if (!objs_are_deoptimized(deoptee, fr)) { // Make sure the frame identified by fr_id is deoptimized and fetch its last vframe compiledVFrame* last_cvf; bool fr_is_deoptimized; @@ -348,9 +345,13 @@ bool EscapeBarrier::deoptimize_objects_internal(JavaThread* deoptee, intptr_t* f Deoptimization::deoptimize_frame(deoptee, fr_id); } else { last_cvf = compiledVFrame::cast(vframe::new_vframe(fst.current(), fst.register_map(), deoptee)); + // Refresh the frame now that it has been deoptimized + fr = *fst.current(); } } while(!fr_is_deoptimized); + assert(fr.is_deoptimized_frame(), "must be deoptimized"); + // collect inlined frames compiledVFrame* cvf = last_cvf; GrowableArray* vfs = new GrowableArray(10); @@ -368,7 +369,7 @@ bool EscapeBarrier::deoptimize_objects_internal(JavaThread* deoptee, intptr_t* f cvf = vfs->at(frame_index); cvf->create_deferred_updates_after_object_deoptimization(); } - set_objs_are_deoptimized(deoptee, fr_id); + set_objs_are_deoptimized(deoptee, fr); } } return !realloc_failures; diff --git a/src/hotspot/share/runtime/escapeBarrier.hpp b/src/hotspot/share/runtime/escapeBarrier.hpp index 454e0b555e118..916491a98f6fe 100644 --- a/src/hotspot/share/runtime/escapeBarrier.hpp +++ b/src/hotspot/share/runtime/escapeBarrier.hpp @@ -59,12 +59,12 @@ class EscapeBarrier : StackObj { void resume_all(); // Deoptimize the given frame and deoptimize objects with optimizations based on escape analysis. - bool deoptimize_objects_internal(JavaThread* deoptee, intptr_t* fr_id); + bool deoptimize_objects_internal(JavaThread* deoptee, frame fr); // Deoptimize objects, i.e. reallocate and relock them. The target frames are deoptimized. // The methods return false iff at least one reallocation failed. - bool deoptimize_objects(intptr_t* fr_id) { - return deoptimize_objects_internal(deoptee_thread(), fr_id); + bool deoptimize_objects(frame fr) { + return deoptimize_objects_internal(deoptee_thread(), fr); } public: @@ -118,7 +118,7 @@ class EscapeBarrier : StackObj { #if COMPILER2_OR_JVMCI // Returns true iff objects were reallocated and relocked because of access through JVMTI. - static bool objs_are_deoptimized(JavaThread* thread, intptr_t* fr_id); + static bool objs_are_deoptimized(JavaThread* thread, frame fr); ~EscapeBarrier() { if (!barrier_active()) return; diff --git a/src/hotspot/share/runtime/frame.cpp b/src/hotspot/share/runtime/frame.cpp index 738b89e77bd47..664b6d4feb2ba 100644 --- a/src/hotspot/share/runtime/frame.cpp +++ b/src/hotspot/share/runtime/frame.cpp @@ -42,6 +42,7 @@ #include "oops/oop.inline.hpp" #include "oops/stackChunkOop.inline.hpp" #include "oops/verifyOopClosure.hpp" +#include "prims/jvmtiDeferredUpdates.hpp" #include "prims/methodHandles.hpp" #include "runtime/continuation.hpp" #include "runtime/continuationEntry.inline.hpp" @@ -233,6 +234,24 @@ void frame::set_pc(address newpc) { } +void frame::set_original_pc(nmethod* nm, address pc) { + *nm->orig_pc_addr(this) = pc; +} + +address frame::get_original_pc(nmethod* nm) const { + address ptr = *nm->orig_pc_addr(this); + if (ptr != nullptr) { + if (nm->is_deopt_pc(_pc)) { + if (CodeCache::contains(ptr)) { + return ptr; + } + JvmtiDeferredUpdates* updates = (JvmtiDeferredUpdates*) ptr; + return updates->original_pc(); + } + } + return ptr; +} + // type testers bool frame::is_ignored_frame() const { return false; // FIXME: some LambdaForm frames should be ignored @@ -358,7 +377,7 @@ void frame::deoptimize(JavaThread* thread) { NativePostCallNop* inst = nativePostCallNop_at(pc()); // Save the original pc before we patch in the new one - nm->set_original_pc(this, pc()); + set_original_pc(nm, pc()); patch_pc(thread, deopt); assert(is_deoptimized_frame(), "must be"); @@ -379,6 +398,50 @@ void frame::deoptimize(JavaThread* thread) { #endif // ASSERT } +GrowableArray* frame::deferred_locals() const { + nmethod* nm = _cb->as_nmethod(); + if (!is_deoptimized_frame()) { + guarantee(is_compiled_frame(), "must at least be compiled"); + return nullptr; + } + + address ptr = *nm->orig_pc_addr(this); + if (ptr != nullptr) { + if (CodeCache::contains(ptr)) { + return nullptr; + } + JvmtiDeferredUpdates* updates = (JvmtiDeferredUpdates*) ptr; + return updates->deferred_locals(); + } + return nullptr; +} + +GrowableArray* frame::create_deferred_locals() const { + nmethod* nm = _cb->as_nmethod(); + guarantee(is_deoptimized_frame(), "must be deoptimized frame"); + + address ptr = *nm->orig_pc_addr(this); + + // deferred updates can only be set after deopt + guarantee(CodeCache::contains(ptr), "should currently be deopt pc"); + + JvmtiDeferredUpdates* updates = new JvmtiDeferredUpdates(ptr); + *nm->orig_pc_addr(this) = (address) updates; + return updates->deferred_locals(); +} + +void frame::clear_deferred_locals() { + if (deferred_locals() != nullptr) { + nmethod* nm = _cb->as_nmethod(); + guarantee(is_deoptimized_frame(), "must be deoptimized frame"); + address ptr = *nm->orig_pc_addr(this); + guarantee(!CodeCache::contains(ptr), "should be DeferredUpdates*"); + JvmtiDeferredUpdates* updates = (JvmtiDeferredUpdates*) ptr; + *nm->orig_pc_addr(this) = updates->original_pc(); + delete updates; + } +} + frame frame::java_sender() const { RegisterMap map(JavaThread::current(), RegisterMap::UpdateMap::skip, @@ -998,6 +1061,15 @@ void frame::oops_nmethod_do(OopClosure* f, NMethodClosure* cf, DerivedOopClosure // closure decides how it wants nmethods to be traced. if (cf != nullptr && _cb->is_nmethod()) cf->do_nmethod(_cb->as_nmethod()); + + if (is_deoptimized_frame() && _cb->is_nmethod()) { + GrowableArray* list = deferred_locals(); + if (list != nullptr) { + for (int i = 0; i < list->length(); i++) { + list->at(i)->oops_do(f); + } + } + } } class CompiledArgumentOopFinder: public SignatureIterator { diff --git a/src/hotspot/share/runtime/frame.hpp b/src/hotspot/share/runtime/frame.hpp index fe531cff8a646..3922b51f9f7c0 100644 --- a/src/hotspot/share/runtime/frame.hpp +++ b/src/hotspot/share/runtime/frame.hpp @@ -47,6 +47,7 @@ class Method; class methodHandle; class RegisterMap; class vframeArray; +class jvmtiDeferredLocalVariableSet; enum class DerivedPointerIterationMode { _with_table, @@ -126,6 +127,11 @@ class frame { // (b) it is a deopt PC address get_deopt_original_pc() const; + // Accessor/mutator for the original pc of a frame before a frame was deopted. + address get_original_pc(nmethod* nm) const; + void set_original_pc(nmethod* nm, address pc); + + void set_pc(address newpc); intptr_t* sp() const { assert_absolute(); return _sp; } @@ -280,6 +286,10 @@ class frame { // Support for deoptimization void deoptimize(JavaThread* thread); + GrowableArray* deferred_locals() const; + GrowableArray* create_deferred_locals() const; + void clear_deferred_locals(); + // The frame's original SP, before any extension by an interpreted callee; // used for packing debug info into vframeArray objects and vframeArray lookup. intptr_t* unextended_sp() const; diff --git a/src/hotspot/share/runtime/frame.inline.hpp b/src/hotspot/share/runtime/frame.inline.hpp index aa2de69a7396a..7b842cefc730f 100644 --- a/src/hotspot/share/runtime/frame.inline.hpp +++ b/src/hotspot/share/runtime/frame.inline.hpp @@ -76,7 +76,7 @@ inline address frame::get_deopt_original_pc() const { nmethod* nm = _cb->as_nmethod_or_null(); if (nm != nullptr && nm->is_deopt_pc(_pc)) { - return nm->get_original_pc(this); + return get_original_pc(nm); } return nullptr; } diff --git a/src/hotspot/share/runtime/javaThread.cpp b/src/hotspot/share/runtime/javaThread.cpp index fd9f75c41b424..dd231cb1d6a1e 100644 --- a/src/hotspot/share/runtime/javaThread.cpp +++ b/src/hotspot/share/runtime/javaThread.cpp @@ -425,7 +425,7 @@ JavaThread::JavaThread(MemTag mem_tag) : _deopt_nmethod(nullptr), _vframe_array_head(nullptr), _vframe_array_last(nullptr), - _jvmti_deferred_updates(nullptr), + _relock_count_after_wait(0), _callee_target(nullptr), _vm_result(nullptr), _vm_result_2(nullptr), @@ -687,14 +687,8 @@ JavaThread::~JavaThread() { delete old_array; } - JvmtiDeferredUpdates* updates = deferred_updates(); - if (updates != nullptr) { - // This can only happen if thread is destroyed before deoptimization occurs. - assert(updates->count() > 0, "Updates holder not deleted"); - // free deferred updates. - delete updates; - set_deferred_updates(nullptr); - } + // This can only happen if thread is destroyed before deoptimization occurs. + // XXX release deferred updates for any live frames // All Java related clean up happens in exit ThreadSafepointState::destroy(this); @@ -1401,14 +1395,6 @@ void JavaThread::oops_do_no_frames(OopClosure* f, NMethodClosure* cf) { DEBUG_ONLY(verify_frame_info();) assert(vframe_array_head() == nullptr, "deopt in progress at a safepoint!"); - // If we have deferred set_locals there might be oops waiting to be - // written - GrowableArray* list = JvmtiDeferredUpdates::deferred_locals(this); - if (list != nullptr) { - for (int i = 0; i < list->length(); i++) { - list->at(i)->oops_do(f); - } - } // Traverse instance variables at the end since the GC may be moving things // around using this function diff --git a/src/hotspot/share/runtime/javaThread.hpp b/src/hotspot/share/runtime/javaThread.hpp index e24138583b894..7f13094d88071 100644 --- a/src/hotspot/share/runtime/javaThread.hpp +++ b/src/hotspot/share/runtime/javaThread.hpp @@ -58,7 +58,6 @@ class InternalOOMEMark; class JNIHandleBlock; class JVMCIRuntime; -class JvmtiDeferredUpdates; class JvmtiSampledObjectAllocEventCollector; class JvmtiThreadState; @@ -129,10 +128,9 @@ class JavaThread: public Thread { nmethod* _deopt_nmethod; // nmethod that is currently being deoptimized vframeArray* _vframe_array_head; // Holds the heap of the active vframeArrays vframeArray* _vframe_array_last; // Holds last vFrameArray we popped - // Holds updates by JVMTI agents for compiled frames that cannot be performed immediately. They - // will be carried out as soon as possible which, in most cases, is just before deoptimization of - // the frame, when control returns to it. - JvmtiDeferredUpdates* _jvmti_deferred_updates; + + // Relocking has to be deferred if the lock owning thread is currently waiting on the monitor. + int _relock_count_after_wait; // Handshake value for fixing 6243940. We need a place for the i2c // adapter to store the callee Method*. This value is NEVER live @@ -749,9 +747,15 @@ class JavaThread: public Thread { void set_vframe_array_head(vframeArray* value) { _vframe_array_head = value; } vframeArray* vframe_array_head() const { return _vframe_array_head; } - // Side structure for deferring update of java frame locals until deopt occurs - JvmtiDeferredUpdates* deferred_updates() const { return _jvmti_deferred_updates; } - void set_deferred_updates(JvmtiDeferredUpdates* du) { _jvmti_deferred_updates = du; } + void inc_relock_count_after_wait() { + _relock_count_after_wait++; + } + + int get_and_reset_relock_count_after_wait() { + int result = _relock_count_after_wait; + _relock_count_after_wait = 0; + return result; + } // These only really exist to make debugging deopt problems simpler diff --git a/src/hotspot/share/runtime/objectMonitor.cpp b/src/hotspot/share/runtime/objectMonitor.cpp index 7d1998ca849c0..a4bc6eca5cabd 100644 --- a/src/hotspot/share/runtime/objectMonitor.cpp +++ b/src/hotspot/share/runtime/objectMonitor.cpp @@ -1857,7 +1857,7 @@ void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) { current->set_current_waiting_monitor(nullptr); guarantee(_recursions == 0, "invariant"); - int relock_count = JvmtiDeferredUpdates::get_and_reset_relock_count_after_wait(current); + int relock_count = current->get_and_reset_relock_count_after_wait(); _recursions = save // restore the old recursion count + relock_count; // increased by the deferred relock count current->inc_held_monitor_count(relock_count); // Deopt never entered these counts. diff --git a/src/hotspot/share/runtime/vframe_hp.cpp b/src/hotspot/share/runtime/vframe_hp.cpp index 49ed47835d6a3..802f973c1b08e 100644 --- a/src/hotspot/share/runtime/vframe_hp.cpp +++ b/src/hotspot/share/runtime/vframe_hp.cpp @@ -69,7 +69,7 @@ StackValueCollection* compiledVFrame::locals() const { // Replace the original values with any stores that have been // performed through compiledVFrame::update_locals. if (!register_map()->in_cont()) { // LOOM TODO - GrowableArray* list = JvmtiDeferredUpdates::deferred_locals(thread()); + GrowableArray* list = fr().deferred_locals(); if (list != nullptr ) { // In real life this never happens or is typically a single element search for (int i = 0; i < list->length(); i++) { @@ -109,8 +109,7 @@ void compiledVFrame::update_monitor(int index, MonitorInfo* val) { void compiledVFrame::update_deferred_value(BasicType type, int index, jvalue value) { assert(fr().is_deoptimized_frame(), "frame must be scheduled for deoptimization"); - assert(!Continuation::is_frame_in_continuation(thread(), fr()), "No support for deferred values in continuations"); - GrowableArray* deferred = JvmtiDeferredUpdates::deferred_locals(thread()); + GrowableArray* deferred = fr().deferred_locals(); jvmtiDeferredLocalVariableSet* locals = nullptr; if (deferred != nullptr ) { // See if this vframe has already had locals with deferred writes @@ -124,8 +123,8 @@ void compiledVFrame::update_deferred_value(BasicType type, int index, jvalue val } else { // No deferred updates pending for this thread. // allocate in C heap - JvmtiDeferredUpdates::create_for(thread()); - deferred = JvmtiDeferredUpdates::deferred_locals(thread()); + + deferred = fr().create_deferred_locals(); } if (locals == nullptr) { locals = new jvmtiDeferredLocalVariableSet(method(), bci(), fr().id(), vframe_id()); @@ -202,7 +201,7 @@ StackValueCollection* compiledVFrame::expressions() const { if (!register_map()->in_cont()) { // LOOM TODO // Replace the original values with any stores that have been // performed through compiledVFrame::update_stack. - GrowableArray* list = JvmtiDeferredUpdates::deferred_locals(thread()); + GrowableArray* list = fr().deferred_locals(); if (list != nullptr ) { // In real life this never happens or is typically a single element search for (int i = 0; i < list->length(); i++) { @@ -285,7 +284,7 @@ GrowableArray* compiledVFrame::monitors() const { // Replace the original values with any stores that have been // performed through compiledVFrame::update_monitors. if (thread() == nullptr) return result; // Unmounted continuations have no thread so nothing to do. - GrowableArrayView* list = JvmtiDeferredUpdates::deferred_locals(thread()); + GrowableArrayView* list = fr().deferred_locals(); if (list != nullptr ) { // In real life this never happens or is typically a single element search for (int i = 0; i < list->length(); i++) { diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotStackFrameReference.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotStackFrameReference.java index db91175fc8dc9..07933f3a64e83 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotStackFrameReference.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotStackFrameReference.java @@ -63,9 +63,6 @@ public boolean isVirtual(int index) { @Override public void materializeVirtualObjects(boolean invalidateCode) { - if (Thread.currentThread().isVirtual()) { - throw new IllegalArgumentException("cannot materialize frames of a virtual thread"); - } compilerToVM.materializeVirtualObjects(this, invalidateCode); } diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotStackIntrospection.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotStackIntrospection.java index 0b18bcbea5fd6..d7148a09436fc 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotStackIntrospection.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotStackIntrospection.java @@ -42,7 +42,6 @@ public T iterateFrames(ResolvedJavaMethod[] initialMethods, ResolvedJavaMeth @Override public boolean canMaterializeVirtualObjects() { - // Virtual threads do not support materializing locals (JDK-8307125) - return !Thread.currentThread().isVirtual(); + return true; } } diff --git a/test/hotspot/jtreg/compiler/jvmci/compilerToVM/MaterializeVirtualObjectTest.java b/test/hotspot/jtreg/compiler/jvmci/compilerToVM/MaterializeVirtualObjectTest.java index 1dcef0c96cd64..386eba325d143 100644 --- a/test/hotspot/jtreg/compiler/jvmci/compilerToVM/MaterializeVirtualObjectTest.java +++ b/test/hotspot/jtreg/compiler/jvmci/compilerToVM/MaterializeVirtualObjectTest.java @@ -158,11 +158,7 @@ public static void main(String[] args) { throw new SkippedException("Test needs compilation level 4"); } - try { - new MaterializeVirtualObjectTest().test(); - } catch (MaterializationNotSupported e) { - Asserts.assertTrue(Thread.currentThread().isVirtual()); - } + new MaterializeVirtualObjectTest().test(); } private static String getName() { @@ -234,14 +230,6 @@ private void recurse(int depth, int iteration) { } } - private static void materializeVirtualObjects(InspectedFrame f, boolean invalidateCode) { - try { - f.materializeVirtualObjects(invalidateCode); - } catch (IllegalArgumentException e) { - throw new MaterializationNotSupported(e); - } - } - private void checkStructure(boolean materialize) { boolean[] framesSeen = new boolean[2]; Object[] helpers = new Object[1]; @@ -260,7 +248,7 @@ private void checkStructure(boolean materialize) { Asserts.assertEQ(((Helper) f.getLocal(3)).string, "foo", "innerHelper.string should be foo"); helpers[0] = f.getLocal(1); if (materialize) { - materializeVirtualObjects(f, false); + f.materializeVirtualObjects(false); } return null; //continue } else { @@ -318,7 +306,7 @@ private void check(int iteration) { Asserts.assertTrue(notMaterialized.hasVirtualObjects(), getName() + ": notMaterialized frame has no virtual object before materialization"); // materialize - materializeVirtualObjects(materialized, INVALIDATE); + materialized.materializeVirtualObjects(INVALIDATE); // check that only not materialized frame has virtual objects Asserts.assertFalse(materialized.hasVirtualObjects(), getName() + " : materialized has virtual object after materialization"); @@ -343,10 +331,4 @@ public Helper(String s) { this.string = s; } } - - static class MaterializationNotSupported extends RuntimeException { - public MaterializationNotSupported(Throwable cause) { - super(cause); - } - } }