Re: RFR: 8252999: Cleanup: replace .equals("") with .isEmpty() within all codebase
On Fri, 11 Sep 2020 15:17:58 GMT, Bradford Wetmore wrote: >> Ok, sorry for the distraction. > > Our local Santuario maintainer says: > > In general, changes to Apache Santuario should also be made at Apache so we > stay in sync. Hi @doom369, I hope we didn't end up wasting too much of your time with this. I wanted to respond to a comment you made earlier in this PR, > I have in mind dozens of improvements all over the code like this one. It's hard to see, but as you discovered, the JDK has different groups of people maintaining different areas, and sometimes there are hidden constraints on those different areas, for example, to avoid divergence with upstream source bases. And as you discovered, sometimes those source bases might need to maintain compatibility with an older JDK ... so we don't want to update this code even though it's IN the JDK. Those kind of constraints generally don't apply to code in the java.base module, since there's nothing upstream of it, and by definition it cannot depend on anything outside the JDK. Generally -- though there are exceptions -- we're more receptive to keeping stuff in java.base (and sometimes related modules close to the core) on the latest and greatest stuff. There are some constraints, however. There are some things we can't use too early during initialization of the JDK, such as lambdas. Also, there is some code lurking around that is sometimes executed by the boot JDK, which is typically one release behind. (This is definitely the case for tools like javac, but I think it might also apply to some things in base.) Anyway, if you'd like to pursue some of these improvements, drop a note to core-libs-dev@openjdk and we can talk about it. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/29
Re: RFR: 8241390: 'Deadlock' with VM_RedefineClasses::lock_classes() [v3]
> The change fixes a 'deadlock' situation in VM_RedefineClasses::lock_classes() > when the current thread is in the middle > of redefining the same class. The change is based on the fix [1] suggested in > the Jira issue [2] comment. > [1] http://cr.openjdk.java.net/~jiangli/8241390/webrev.00/ > [2] https://bugs.openjdk.java.net/browse/JDK-8241390 > > Testing: :jdk_instrument, tier1-tier3, and tier5 tests pass. Daniil Titov has updated the pull request incrementally with one additional commit since the last revision: Corrected test summary - Changes: - all: https://git.openjdk.java.net/jdk/pull/190/files - new: https://git.openjdk.java.net/jdk/pull/190/files/3502f018..a2609929 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=190&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=190&range=01-02 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/190.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/190/head:pull/190 PR: https://git.openjdk.java.net/jdk/pull/190
Re: RFR: 8241390: 'Deadlock' with VM_RedefineClasses::lock_classes() [v2]
On Thu, 17 Sep 2020 20:57:24 GMT, Daniil Titov wrote: >> The change fixes a 'deadlock' situation in >> VM_RedefineClasses::lock_classes() when the current thread is in the middle >> of redefining the same class. The change is based on the fix [1] suggested >> in the Jira issue [2] comment. >> [1] http://cr.openjdk.java.net/~jiangli/8241390/webrev.00/ >> [2] https://bugs.openjdk.java.net/browse/JDK-8241390 >> >> Testing: :jdk_instrument, tier1-tier3, and tier5 tests pass. > > Daniil Titov has updated the pull request incrementally with one additional > commit since the last revision: > > Changed test to not use shell scripts > > Additional changes: >- Replaced 'cls' identifier with 'redef_classes' >- Added a comment that the same class can be pushed to the list multiple > times. > > Testing: tier1-tier3 tests pass. The following test @summary needs some tweaks: * @summary Test recursively redefines the same class. The test hangs if * a deadlock happens. Please, replace: redefines => retransforms. Also, it'd be nice to align "a deadlock happens" with the "Test recursively ...". - Changes requested by sspitsyn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/190
Integrated: 8253028: SA core file tests still time out on OSX with "java.io.IOException: App waiting timeout"
On Tue, 15 Sep 2020 18:53:30 GMT, Chris Plummer wrote: > Some SA core file testing requires longer timeouts since core files can on > occasion take a very long time to produce on > OSX. > I'd like to treat this as a trival change. This pull request has now been integrated. Changeset: d4269fd5 Author:Chris Plummer URL: https://git.openjdk.java.net/jdk/commit/d4269fd5 Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod 8253028: SA core file tests still time out on OSX with "java.io.IOException: App waiting timeout" Reviewed-by: amenkov - PR: https://git.openjdk.java.net/jdk/pull/192
Re: RFR: 8241390: 'Deadlock' with VM_RedefineClasses::lock_classes() [v2]
> The change fixes a 'deadlock' situation in VM_RedefineClasses::lock_classes() > when the current thread is in the middle > of redefining the same class. The change is based on the fix [1] suggested in > the Jira issue [2] comment. > [1] http://cr.openjdk.java.net/~jiangli/8241390/webrev.00/ > [2] https://bugs.openjdk.java.net/browse/JDK-8241390 > > Testing: :jdk_instrument, tier1-tier3, and tier5 tests pass. Daniil Titov has updated the pull request incrementally with one additional commit since the last revision: Changed test to not use shell scripts Additional changes: - Replaced 'cls' identifier with 'redef_classes' - Added a comment that the same class can be pushed to the list multiple times. Testing: tier1-tier3 tests pass. - Changes: - all: https://git.openjdk.java.net/jdk/pull/190/files - new: https://git.openjdk.java.net/jdk/pull/190/files/6e6b1b81..3502f018 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=190&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=190&range=00-01 Stats: 275 lines in 6 files changed: 119 ins; 142 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/190.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/190/head:pull/190 PR: https://git.openjdk.java.net/jdk/pull/190
Re: RFR: 8238761: Asynchronous handshakes [v2]
On Thu, 17 Sep 2020 19:23:09 GMT, Robbin Ehn wrote:
>> src/hotspot/share/runtime/handshake.cpp line 508:
>>
>>> 506: assert(op->_target == NULL || _handshakee == op->_target, "Wrong
>>> thread");
>>> 507: log_trace(handshake)("Processing handshake " INTPTR_FORMAT " by
>>> %s", p2i(op),
>>> 508:executioner_name(current_thread,
>>> _handshakee, op == match_op));
>>
>> With the above change we could even avoid factoring the code into
>> executioner_name() and just do:
>> log_trace(handshake)("Processing handshake " INTPTR_FORMAT " by %s%s",
>> p2i(op),
>> op == match_op ? "handshaker" :
>> "cooperative",
>> current_thread->is_VM_thread() ?
>> "(VM Thread)" : "(JavaThread)");
>
> I added a second log line where I use that function also:
> log_trace(handshake)("Thread %s(" INTPTR_FORMAT ") executed %d ops for
> JavaThread: " INTPTR_FORMAT " %s target op: "
> INTPTR_FORMAT,
>executioner_name(current_thread, _handshakee, pr_ret
> == HandshakeState::_succeed),
>p2i(current_thread), executed, p2i(_handshakee),
>pr_ret == HandshakeState::_succeed ? "including" :
> "excluding", p2i(match_op));
Push commit 469f8fc, please have a look.
-
PR: https://git.openjdk.java.net/jdk/pull/151
Re: RFR: 8238761: Asynchronous handshakes [v3]
> This patch implements asynchronous handshake, which changes how handshakes > works by default. Asynchronous handshakes > are target only executed, which they may never be executed. (target may block > on socket for the rest of VM lifetime) > Since we have several use-cases for them we can have many handshake pending. > (should be very rare) To be able handle an > arbitrary amount of handshakes this patch adds a per JavaThread queue and > heap allocated HandshakeOperations. It's a > singly linked list where you push/insert to the end and pop/get from the > front. Inserts are done via CAS on first > pointer, no lock needed. Pops are done while holding the per handshake state > lock, and when working on the first > pointer also CAS. The thread grabbing the handshake state lock for a > JavaThread will pop and execute all handshake > operations matching the filter. The JavaThread itself uses no filter and any > other thread uses the filter of everything > except asynchronous handshakes. In this initial change-set there is no need > to do any other filtering. If needed > filtering can easily be exposed as a virtual method on the HandshakeClosure, > but note that filtering causes handshake > operation to be done out-order. Since the filter determins who execute the > operation and not the invoked method, there > is now only one method to call when handshaking one thread. Some comments > about the changes: > - HandshakeClosure uses ThreadClosure, since it neat to use the same closure > for both alla JavThreads do and Handshake > all threads. With heap allocating it cannot extends StackObj. I tested > several ways to fix this, but those very much > worse then this. > > - I added a is_handshake_safe_for for checking if it's current thread is > operating on itself or the handshaker of that > thread. > > - Simplified JVM TI with a JvmtiHandshakeClosure and also made them not > needing a JavaThread when executing as a > handshaker on a JavaThread, e.g. VM Thread can execute the handshake > operation. > > - Added WB testing method. > > - Removed VM_HandshakeOneThread, the VM thread uses the same call path as > direct handshakes did. > > - Changed the handshake semaphores to mutex to be able to handle deadlocks > with lock ranking. > > - VM_HandshakeAllThreadsis still a VM operation, since we do support half of > the threads being handshaked before a > safepoint and half of them after, in many handshake all operations. > > - ThreadInVMForHandshake do not need to do a fenced transistion since this is > always a transistion from unsafe to unsafe. > > - Added NoSafepointVerifyer, we are thinking about supporting safepoints > inside handshake, but it's not needed at the > moment. To make sure that gets well tested if added the NoSafepointVerifyer > will raise eyebrows. > > - Added ttyLocker::break_tty_lock_for_safepoint(os::current_thread_id()); due > to lock rank. > > - Added filtered queue and gtest for it. > > Passes multiple t1-8 runs. > Been through some pre-reviwing. Robbin Ehn has updated the pull request incrementally with one additional commit since the last revision: Removed double check, fix comment, removed not needed function, updated logs - Changes: - all: https://git.openjdk.java.net/jdk/pull/151/files - new: https://git.openjdk.java.net/jdk/pull/151/files/86b83d05..469f8fc8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=151&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=151&range=01-02 Stats: 22 lines in 2 files changed: 1 ins; 12 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/151.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/151/head:pull/151 PR: https://git.openjdk.java.net/jdk/pull/151
Re: RFR: 8238761: Asynchronous handshakes [v2]
On Thu, 17 Sep 2020 18:28:20 GMT, Patricio Chilano Mateo
wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixed double checks
>> Added NSV
>> ProcessResult to enum
>> Fixed logging
>> Moved _active_handshaker to private
>
> src/hotspot/share/runtime/handshake.cpp line 464:
>
>> 462:
>> 463: const char* executioner_name(Thread* current_thread, Thread*
>> handshakee, bool current_is_requester) {
>> 464: if (current_thread == handshakee) return "self(JavaThread)";
>
> I think we can remove this line since executioner_name() is only called by
> the handshaker.
Yes, fixing.
> src/hotspot/share/runtime/handshake.cpp line 508:
>
>> 506: assert(op->_target == NULL || _handshakee == op->_target, "Wrong
>> thread");
>> 507: log_trace(handshake)("Processing handshake " INTPTR_FORMAT " by
>> %s", p2i(op),
>> 508:executioner_name(current_thread,
>> _handshakee, op == match_op));
>
> With the above change we could even avoid factoring the code into
> executioner_name() and just do:
> log_trace(handshake)("Processing handshake " INTPTR_FORMAT " by %s%s",
> p2i(op),
> op == match_op ? "handshaker" :
> "cooperative",
> current_thread->is_VM_thread() ?
> "(VM Thread)" : "(JavaThread)");
I added a second log line where I use that function also:
log_trace(handshake)("Thread %s(" INTPTR_FORMAT ") executed %d ops for
JavaThread: " INTPTR_FORMAT " %s target op: "
INTPTR_FORMAT,
executioner_name(current_thread, _handshakee, pr_ret ==
HandshakeState::_succeed),
p2i(current_thread), executed, p2i(_handshakee),
pr_ret == HandshakeState::_succeed ? "including" :
"excluding", p2i(match_op));
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 908:
>
>> 906: #endif
>> 907: Thread* current_thread = Thread::current();
>> 908: assert(current_thread == java_thread ||
>
> One extra check here.
Fixing
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1164:
>
>> 1162: #ifdef ASSERT
>> 1163: Thread *current_thread = Thread::current();
>> 1164: assert(current_thread == thr ||
>
> One extra check here.
Fixing
-
PR: https://git.openjdk.java.net/jdk/pull/151
Re: RFR: 8238761: Asynchronous handshakes [v2]
On Thu, 17 Sep 2020 12:07:15 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM lifetime)
>> Since we have several use-cases for them we can have many handshake pending.
>> (should be very rare) To be able handle an
>> arbitrary amount of handshakes this patch adds a per JavaThread queue and
>> heap allocated HandshakeOperations. It's a
>> singly linked list where you push/insert to the end and pop/get from the
>> front. Inserts are done via CAS on first
>> pointer, no lock needed. Pops are done while holding the per handshake state
>> lock, and when working on the first
>> pointer also CAS. The thread grabbing the handshake state lock for a
>> JavaThread will pop and execute all handshake
>> operations matching the filter. The JavaThread itself uses no filter and any
>> other thread uses the filter of everything
>> except asynchronous handshakes. In this initial change-set there is no need
>> to do any other filtering. If needed
>> filtering can easily be exposed as a virtual method on the HandshakeClosure,
>> but note that filtering causes handshake
>> operation to be done out-order. Since the filter determins who execute the
>> operation and not the invoked method, there
>> is now only one method to call when handshaking one thread. Some comments
>> about the changes:
>> - HandshakeClosure uses ThreadClosure, since it neat to use the same closure
>> for both alla JavThreads do and Handshake
>> all threads. With heap allocating it cannot extends StackObj. I tested
>> several ways to fix this, but those very much
>> worse then this.
>>
>> - I added a is_handshake_safe_for for checking if it's current thread is
>> operating on itself or the handshaker of that
>> thread.
>>
>> - Simplified JVM TI with a JvmtiHandshakeClosure and also made them not
>> needing a JavaThread when executing as a
>> handshaker on a JavaThread, e.g. VM Thread can execute the handshake
>> operation.
>>
>> - Added WB testing method.
>>
>> - Removed VM_HandshakeOneThread, the VM thread uses the same call path as
>> direct handshakes did.
>>
>> - Changed the handshake semaphores to mutex to be able to handle deadlocks
>> with lock ranking.
>>
>> - VM_HandshakeAllThreadsis still a VM operation, since we do support half of
>> the threads being handshaked before a
>> safepoint and half of them after, in many handshake all operations.
>>
>> - ThreadInVMForHandshake do not need to do a fenced transistion since this
>> is always a transistion from unsafe to unsafe.
>>
>> - Added NoSafepointVerifyer, we are thinking about supporting safepoints
>> inside handshake, but it's not needed at the
>> moment. To make sure that gets well tested if added the
>> NoSafepointVerifyer will raise eyebrows.
>>
>> - Added ttyLocker::break_tty_lock_for_safepoint(os::current_thread_id());
>> due to lock rank.
>>
>> - Added filtered queue and gtest for it.
>>
>> Passes multiple t1-8 runs.
>> Been through some pre-reviwing.
>
> Robbin Ehn has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixed double checks
> Added NSV
> ProcessResult to enum
> Fixed logging
> Moved _active_handshaker to private
Changes look good, thanks for fixing! I added some comments on the changes.
src/hotspot/share/runtime/handshake.cpp line 464:
> 462:
> 463: const char* executioner_name(Thread* current_thread, Thread* handshakee,
> bool current_is_requester) {
> 464: if (current_thread == handshakee) return "self(JavaThread)";
I think we can remove this line since executioner_name() is only called by the
handshaker.
src/hotspot/share/runtime/handshake.cpp line 508:
> 506: assert(op->_target == NULL || _handshakee == op->_target, "Wrong
> thread");
> 507: log_trace(handshake)("Processing handshake " INTPTR_FORMAT " by
> %s", p2i(op),
> 508:executioner_name(current_thread, _handshakee,
> op == match_op));
With the above change we could even avoid factoring the code into
executioner_name() and just do:
log_trace(handshake)("Processing handshake " INTPTR_FORMAT " by %s%s", p2i(op),
op == match_op ? "handshaker" :
"cooperative",
current_thread->is_VM_thread() ? "(VM
Thread)" : "(JavaThread)");
src/hotspot/share/prims/jvmtiEnvBase.cpp line 908:
> 906: #endif
> 907: Thread* current_thread = Thread::current();
> 908: assert(current_thread == java_thread ||
One extra check here.
src/hotspot/share/prims/jvmtiEnvBase.cpp line 1164:
> 1162: #ifdef ASSERT
> 1163: Thread *current_thread = Thread::current();
> 1164: assert(current_thread == thr ||
One extra check here.
-
PR: https://git.openjdk.java.net/jdk/pull/151
Integrated: 8252593: [TESTBUG] serviceability/jvmti/GetLocalVariable/GetLocalWithoutSuspendTest.java failed with JVMTI_ERROR_INVALID_SLOT
On Sat, 12 Sep 2020 23:19:46 GMT, Richard Reingruber wrote: > Continuing review [1] after transition to Git/Github. > > I still cannot reproduce the issue. > > RFC on alternatives: > > 1. Integrate this change and ignore future JVMTI_ERROR_INVALID_SLOT > 2. Don't ignore JVMTI_ERROR_INVALID_SLOT but integrate the rest of this > patch. If the test still fails with > JVMTI_ERROR_INVALID_SLOT we will at least know the depth of the frame. 3. Add > trace code to VM_GetOrSetLocal in > !PRODUCT or ASSERT configurations depending on an option or property. > Any other ideas? > > I'm in favour of 1. > > [1] > http://mail.openjdk.java.net/pipermail/serviceability-dev/2020-September/032876.html This pull request has now been integrated. Changeset: a4c6a99e Author:Richard Reingruber URL: https://git.openjdk.java.net/jdk/commit/a4c6a99e Stats: 7 lines in 1 file changed: 0 ins; 4 del; 3 mod 8252593: [TESTBUG] serviceability/jvmti/GetLocalVariable/GetLocalWithoutSuspendTest.java failed with JVMTI_ERROR_INVALID_SLOT Reviewed-by: sspitsyn, cjplummer - PR: https://git.openjdk.java.net/jdk/pull/142
Re: RFR: 8238761: Asynchronous handshakes
On Mon, 14 Sep 2020 13:00:59 GMT, Robbin Ehn wrote: > This patch implements asynchronous handshake, which changes how handshakes > works by default. Asynchronous handshakes > are target only executed, which they may never be executed. (target may block > on socket for the rest of VM lifetime) > Since we have several use-cases for them we can have many handshake pending. > (should be very rare) To be able handle an > arbitrary amount of handshakes this patch adds a per JavaThread queue and > heap allocated HandshakeOperations. It's a > singly linked list where you push/insert to the end and pop/get from the > front. Inserts are done via CAS on first > pointer, no lock needed. Pops are done while holding the per handshake state > lock, and when working on the first > pointer also CAS. The thread grabbing the handshake state lock for a > JavaThread will pop and execute all handshake > operations matching the filter. The JavaThread itself uses no filter and any > other thread uses the filter of everything > except asynchronous handshakes. In this initial change-set there is no need > to do any other filtering. If needed > filtering can easily be exposed as a virtual method on the HandshakeClosure, > but note that filtering causes handshake > operation to be done out-order. Since the filter determins who execute the > operation and not the invoked method, there > is now only one method to call when handshaking one thread. Some comments > about the changes: > - HandshakeClosure uses ThreadClosure, since it neat to use the same closure > for both alla JavThreads do and Handshake > all threads. With heap allocating it cannot extends StackObj. I tested > several ways to fix this, but those very much > worse then this. > > - I added a is_handshake_safe_for for checking if it's current thread is > operating on itself or the handshaker of that > thread. > > - Simplified JVM TI with a JvmtiHandshakeClosure and also made them not > needing a JavaThread when executing as a > handshaker on a JavaThread, e.g. VM Thread can execute the handshake > operation. > > - Added WB testing method. > > - Removed VM_HandshakeOneThread, the VM thread uses the same call path as > direct handshakes did. > > - Changed the handshake semaphores to mutex to be able to handle deadlocks > with lock ranking. > > - VM_HandshakeAllThreadsis still a VM operation, since we do support half of > the threads being handshaked before a > safepoint and half of them after, in many handshake all operations. > > - ThreadInVMForHandshake do not need to do a fenced transistion since this is > always a transistion from unsafe to unsafe. > > - Added NoSafepointVerifyer, we are thinking about supporting safepoints > inside handshake, but it's not needed at the > moment. To make sure that gets well tested if added the NoSafepointVerifyer > will raise eyebrows. > > - Added ttyLocker::break_tty_lock_for_safepoint(os::current_thread_id()); due > to the NoSafepointVerifyer. > > - Added filtered queue and gtest for it. > > Passes multiple t1-8 runs. > Been through some pre-reviwing. > src/hotspot/share/prims/jvmtiThreadState.cpp > src/hotspot/share/prims/jvmtiEnvBase.cpp Removed double checks. > src/hotspot/share/runtime/handshake.cpp > ... > I would just leave ProcessResult as an enum and log as before. Reverted to plain enum and updated logs. (better?) > src/hotspot/share/runtime/handshake.cpp > 387: NoSafepointVerifier nsv; > 388: process_self_inner(); I wanted a NSV to cover the process_self_inner method. So I added a second one in suggested place: > NoSafepointVerifier nsv; > _handshake_cl->do_thread(thread); > } > src/hotspot/share/runtime/interfaceSupport.inline.hpp > 156: // Threads shouldn't block if they are in the middle of > printing, but... > 157: ttyLocker::break_tty_lock_for_safepoint(os::current_thread_id()); > What's the issue of having NoSafepointVerifier inside the handshake? Sorry, the issue is the lock rank. Right now the semaphore hides this issue. Please see commit 86b83d0. - PR: https://git.openjdk.java.net/jdk/pull/151
Re: RFR: 8238761: Asynchronous handshakes [v2]
> This patch implements asynchronous handshake, which changes how handshakes > works by default. Asynchronous handshakes > are target only executed, which they may never be executed. (target may block > on socket for the rest of VM lifetime) > Since we have several use-cases for them we can have many handshake pending. > (should be very rare) To be able handle an > arbitrary amount of handshakes this patch adds a per JavaThread queue and > heap allocated HandshakeOperations. It's a > singly linked list where you push/insert to the end and pop/get from the > front. Inserts are done via CAS on first > pointer, no lock needed. Pops are done while holding the per handshake state > lock, and when working on the first > pointer also CAS. The thread grabbing the handshake state lock for a > JavaThread will pop and execute all handshake > operations matching the filter. The JavaThread itself uses no filter and any > other thread uses the filter of everything > except asynchronous handshakes. In this initial change-set there is no need > to do any other filtering. If needed > filtering can easily be exposed as a virtual method on the HandshakeClosure, > but note that filtering causes handshake > operation to be done out-order. Since the filter determins who execute the > operation and not the invoked method, there > is now only one method to call when handshaking one thread. Some comments > about the changes: > - HandshakeClosure uses ThreadClosure, since it neat to use the same closure > for both alla JavThreads do and Handshake > all threads. With heap allocating it cannot extends StackObj. I tested > several ways to fix this, but those very much > worse then this. > > - I added a is_handshake_safe_for for checking if it's current thread is > operating on itself or the handshaker of that > thread. > > - Simplified JVM TI with a JvmtiHandshakeClosure and also made them not > needing a JavaThread when executing as a > handshaker on a JavaThread, e.g. VM Thread can execute the handshake > operation. > > - Added WB testing method. > > - Removed VM_HandshakeOneThread, the VM thread uses the same call path as > direct handshakes did. > > - Changed the handshake semaphores to mutex to be able to handle deadlocks > with lock ranking. > > - VM_HandshakeAllThreadsis still a VM operation, since we do support half of > the threads being handshaked before a > safepoint and half of them after, in many handshake all operations. > > - ThreadInVMForHandshake do not need to do a fenced transistion since this is > always a transistion from unsafe to unsafe. > > - Added NoSafepointVerifyer, we are thinking about supporting safepoints > inside handshake, but it's not needed at the > moment. To make sure that gets well tested if added the NoSafepointVerifyer > will raise eyebrows. > > - Added ttyLocker::break_tty_lock_for_safepoint(os::current_thread_id()); due > to the NoSafepointVerifyer. > > - Added filtered queue and gtest for it. > > Passes multiple t1-8 runs. > Been through some pre-reviwing. Robbin Ehn has updated the pull request incrementally with one additional commit since the last revision: Fixed double checks Added NSV ProcessResult to enum Fixed logging Moved _active_handshaker to private - Changes: - all: https://git.openjdk.java.net/jdk/pull/151/files - new: https://git.openjdk.java.net/jdk/pull/151/files/efbee6f0..86b83d05 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=151&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=151&range=00-01 Stats: 72 lines in 4 files changed: 32 ins; 15 del; 25 mod Patch: https://git.openjdk.java.net/jdk/pull/151.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/151/head:pull/151 PR: https://git.openjdk.java.net/jdk/pull/151
Re: RFR: 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents
On Thu, 17 Sep 2020 07:44:09 GMT, Goetz Lindenmaier wrote: >> Hi, >> >> this is the continuation of the review of the implementation for: >> >> https://bugs.openjdk.java.net/browse/JDK-8227745 >> https://bugs.openjdk.java.net/browse/JDK-8233915 >> >> It allows for JIT optimizations based on escape analysis even if JVMTI >> agents acquire capabilities to access references >> to objects that are subject to such optimizations, e.g. scalar replacement. >> The implementation reverts such >> optimizations just before access very much as when switching from JIT >> compiled execution to the interpreter, aka >> "deoptimization". Webrev.8 was the last one before before the transition to >> Git/Github: >> >> http://cr.openjdk.java.net/~rrich/webrevs/8227745/webrev.8/ >> >> Thanks, Richard. > > Marked as reviewed by goetz (Reviewer). Reviewed in the good old hg times :) See also http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-August/041453.html - PR: https://git.openjdk.java.net/jdk/pull/119
Re: RFR: 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents
On Thu, 10 Sep 2020 20:48:23 GMT, Richard Reingruber wrote: > Hi, > > this is the continuation of the review of the implementation for: > > https://bugs.openjdk.java.net/browse/JDK-8227745 > https://bugs.openjdk.java.net/browse/JDK-8233915 > > It allows for JIT optimizations based on escape analysis even if JVMTI agents > acquire capabilities to access references > to objects that are subject to such optimizations, e.g. scalar replacement. > The implementation reverts such > optimizations just before access very much as when switching from JIT > compiled execution to the interpreter, aka > "deoptimization". Webrev.8 was the last one before before the transition to > Git/Github: > > http://cr.openjdk.java.net/~rrich/webrevs/8227745/webrev.8/ > > Thanks, Richard. Marked as reviewed by goetz (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/119
