From 7bf531324404419e7de3e83e245d351e1a4e4499 Mon Sep 17 00:00:00 2001 From: Jorn Vernee Date: Thu, 18 Jul 2024 11:00:39 +0000 Subject: [PATCH] 8335480: Only deoptimize threads if needed when closing shared arena Reviewed-by: mcimadamore, kvn, uschindler, vlivanov, eosterlund --- src/hotspot/share/c1/c1_Compilation.cpp | 2 + src/hotspot/share/c1/c1_Compilation.hpp | 3 + src/hotspot/share/c1/c1_GraphBuilder.cpp | 3 + src/hotspot/share/ci/ciEnv.cpp | 2 + src/hotspot/share/ci/ciEnv.hpp | 1 + src/hotspot/share/ci/ciMethod.cpp | 8 + src/hotspot/share/ci/ciMethod.hpp | 1 + src/hotspot/share/code/nmethod.cpp | 1 + src/hotspot/share/code/nmethod.hpp | 4 + src/hotspot/share/jvmci/jvmciRuntime.cpp | 1 + src/hotspot/share/opto/compile.cpp | 1 + src/hotspot/share/opto/compile.hpp | 3 + src/hotspot/share/opto/output.cpp | 1 + src/hotspot/share/opto/parse1.cpp | 4 + .../share/prims/scopedMemoryAccess.cpp | 138 +++++++++---- src/hotspot/share/runtime/vframe.cpp | 86 ++++---- src/hotspot/share/runtime/vframe.hpp | 18 +- .../jdk/java/foreign/TestConcurrentClose.java | 188 ++++++++++++++++++ test/jdk/java/foreign/TestHandshake.java | 2 +- .../java/lang/foreign/ConcurrentClose.java | 85 ++++++++ 20 files changed, 463 insertions(+), 89 deletions(-) create mode 100644 test/jdk/java/foreign/TestConcurrentClose.java create mode 100644 test/micro/org/openjdk/bench/java/lang/foreign/ConcurrentClose.java diff --git a/src/hotspot/share/c1/c1_Compilation.cpp b/src/hotspot/share/c1/c1_Compilation.cpp index bef1fae7f74..48201a86053 100644 --- a/src/hotspot/share/c1/c1_Compilation.cpp +++ b/src/hotspot/share/c1/c1_Compilation.cpp @@ -437,6 +437,7 @@ void Compilation::install_code(int frame_size) { has_unsafe_access(), SharedRuntime::is_wide_vector(max_vector_size()), has_monitors(), + has_scoped_access(), _immediate_oops_patched ); } @@ -578,6 +579,7 @@ Compilation::Compilation(AbstractCompiler* compiler, ciEnv* env, ciMethod* metho , _has_method_handle_invokes(false) , _has_reserved_stack_access(method->has_reserved_stack_access()) , _has_monitors(method->is_synchronized() || method->has_monitor_bytecodes()) +, _has_scoped_access(method->is_scoped()) , _install_code(install_code) , _bailout_msg(nullptr) , _first_failure_details(nullptr) diff --git a/src/hotspot/share/c1/c1_Compilation.hpp b/src/hotspot/share/c1/c1_Compilation.hpp index f344695509e..d55a8debca7 100644 --- a/src/hotspot/share/c1/c1_Compilation.hpp +++ b/src/hotspot/share/c1/c1_Compilation.hpp @@ -84,6 +84,7 @@ class Compilation: public StackObj { bool _has_method_handle_invokes; // True if this method has MethodHandle invokes. bool _has_reserved_stack_access; bool _has_monitors; // Fastpath monitors detection for Continuations + bool _has_scoped_access; // For shared scope closure bool _install_code; const char* _bailout_msg; CompilationFailureInfo* _first_failure_details; // Details for the first failure happening during compilation @@ -143,6 +144,7 @@ class Compilation: public StackObj { bool has_fpu_code() const { return _has_fpu_code; } bool has_unsafe_access() const { return _has_unsafe_access; } bool has_monitors() const { return _has_monitors; } + bool has_scoped_access() const { return _has_scoped_access; } bool has_irreducible_loops() const { return _has_irreducible_loops; } int max_vector_size() const { return 0; } ciMethod* method() const { return _method; } @@ -175,6 +177,7 @@ class Compilation: public StackObj { void set_would_profile(bool f) { _would_profile = f; } void set_has_access_indexed(bool f) { _has_access_indexed = f; } void set_has_monitors(bool f) { _has_monitors = f; } + void set_has_scoped_access(bool f) { _has_scoped_access = f; } // Add a set of exception handlers covering the given PC offset void add_exception_handlers_for_pco(int pco, XHandlers* exception_handlers); // Statistics gathering diff --git a/src/hotspot/share/c1/c1_GraphBuilder.cpp b/src/hotspot/share/c1/c1_GraphBuilder.cpp index 46d74db6d6a..0493b0458cd 100644 --- a/src/hotspot/share/c1/c1_GraphBuilder.cpp +++ b/src/hotspot/share/c1/c1_GraphBuilder.cpp @@ -3518,6 +3518,9 @@ static void set_flags_for_inlined_callee(Compilation* compilation, ciMethod* cal if (callee->is_synchronized() || callee->has_monitor_bytecodes()) { compilation->set_has_monitors(true); } + if (callee->is_scoped()) { + compilation->set_has_scoped_access(true); + } } bool GraphBuilder::try_inline(ciMethod* callee, bool holder_known, bool ignore_return, Bytecodes::Code bc, Value receiver) { diff --git a/src/hotspot/share/ci/ciEnv.cpp b/src/hotspot/share/ci/ciEnv.cpp index 565300f43b4..3079d469ebe 100644 --- a/src/hotspot/share/ci/ciEnv.cpp +++ b/src/hotspot/share/ci/ciEnv.cpp @@ -1032,6 +1032,7 @@ void ciEnv::register_method(ciMethod* target, bool has_unsafe_access, bool has_wide_vectors, bool has_monitors, + bool has_scoped_access, int immediate_oops_patched) { VM_ENTRY_MARK; nmethod* nm = nullptr; @@ -1124,6 +1125,7 @@ void ciEnv::register_method(ciMethod* target, nm->set_has_unsafe_access(has_unsafe_access); nm->set_has_wide_vectors(has_wide_vectors); nm->set_has_monitors(has_monitors); + nm->set_has_scoped_access(has_scoped_access); assert(!method->is_synchronized() || nm->has_monitors(), ""); if (entry_bci == InvocationEntryBci) { diff --git a/src/hotspot/share/ci/ciEnv.hpp b/src/hotspot/share/ci/ciEnv.hpp index be63632ba4e..5ee9b420033 100644 --- a/src/hotspot/share/ci/ciEnv.hpp +++ b/src/hotspot/share/ci/ciEnv.hpp @@ -383,6 +383,7 @@ public: bool has_unsafe_access, bool has_wide_vectors, bool has_monitors, + bool has_scoped_access, int immediate_oops_patched); // Access to certain well known ciObjects. diff --git a/src/hotspot/share/ci/ciMethod.cpp b/src/hotspot/share/ci/ciMethod.cpp index bc95f322d0d..b2ac3a8b217 100644 --- a/src/hotspot/share/ci/ciMethod.cpp +++ b/src/hotspot/share/ci/ciMethod.cpp @@ -960,6 +960,14 @@ bool ciMethod::is_object_initializer() const { return name() == ciSymbols::object_initializer_name(); } +// ------------------------------------------------------------------ +// ciMethod::is_scoped +// +// Return true for methods annotated with @Scoped +bool ciMethod::is_scoped() const { + return get_Method()->is_scoped(); +} + // ------------------------------------------------------------------ // ciMethod::has_member_arg // diff --git a/src/hotspot/share/ci/ciMethod.hpp b/src/hotspot/share/ci/ciMethod.hpp index a7c18b09f13..5a4a1dd1c7f 100644 --- a/src/hotspot/share/ci/ciMethod.hpp +++ b/src/hotspot/share/ci/ciMethod.hpp @@ -360,6 +360,7 @@ class ciMethod : public ciMetadata { bool is_unboxing_method() const; bool is_vector_method() const; bool is_object_initializer() const; + bool is_scoped() const; bool can_be_statically_bound(ciInstanceKlass* context) const; diff --git a/src/hotspot/share/code/nmethod.cpp b/src/hotspot/share/code/nmethod.cpp index ea56ee61298..54aeac6be04 100644 --- a/src/hotspot/share/code/nmethod.cpp +++ b/src/hotspot/share/code/nmethod.cpp @@ -1234,6 +1234,7 @@ void nmethod::init_defaults(CodeBuffer *code_buffer, CodeOffsets* offsets) { _has_method_handle_invokes = 0; _has_wide_vectors = 0; _has_monitors = 0; + _has_scoped_access = 0; _has_flushed_dependencies = 0; _is_unlinked = 0; _load_reported = 0; // jvmti state diff --git a/src/hotspot/share/code/nmethod.hpp b/src/hotspot/share/code/nmethod.hpp index ee0fe004331..095c0ba4a26 100644 --- a/src/hotspot/share/code/nmethod.hpp +++ b/src/hotspot/share/code/nmethod.hpp @@ -271,6 +271,7 @@ class nmethod : public CodeBlob { _has_method_handle_invokes:1,// Has this method MethodHandle invokes? _has_wide_vectors:1, // Preserve wide vectors at safepoints _has_monitors:1, // Fastpath monitor detection for continuations + _has_scoped_access:1, // used by for shared scope closure (scopedMemoryAccess.cpp) _has_flushed_dependencies:1, // Used for maintenance of dependencies (under CodeCache_lock) _is_unlinked:1, // mark during class unloading _load_reported:1; // used by jvmti to track if an event has been posted for this nmethod @@ -664,6 +665,9 @@ public: bool has_monitors() const { return _has_monitors; } void set_has_monitors(bool z) { _has_monitors = z; } + bool has_scoped_access() const { return _has_scoped_access; } + void set_has_scoped_access(bool z) { _has_scoped_access = z; } + bool has_method_handle_invokes() const { return _has_method_handle_invokes; } void set_has_method_handle_invokes(bool z) { _has_method_handle_invokes = z; } diff --git a/src/hotspot/share/jvmci/jvmciRuntime.cpp b/src/hotspot/share/jvmci/jvmciRuntime.cpp index 9770e329a88..371a6540898 100644 --- a/src/hotspot/share/jvmci/jvmciRuntime.cpp +++ b/src/hotspot/share/jvmci/jvmciRuntime.cpp @@ -2183,6 +2183,7 @@ JVMCI::CodeInstallResult JVMCIRuntime::register_method(JVMCIEnv* JVMCIENV, nm->set_has_unsafe_access(has_unsafe_access); nm->set_has_wide_vectors(has_wide_vector); nm->set_has_monitors(has_monitors); + nm->set_has_scoped_access(true); // conservative JVMCINMethodData* data = nm->jvmci_nmethod_data(); assert(data != nullptr, "must be"); diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp index 4f5c7bf5c72..a9d73364e2d 100644 --- a/src/hotspot/share/opto/compile.cpp +++ b/src/hotspot/share/opto/compile.cpp @@ -1061,6 +1061,7 @@ void Compile::Init(bool aliasing) { set_do_vector_loop(false); set_has_monitors(false); + set_has_scoped_access(false); if (AllowVectorizeOnDemand) { if (has_method() && _directive->VectorizeOption) { diff --git a/src/hotspot/share/opto/compile.hpp b/src/hotspot/share/opto/compile.hpp index 7c998d59d37..15c91f6ad12 100644 --- a/src/hotspot/share/opto/compile.hpp +++ b/src/hotspot/share/opto/compile.hpp @@ -354,6 +354,7 @@ class Compile : public Phase { // JSR 292 bool _has_method_handle_invokes; // True if this method has MethodHandle invokes. bool _has_monitors; // Metadata transfered to nmethod to enable Continuations lock-detection fastpath + bool _has_scoped_access; // For shared scope closure bool _clinit_barrier_on_entry; // True if clinit barrier is needed on nmethod entry int _loop_opts_cnt; // loop opts round uint _stress_seed; // Seed for stress testing @@ -672,6 +673,8 @@ private: void set_clinit_barrier_on_entry(bool z) { _clinit_barrier_on_entry = z; } bool has_monitors() const { return _has_monitors; } void set_has_monitors(bool v) { _has_monitors = v; } + bool has_scoped_access() const { return _has_scoped_access; } + void set_has_scoped_access(bool v) { _has_scoped_access = v; } // check the CompilerOracle for special behaviours for this compile bool method_has_option(CompileCommandEnum option) { diff --git a/src/hotspot/share/opto/output.cpp b/src/hotspot/share/opto/output.cpp index fd7d0eaf171..42374a0bcd4 100644 --- a/src/hotspot/share/opto/output.cpp +++ b/src/hotspot/share/opto/output.cpp @@ -3458,6 +3458,7 @@ void PhaseOutput::install_code(ciMethod* target, has_unsafe_access, SharedRuntime::is_wide_vector(C->max_vector_size()), C->has_monitors(), + C->has_scoped_access(), 0); if (C->log() != nullptr) { // Print code cache state into compiler log diff --git a/src/hotspot/share/opto/parse1.cpp b/src/hotspot/share/opto/parse1.cpp index 7e19336e735..9b194099183 100644 --- a/src/hotspot/share/opto/parse1.cpp +++ b/src/hotspot/share/opto/parse1.cpp @@ -437,6 +437,10 @@ Parse::Parse(JVMState* caller, ciMethod* parse_method, float expected_uses) C->set_has_monitors(true); } + if (parse_method->is_scoped()) { + C->set_has_scoped_access(true); + } + _iter.reset_to_method(method()); C->set_has_loops(C->has_loops() || method()->has_loops()); diff --git a/src/hotspot/share/prims/scopedMemoryAccess.cpp b/src/hotspot/share/prims/scopedMemoryAccess.cpp index 62546c4fe51..e23eb73c81c 100644 --- a/src/hotspot/share/prims/scopedMemoryAccess.cpp +++ b/src/hotspot/share/prims/scopedMemoryAccess.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -26,6 +26,7 @@ #include "classfile/vmSymbols.hpp" #include "jni.h" #include "jvm.h" +#include "logging/logStream.hpp" #include "oops/access.inline.hpp" #include "oops/oop.inline.hpp" #include "prims/stackwalk.hpp" @@ -35,35 +36,76 @@ #include "runtime/sharedRuntime.hpp" #include "runtime/vframe.inline.hpp" -static bool is_in_scoped_access(JavaThread* jt, oop session) { +template +static bool for_scoped_method(JavaThread* jt, const Func& func) { + ResourceMark rm; +#ifdef ASSERT + LogMessage(foreign) msg; + NonInterleavingLogStream ls{LogLevelType::Trace, msg}; + if (ls.is_enabled()) { + ls.print_cr("Walking thread: %s", jt->name()); + } +#endif + const int max_critical_stack_depth = 10; int depth = 0; for (vframeStream stream(jt); !stream.at_end(); stream.next()) { Method* m = stream.method(); - if (m->is_scoped()) { - StackValueCollection* locals = stream.asJavaVFrame()->locals(); - for (int i = 0; i < locals->size(); i++) { - StackValue* var = locals->at(i); - if (var->type() == T_OBJECT) { - if (var->get_obj() == session) { - assert(depth < max_critical_stack_depth, "can't have more than %d critical frames", max_critical_stack_depth); - return true; - } - } - } - break; + bool is_scoped = m->is_scoped(); + +#ifdef ASSERT + if (ls.is_enabled()) { + stream.asJavaVFrame()->print_value(&ls); + ls.print_cr(" is_scoped=%s", is_scoped ? "true" : "false"); + } +#endif + + if (is_scoped) { + assert(depth < max_critical_stack_depth, "can't have more than %d critical frames", max_critical_stack_depth); + return func(stream); } depth++; + #ifndef ASSERT + // On debug builds, just keep searching the stack + // in case we missed an @Scoped method further up if (depth >= max_critical_stack_depth) { break; } #endif } - return false; } +static bool is_accessing_session(JavaThread* jt, oop session, bool& in_scoped) { + return for_scoped_method(jt, [&](vframeStream& stream){ + in_scoped = true; + StackValueCollection* locals = stream.asJavaVFrame()->locals(); + for (int i = 0; i < locals->size(); i++) { + StackValue* var = locals->at(i); + if (var->type() == T_OBJECT) { + if (var->get_obj() == session) { + return true; + } + } + } + return false; + }); +} + +static frame get_last_frame(JavaThread* jt) { + frame last_frame = jt->last_frame(); + RegisterMap register_map(jt, + RegisterMap::UpdateMap::include, + RegisterMap::ProcessFrames::include, + RegisterMap::WalkContinuation::skip); + + if (last_frame.is_safepoint_blob_frame()) { + last_frame = last_frame.sender(®ister_map); + } + return last_frame; +} + class ScopedAsyncExceptionHandshake : public AsyncExceptionHandshake { OopHandle _session; @@ -78,8 +120,8 @@ public: virtual void do_thread(Thread* thread) { JavaThread* jt = JavaThread::cast(thread); - ResourceMark rm; - if (is_in_scoped_access(jt, _session.resolve())) { + bool ignored; + if (is_accessing_session(jt, _session.resolve(), ignored)) { // Throw exception to unwind out from the scoped access AsyncExceptionHandshake::do_thread(thread); } @@ -104,31 +146,14 @@ public: return; } - frame last_frame = jt->last_frame(); - RegisterMap register_map(jt, - RegisterMap::UpdateMap::include, - RegisterMap::ProcessFrames::include, - RegisterMap::WalkContinuation::skip); - - if (last_frame.is_safepoint_blob_frame()) { - last_frame = last_frame.sender(®ister_map); - } - - ResourceMark rm; - if (last_frame.is_compiled_frame() && last_frame.can_be_deoptimized()) { - // FIXME: we would like to conditionally deoptimize only if the corresponding - // _session is reachable from the frame, but reachabilityFence doesn't currently - // work the way it should. Therefore we deopt unconditionally for now. - Deoptimization::deoptimize(jt, last_frame); - } - if (jt->has_async_exception_condition()) { // Target thread just about to throw an async exception using async handshakes, // we will then unwind out from the scoped memory access. return; } - if (is_in_scoped_access(jt, JNIHandles::resolve(_session))) { + bool in_scoped = false; + if (is_accessing_session(jt, JNIHandles::resolve(_session), in_scoped)) { // We have found that the target thread is inside of a scoped access. // An asynchronous handshake is sent to the target thread, telling it // to throw an exception, which will unwind the target thread out from @@ -136,6 +161,47 @@ public: OopHandle session(Universe::vm_global(), JNIHandles::resolve(_session)); OopHandle error(Universe::vm_global(), JNIHandles::resolve(_error)); jt->install_async_exception(new ScopedAsyncExceptionHandshake(session, error)); + } else if (!in_scoped) { + frame last_frame = get_last_frame(jt); + if (last_frame.is_compiled_frame() && last_frame.can_be_deoptimized()) { + // We are not at a safepoint that is 'in' an @Scoped method, but due to the compiler + // moving code around/hoisting checks, we may be in a situation like this: + // + // liveness check (from @Scoped method) + // for (...) { + // for (...) { // strip-mining inner loop + // memory access (from @Scoped method) + // } + // safepoint <-- STOPPED HERE + // } + // + // The safepoint at which we're stopped may be in between the liveness check + // and actual memory access, but is itself 'outside' of @Scoped code + // + // However, we're not sure whether we are in this exact situation, and + // we're also not sure whether a memory access will actually occur after + // this safepoint. So, we can not just install an async exception here + // + // Instead, we mark the frame for deoptimization (which happens just before + // execution in this frame continues) to get back to code like this: + // + // for (...) { + // call to ScopedMemoryAccess + // safepoint <-- STOPPED HERE + // } + // + // This means that we will re-do the liveness check before attempting + // another memory access. If the scope has been closed at that point, + // the target thread will see it and throw an exception. + + nmethod* code = last_frame.cb()->as_nmethod(); + if (code->has_scoped_access()) { + // We would like to deoptimize here only if last_frame::oops_do + // reports the session oop being live at this safepoint, but this + // currently isn't possible due to JDK-8290892 + Deoptimization::deoptimize(jt, last_frame); + } + } } } }; diff --git a/src/hotspot/share/runtime/vframe.cpp b/src/hotspot/share/runtime/vframe.cpp index be2e275bf7f..ebd5af5b74e 100644 --- a/src/hotspot/share/runtime/vframe.cpp +++ b/src/hotspot/share/runtime/vframe.cpp @@ -618,94 +618,94 @@ javaVFrame* vframeStreamCommon::asJavaVFrame() { } #ifndef PRODUCT -void vframe::print() { - if (WizardMode) _fr.print_value_on(tty,nullptr); +void vframe::print(outputStream* output) { + if (WizardMode) _fr.print_value_on(output, nullptr); } -void vframe::print_value() const { - ((vframe*)this)->print(); +void vframe::print_value(outputStream* output) const { + ((vframe*)this)->print(output); } -void entryVFrame::print_value() const { - ((entryVFrame*)this)->print(); +void entryVFrame::print_value(outputStream* output) const { + ((entryVFrame*)this)->print(output); } -void entryVFrame::print() { - vframe::print(); - tty->print_cr("C Chunk in between Java"); - tty->print_cr("C link " INTPTR_FORMAT, p2i(_fr.link())); +void entryVFrame::print(outputStream* output) { + vframe::print(output); + output->print_cr("C Chunk in between Java"); + output->print_cr("C link " INTPTR_FORMAT, p2i(_fr.link())); } // ------------- javaVFrame -------------- -static void print_stack_values(const char* title, StackValueCollection* values) { +static void print_stack_values(outputStream* output, const char* title, StackValueCollection* values) { if (values->is_empty()) return; - tty->print_cr("\t%s:", title); + output->print_cr("\t%s:", title); values->print(); } -void javaVFrame::print() { +void javaVFrame::print(outputStream* output) { Thread* current_thread = Thread::current(); ResourceMark rm(current_thread); HandleMark hm(current_thread); - vframe::print(); - tty->print("\t"); + vframe::print(output); + output->print("\t"); method()->print_value(); - tty->cr(); - tty->print_cr("\tbci: %d", bci()); + output->cr(); + output->print_cr("\tbci: %d", bci()); - print_stack_values("locals", locals()); - print_stack_values("expressions", expressions()); + print_stack_values(output, "locals", locals()); + print_stack_values(output, "expressions", expressions()); GrowableArray* list = monitors(); if (list->is_empty()) return; - tty->print_cr("\tmonitor list:"); + output->print_cr("\tmonitor list:"); for (int index = (list->length()-1); index >= 0; index--) { MonitorInfo* monitor = list->at(index); - tty->print("\t obj\t"); + output->print("\t obj\t"); if (monitor->owner_is_scalar_replaced()) { Klass* k = java_lang_Class::as_Klass(monitor->owner_klass()); - tty->print("( is scalar replaced %s)", k->external_name()); + output->print("( is scalar replaced %s)", k->external_name()); } else if (monitor->owner() == nullptr) { - tty->print("( null )"); + output->print("( null )"); } else { monitor->owner()->print_value(); - tty->print("(owner=" INTPTR_FORMAT ")", p2i(monitor->owner())); + output->print("(owner=" INTPTR_FORMAT ")", p2i(monitor->owner())); } if (monitor->eliminated()) { if(is_compiled_frame()) { - tty->print(" ( lock is eliminated in compiled frame )"); + output->print(" ( lock is eliminated in compiled frame )"); } else { - tty->print(" ( lock is eliminated, frame not compiled )"); + output->print(" ( lock is eliminated, frame not compiled )"); } } - tty->cr(); - tty->print("\t "); - monitor->lock()->print_on(tty, monitor->owner()); - tty->cr(); + output->cr(); + output->print("\t "); + monitor->lock()->print_on(output, monitor->owner()); + output->cr(); } } -void javaVFrame::print_value() const { +void javaVFrame::print_value(outputStream* output) const { Method* m = method(); InstanceKlass* k = m->method_holder(); - tty->print_cr("frame( sp=" INTPTR_FORMAT ", unextended_sp=" INTPTR_FORMAT ", fp=" INTPTR_FORMAT ", pc=" INTPTR_FORMAT ")", + output->print_cr("frame( sp=" INTPTR_FORMAT ", unextended_sp=" INTPTR_FORMAT ", fp=" INTPTR_FORMAT ", pc=" INTPTR_FORMAT ")", p2i(_fr.sp()), p2i(_fr.unextended_sp()), p2i(_fr.fp()), p2i(_fr.pc())); - tty->print("%s.%s", k->internal_name(), m->name()->as_C_string()); + output->print("%s.%s", k->internal_name(), m->name()->as_C_string()); if (!m->is_native()) { Symbol* source_name = k->source_file_name(); int line_number = m->line_number_from_bci(bci()); if (source_name != nullptr && (line_number != -1)) { - tty->print("(%s:%d)", source_name->as_C_string(), line_number); + output->print("(%s:%d)", source_name->as_C_string(), line_number); } } else { - tty->print("(Native Method)"); + output->print("(Native Method)"); } // Check frame size and print warning if it looks suspiciously large if (fr().sp() != nullptr) { @@ -719,25 +719,25 @@ void javaVFrame::print_value() const { } } -void javaVFrame::print_activation(int index) const { +void javaVFrame::print_activation(int index, outputStream* output) const { // frame number and method - tty->print("%2d - ", index); + output->print("%2d - ", index); ((vframe*)this)->print_value(); - tty->cr(); + output->cr(); if (WizardMode) { ((vframe*)this)->print(); - tty->cr(); + output->cr(); } } // ------------- externalVFrame -------------- -void externalVFrame::print() { - _fr.print_value_on(tty,nullptr); +void externalVFrame::print(outputStream* output) { + _fr.print_value_on(output, nullptr); } -void externalVFrame::print_value() const { - ((vframe*)this)->print(); +void externalVFrame::print_value(outputStream* output) const { + ((vframe*)this)->print(output); } #endif // PRODUCT diff --git a/src/hotspot/share/runtime/vframe.hpp b/src/hotspot/share/runtime/vframe.hpp index 427fea2f78e..c2a777c1556 100644 --- a/src/hotspot/share/runtime/vframe.hpp +++ b/src/hotspot/share/runtime/vframe.hpp @@ -99,8 +99,8 @@ class vframe: public ResourceObj { #ifndef PRODUCT // printing operations - virtual void print_value() const; - virtual void print(); + virtual void print_value(outputStream* output = tty) const; + virtual void print(outputStream* output = tty); #endif }; @@ -145,9 +145,9 @@ class javaVFrame: public vframe { #ifndef PRODUCT public: // printing operations - void print(); - void print_value() const; - void print_activation(int index) const; + void print(outputStream* output = tty); + void print_value(outputStream* output = tty) const; + void print_activation(int index, outputStream* output = tty) const; #endif friend class vframe; }; @@ -195,8 +195,8 @@ class externalVFrame: public vframe { #ifndef PRODUCT public: // printing operations - void print_value() const; - void print(); + void print_value(outputStream* output = tty) const; + void print(outputStream* output = tty); #endif friend class vframe; }; @@ -211,8 +211,8 @@ class entryVFrame: public externalVFrame { #ifndef PRODUCT public: // printing - void print_value() const; - void print(); + void print_value(outputStream* output = tty) const; + void print(outputStream* output = tty); #endif friend class vframe; }; diff --git a/test/jdk/java/foreign/TestConcurrentClose.java b/test/jdk/java/foreign/TestConcurrentClose.java new file mode 100644 index 00000000000..7f39ef39040 --- /dev/null +++ b/test/jdk/java/foreign/TestConcurrentClose.java @@ -0,0 +1,188 @@ +/* + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @requires vm.compiler2.enabled + * @modules java.base/jdk.internal.vm.annotation java.base/jdk.internal.misc + * @key randomness + * @library /test/lib + * + * @build jdk.test.whitebox.WhiteBox + * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox + * + * @run testng/othervm + * -Xbootclasspath/a:. + * -XX:+UnlockDiagnosticVMOptions + * -XX:+WhiteBoxAPI + * -XX:CompileCommand=dontinline,TestConcurrentClose$SegmentAccessor::doAccess + * TestConcurrentClose + */ + +import jdk.test.whitebox.WhiteBox; +import org.testng.annotations.Test; + +import java.lang.foreign.Arena; +import java.lang.foreign.MemorySegment; +import java.lang.reflect.Method; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; + +import static java.lang.foreign.ValueLayout.JAVA_BYTE; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +public class TestConcurrentClose { + static final WhiteBox WB = WhiteBox.getWhiteBox(); + static final Method DO_ACCESS_METHOD; + static final int C2_COMPILED_LEVEL = 4; + + static { + try { + DO_ACCESS_METHOD = SegmentAccessor.class.getDeclaredMethod("doAccess"); + } catch (ReflectiveOperationException e) { + throw new ExceptionInInitializerError(e); + } + } + + static final int ITERATIONS = 5; + static final int SEGMENT_SIZE = 10_000; + static final int MAX_EXECUTOR_WAIT_SECONDS = 20; + static final int NUM_ACCESSORS = 50; + + static final AtomicLong start = new AtomicLong(); + static final AtomicBoolean started = new AtomicBoolean(); + + @Test + public void testHandshake() throws InterruptedException { + for (int it = 0; it < ITERATIONS; it++) { + System.out.println("ITERATION " + it + " - starting"); + ExecutorService accessExecutor = Executors.newCachedThreadPool(); + start.set(System.currentTimeMillis()); + started.set(false); + CountDownLatch startClosureLatch = new CountDownLatch(1); + + for (int i = 0; i < NUM_ACCESSORS ; i++) { + Arena arena = Arena.ofShared(); + MemorySegment segment = arena.allocate(SEGMENT_SIZE, 1); + accessExecutor.execute(new SegmentAccessor(i, segment)); + accessExecutor.execute(new Closer(i, startClosureLatch, arena)); + } + + awaitCompilation(); + + long closeDelay = System.currentTimeMillis() - start.get(); + System.out.println("Starting closers after delay of " + closeDelay + " millis"); + startClosureLatch.countDown(); + accessExecutor.shutdown(); + assertTrue(accessExecutor.awaitTermination(MAX_EXECUTOR_WAIT_SECONDS, TimeUnit.SECONDS)); + long finishDelay = System.currentTimeMillis() - start.get(); + System.out.println("ITERATION " + it + " - finished, after " + finishDelay + "milis"); + } + } + + static class SegmentAccessor implements Runnable { + final MemorySegment segment; + final int id; + boolean hasFailed = false; + + SegmentAccessor(int id, MemorySegment segment) { + this.id = id; + this.segment = segment; + } + + @Override + public final void run() { + start("Accessor #" + id); + while (segment.scope().isAlive()) { + try { + doAccess(); + } catch (IllegalStateException ex) { + // scope was closed, loop should exit + assertFalse(hasFailed); + hasFailed = true; + } + } + long delay = System.currentTimeMillis() - start.get(); + System.out.println("Accessor #" + id + " terminated - elapsed (ms): " + delay); + } + + // keep this out of line, so it has a chance to be fully C2 compiled + private int doAccess() { + int sum = 0; + for (int i = 0; i < segment.byteSize(); i++) { + sum += segment.get(JAVA_BYTE, i); + } + return sum; + } + } + + static class Closer implements Runnable { + final int id; + final Arena arena; + final CountDownLatch startLatch; + + Closer(int id, CountDownLatch startLatch, Arena arena) { + this.id = id; + this.arena = arena; + this.startLatch = startLatch; + } + + @Override + public void run() { + start("Closer #" + id); + try { + // try to close all at the same time, to simulate concurrent + // closures of unrelated arenas + startLatch.await(); + } catch (InterruptedException e) { + throw new RuntimeException("Unexpected interruption", e); + } + arena.close(); // This should NOT throw + long delay = System.currentTimeMillis() - start.get(); + System.out.println("Closer #" + id + "terminated - elapsed (ms): " + delay); + } + } + + static void start(String name) { + if (started.compareAndSet(false, true)) { + long delay = System.currentTimeMillis() - start.get(); + System.out.println("Started first thread: " + name + " ; elapsed (ms): " + delay); + } + } + + private static void awaitCompilation() throws InterruptedException { + int retries = 0; + while (WB.getMethodCompilationLevel(DO_ACCESS_METHOD, false) != C2_COMPILED_LEVEL) { + if (retries > 20) { + throw new IllegalStateException("SegmentAccessor::doAccess method not being compiled"); + } + Thread.sleep(1000); + retries++; + } + } +} diff --git a/test/jdk/java/foreign/TestHandshake.java b/test/jdk/java/foreign/TestHandshake.java index efa175eb5c4..ff85c3482a2 100644 --- a/test/jdk/java/foreign/TestHandshake.java +++ b/test/jdk/java/foreign/TestHandshake.java @@ -100,7 +100,7 @@ public class TestHandshake { @Override public final void run() { - start("\"Accessor #\" + id"); + start("Accessor #" + id); while (segment.scope().isAlive()) { try { doAccess(); diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/ConcurrentClose.java b/test/micro/org/openjdk/bench/java/lang/foreign/ConcurrentClose.java new file mode 100644 index 00000000000..5a29e1c4ed3 --- /dev/null +++ b/test/micro/org/openjdk/bench/java/lang/foreign/ConcurrentClose.java @@ -0,0 +1,85 @@ +/* + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package org.openjdk.bench.java.lang.foreign; + +import java.lang.foreign.Arena; +import java.lang.foreign.MemorySegment; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; +import java.util.concurrent.TimeUnit; + +import org.openjdk.jmh.annotations.*; + +import static java.lang.foreign.ValueLayout.*; + +@BenchmarkMode(Mode.AverageTime) +@Warmup(iterations = 5, time = 10, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 10, time = 10, timeUnit = TimeUnit.SECONDS) +@State(Scope.Thread) +@OutputTimeUnit(TimeUnit.MICROSECONDS) +@Fork(1) +public class ConcurrentClose { + + static final int SIZE = 10_000; + static final VarHandle BYTES = MethodHandles.arrayElementVarHandle(byte[].class); + + MemorySegment segment; + byte[] array; + + @Setup + public void setup() { + segment = Arena.global().allocate(SIZE); + array = new byte[SIZE]; + } + + @Benchmark + @GroupThreads(1) + @Group("sharedClose") + public void closing() { + Arena arena = Arena.ofShared(); + arena.close(); + } + + @Benchmark + @GroupThreads(1) + @Group("sharedClose") + public int memorySegmentAccess() { + int sum = 0; + for (long i = 0; i < segment.byteSize(); i++) { + sum += segment.get(JAVA_BYTE, i); + } + return sum; + } + + @Benchmark + @GroupThreads(1) + @Group("sharedClose") + public int otherAccess() { + int sum = 0; + for (int i = 0; i < array.length; i++) { + sum += (byte) BYTES.get(array, i); + } + return sum; + } +}