Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v6]
On Wed, 16 Sep 2020 16:13:23 GMT, Daniel D. Daugherty
wrote:
>> This RFE is to migrate the following field to OopStorage:
>>
>> class ObjectMonitor {
>>
>> void* volatile _object; // backward object pointer - strong root
>>
>> Unlike the previous patches in this series, there are a lot of collateral
>> changes so this is not a trivial review. Sorry for the tedious parts of
>> the review. Since Erik and I are both contributors to this patch, we
>> would like at least 1 GC team reviewer and 1 Runtime team reviewer.
>>
>> This changeset was tested with Mach5 Tier[1-3],4,5,6,7,8 testing
>> along with JDK-8252980 and JDK-8252981. I also ran it through my
>> inflation stress kit for 48 hours on my Linux-X64 machine.
>
> Daniel D. Daugherty has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Minor changes/deletions to address kimbarrett and sspitsyn CR comments.
Marked as reviewed by kbarrett (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/135
Re: RFR: 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. Agreed that this is trivial change - Marked as reviewed by amenkov (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/192
Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v6]
On Wed, 16 Sep 2020 16:13:23 GMT, Daniel D. Daugherty
wrote:
>> This RFE is to migrate the following field to OopStorage:
>>
>> class ObjectMonitor {
>>
>> void* volatile _object; // backward object pointer - strong root
>>
>> Unlike the previous patches in this series, there are a lot of collateral
>> changes so this is not a trivial review. Sorry for the tedious parts of
>> the review. Since Erik and I are both contributors to this patch, we
>> would like at least 1 GC team reviewer and 1 Runtime team reviewer.
>>
>> This changeset was tested with Mach5 Tier[1-3],4,5,6,7,8 testing
>> along with JDK-8252980 and JDK-8252981. I also ran it through my
>> inflation stress kit for 48 hours on my Linux-X64 machine.
>
> Daniel D. Daugherty has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Minor changes/deletions to address kimbarrett and sspitsyn CR comments.
Marked as reviewed by dholmes (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/135
Re: RFR: 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. Ping! This change is trivial. Thanks! - PR: https://git.openjdk.java.net/jdk/pull/192
RFR: 8234808: jdb quoted option parsing broken
Migrating this review from the mailing list into a pull request. Some background: when jdb launches debuggee process it passes java options from "options" value for CommandLineLaunch connector and forward options specified before command. The fix solves several discovered issues: - proper handling of java options with spaces - if both way are used to specify java options, forwarded options override options from "options" value VMConnection class implements tricky logic for "options" field parsing for JFR needs (handling of single and double quotes). I decided to keep it as is to avoid massive test failures with JFR (there is no test coverage for this functionality and I'm not sure I understand all requirements). - Commit messages: - 8234808: jdb quoted option parsing broken Changes: https://git.openjdk.java.net/jdk/pull/211/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=211=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8234808 Stats: 205 lines in 4 files changed: 190 ins; 4 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/211.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/211/head:pull/211 PR: https://git.openjdk.java.net/jdk/pull/211
Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v6]
On Wed, 16 Sep 2020 16:13:23 GMT, Daniel D. Daugherty
wrote:
>> This RFE is to migrate the following field to OopStorage:
>>
>> class ObjectMonitor {
>>
>> void* volatile _object; // backward object pointer - strong root
>>
>> Unlike the previous patches in this series, there are a lot of collateral
>> changes so this is not a trivial review. Sorry for the tedious parts of
>> the review. Since Erik and I are both contributors to this patch, we
>> would like at least 1 GC team reviewer and 1 Runtime team reviewer.
>>
>> This changeset was tested with Mach5 Tier[1-3],4,5,6,7,8 testing
>> along with JDK-8252980 and JDK-8252981. I also ran it through my
>> inflation stress kit for 48 hours on my Linux-X64 machine.
>
> Daniel D. Daugherty has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Minor changes/deletions to address kimbarrett and sspitsyn CR comments.
Marked as reviewed by eosterlund (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/135
Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v6]
On Wed, 16 Sep 2020 16:13:23 GMT, Daniel D. Daugherty
wrote:
>> This RFE is to migrate the following field to OopStorage:
>>
>> class ObjectMonitor {
>>
>> void* volatile _object; // backward object pointer - strong root
>>
>> Unlike the previous patches in this series, there are a lot of collateral
>> changes so this is not a trivial review. Sorry for the tedious parts of
>> the review. Since Erik and I are both contributors to this patch, we
>> would like at least 1 GC team reviewer and 1 Runtime team reviewer.
>>
>> This changeset was tested with Mach5 Tier[1-3],4,5,6,7,8 testing
>> along with JDK-8252980 and JDK-8252981. I also ran it through my
>> inflation stress kit for 48 hours on my Linux-X64 machine.
>
> Daniel D. Daugherty has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Minor changes/deletions to address kimbarrett and sspitsyn CR comments.
Marked as reviewed by sspitsyn (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/135
Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v5]
On Tue, 15 Sep 2020 10:17:59 GMT, Kim Barrett wrote: >> Daniel D. Daugherty has updated the pull request incrementally with one >> additional commit since the last revision: >> >> rkennke, coleenp, fisk CR - delete random assert() that knows too much >> about markWords. > > Marked as reviewed by kbarrett (Reviewer). @kimbarrett and @sspitsyn - Your most recent comments should be resolved via https://github.com/openjdk/jdk/pull/135/commits/215084ac7ef481713560d14498ce420a40ca813a. - PR: https://git.openjdk.java.net/jdk/pull/135
Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v6]
> This RFE is to migrate the following field to OopStorage:
>
> class ObjectMonitor {
>
> void* volatile _object; // backward object pointer - strong root
>
> Unlike the previous patches in this series, there are a lot of collateral
> changes so this is not a trivial review. Sorry for the tedious parts of
> the review. Since Erik and I are both contributors to this patch, we
> would like at least 1 GC team reviewer and 1 Runtime team reviewer.
>
> This changeset was tested with Mach5 Tier[1-3],4,5,6,7,8 testing
> along with JDK-8252980 and JDK-8252981. I also ran it through my
> inflation stress kit for 48 hours on my Linux-X64 machine.
Daniel D. Daugherty has updated the pull request incrementally with one
additional commit since the last revision:
Minor changes/deletions to address kimbarrett and sspitsyn CR comments.
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/135/files
- new: https://git.openjdk.java.net/jdk/pull/135/files/eeb9d761..215084ac
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=135=05
- incr: https://webrevs.openjdk.java.net/?repo=jdk=135=04-05
Stats: 9 lines in 2 files changed: 2 ins; 7 del; 0 mod
Patch: https://git.openjdk.java.net/jdk/pull/135.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/135/head:pull/135
PR: https://git.openjdk.java.net/jdk/pull/135
Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v5]
On Wed, 16 Sep 2020 08:23:03 GMT, Erik Österlund wrote: >> Marked as reviewed by sspitsyn (Reviewer). > > I added a release note (https://bugs.openjdk.java.net/browse/JDK-8253225) > describing that these roots are now weak, and > hence won't be reported. Please have a look at that, to make sure what I am > describing makes sense. @kimbarrett - Thanks for the cleanup suggestion. @sspitsyn - Thanks for the review. I've gone ahead and "resolved" the comments that were addressed by previous commits. I've made changes for both Kim and Serguei's reviews. Building those bits now. - PR: https://git.openjdk.java.net/jdk/pull/135
Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v5]
On Wed, 16 Sep 2020 00:07:08 GMT, Serguei Spitsyn wrote: >> I've taken a first pass at creating a CSR: >> JDK-8253121 migrate ObjectMonitor::_object to OopStorage >> https://bugs.openjdk.java.net/browse/JDK-8253121 > > I've looked at the CSR and added myself as a reviewer. > We had a slack chat with Dan, and I agree that with a weak handle it would be > racy/unsafe for > JVMTI_HEAP_REFERENCE_MONITOR calls back to be called. The ObjectMonitors do > not pin objects anymore (there are no > strong refs from them), so it has to be okay to skip reporting the > JVMTI_HEAP_REFERENCE_MONITOR and and > JVMTI_HEAP_ROOT_MONITOR (old Heap Walking API) reference types. The JVMTI > does not need an update as other VM > implementations can still report these reference types. Alan added a comment > to the CSR saying that memory profiling > tools that use the JVMTI functions (FollowReferences with > jvmtiHeapReferenceCallback or IterateOverReachableObjects > with jvmtiHeapRootCallback) to iterate over the heap should not have a > compatibility impact as these reference types > are just informational but adding a release note can be useful. Slight clarification. Serguei and I were discussing whether we could continue to make JVMTI_HEAP_REFERENCE_MONITOR call backs or emit HPROF_GC_ROOT_MONITOR_USED entries in heap dump output as a way to ease the transition phase of getting used to these things going away. My answer was that we could do that but it would racy and unsafe due to the ObjectMonitor's object being GC'ed. It's also incorrect to make JVMTI_HEAP_REFERENCE_MONITOR call backs or emit HPROF_GC_ROOT_MONITOR_USED entries in heap dump once the ObjectMonitor's object ref becomes a weak handle. That weak handle no longer prevents the associated object from being GC'ed so it is no longer a strong root. See Erik's comment above: https://github.com/openjdk/jdk/pull/135#discussion_r487291746 - PR: https://git.openjdk.java.net/jdk/pull/135
Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v5]
On Wed, 16 Sep 2020 00:20:16 GMT, Serguei Spitsyn wrote:
>> I've taken a first pass at creating a CSR:
>> JDK-8253121 migrate ObjectMonitor::_object to OopStorage
>> https://bugs.openjdk.java.net/browse/JDK-8253121
>
> Just a minor comment.
> The line 1754 can be deleted as the JVMTI_HEAP_REFERENCE_MONITOR reference
> type will be never encountered:
>
> 1750 static jvmtiHeapRootKind toJvmtiHeapRootKind(jvmtiHeapReferenceKind
> kind) {
> 1751 switch (kind) {
> 1752 case JVMTI_HEAP_REFERENCE_JNI_GLOBAL: return
> JVMTI_HEAP_ROOT_JNI_GLOBAL;
> 1753 case JVMTI_HEAP_REFERENCE_SYSTEM_CLASS: return
> JVMTI_HEAP_ROOT_SYSTEM_CLASS;
> 1754 case JVMTI_HEAP_REFERENCE_MONITOR: return
> JVMTI_HEAP_ROOT_MONITOR;
> 1755 case JVMTI_HEAP_REFERENCE_STACK_LOCAL: return
> JVMTI_HEAP_ROOT_STACK_LOCAL;
> 1756 case JVMTI_HEAP_REFERENCE_JNI_LOCAL:return
> JVMTI_HEAP_ROOT_JNI_LOCAL;
> 1757 case JVMTI_HEAP_REFERENCE_THREAD: return
> JVMTI_HEAP_ROOT_THREAD;
> 1758 case JVMTI_HEAP_REFERENCE_OTHER:return JVMTI_HEAP_ROOT_OTHER;
> 1759 default: ShouldNotReachHere(); return JVMTI_HEAP_ROOT_OTHER;
> 1760 }
> 1761 }
>
> Other than that the update in this file looks okay to me.
I cleaned that up. The only references to JVMTI_HEAP_REFERENCE_MONITOR and
JVMTI_HEAP_ROOT_MONITOR that remain are in the spec:
$ egrep -r 'JVMTI_HEAP_REFERENCE_MONITOR|JVMTI_HEAP_ROOT_MONITOR' src/hotspot
src/hotspot/share/prims/jvmti.xml:
src/hotspot/share/prims/jvmti.xml:
src/hotspot/share/prims/jvmti.xml:
JVMTI_HEAP_ROOT_MONITOR,
-
PR: https://git.openjdk.java.net/jdk/pull/135
Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v4]
On Tue, 15 Sep 2020 10:17:45 GMT, Kim Barrett wrote: >> Daniel D. Daugherty has updated the pull request incrementally with one >> additional commit since the last revision: >> >> kimbarrett CR - made minor changes to address Kim's code review. > > src/hotspot/share/runtime/objectMonitor.cpp line 244: > >> 242: } >> 243: >> 244: #ifdef ASSERT > > There would be less `#ifdef ASSERT` clutter if just the body of > check_object_context were conditionalized. Then the > calls wouldn't need to be. Your call... I'll make that change. - PR: https://git.openjdk.java.net/jdk/pull/135
Re: RFR: 8252104: parallel heap inspection for ShenandoahHeap [v3]
On Sat, 12 Sep 2020 00:17:58 GMT, Lin Zang wrote: >> - enable parallel heap inspecton for ShenandoahHeap >> - preliminary evaluation: >> Time of jmap histo on (8GB heap with 4GB objects) >> * before: 5.186s >> * after : 1.698s > > Lin Zang has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views > will show differences compared to the previous content of the PR. The pull > request contains one new commit since the > last revision: > 8252104: parallel heap inspection for ShenandoahHeap > > - enable parallel heap inspecton for ShenandoahHeap > - preliminary evaluation: > Time of jmap histo on (8GB heap with 4GB objects) > * before: 5.186s > * after : 1.698s Marked as reviewed by zgu (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/67
Re: RFR: 8252104: parallel heap inspection for ShenandoahHeap [v3]
On Sat, 12 Sep 2020 00:15:06 GMT, Lin Zang wrote: > Hi @zhengyu123 , > Thanks for your comments! I have refined the code and update the PR. > -Lin Looks good to me. @shipilev may want to take a look. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/67
Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v5]
On Wed, 16 Sep 2020 00:21:02 GMT, Serguei Spitsyn wrote: >> Daniel D. Daugherty has updated the pull request incrementally with one >> additional commit since the last revision: >> >> rkennke, coleenp, fisk CR - delete random assert() that knows too much >> about markWords. > > Marked as reviewed by sspitsyn (Reviewer). I added a release note (https://bugs.openjdk.java.net/browse/JDK-8253225) describing that these roots are now weak, and hence won't be reported. Please have a look at that, to make sure what I am describing makes sense. - PR: https://git.openjdk.java.net/jdk/pull/135
Re: RFR: 8238761: Asynchronous handshakes
Hi Robbin,
Changes look good to me! Some minor comments:
src/hotspot/share/prims/jvmtiThreadState.cpp
222: assert(current_thread == get_thread() ||
223: SafepointSynchronize::is_at_safepoint() ||
224: get_thread()->is_handshake_safe_for(current_thread),
225: "call by myself / at safepoint / at handshake");
Extra current_thread == get_thread() is already handled by
is_handshake_safe_for().
src/hotspot/share/prims/jvmtiEnvBase.cpp
Same as above.
src/hotspot/share/runtime/handshake.cpp
198: log_info(handshake)("Handshake \"%s\", Targeted threads: %d,
Executed by targeted threads: %d, Total completion time: " JLONG_FORMAT
" ns%s%s",
199: name, targets,
200: targets - vmt_executed,
In the calls to log_handshake_info() in VM_HandshakeAllThreads and
Handshake::execute() we are passing as vmt_executed the number of
handshakes that the driver executed which could be more than the targets
parameter. So the operation "targets - vmt_executed" to calculate the
handshakes executed by the targets would no longer be valid. Personally
I would just leave ProcessResult as an enum and log as before. We still
have a log_trace() in try_process(), so that already keeps track of
extra handshakes executed by the handshaker.
src/hotspot/share/runtime/handshake.cpp
387: NoSafepointVerifier nsv;
388: process_self_inner();
389: }
Shouldn't NoSafepointVerifier be placed before the execution of the
handshake closure so that we also cover the case when the handshake is
executed by the handshaker? As in:
// Only actually execute the operation for non terminated threads.
if (!thread->is_terminated()) {
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?
Thanks!
Patricio
On 9/15/20 4:39 AM, 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.
Re: RFR: 8252593: [TESTBUG] serviceability/jvmti/GetLocalVariable/GetLocalWithoutSuspendTest.java failed with JVMTI_ERROR_INVALID_SLOT
On Wed, 16 Sep 2020 06:26:51 GMT, Chris Plummer 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 > > Marked as reviewed by cjplummer (Reviewer). Thanks Serguei and Chris. I'll integrate this tomorrow. - PR: https://git.openjdk.java.net/jdk/pull/142
Re: RFR: 8241390: 'Deadlock' with VM_RedefineClasses::lock_classes()
On Tue, 15 Sep 2020 20:02:38 GMT, Coleen Phillimore 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. > > Changes requested by coleenp (Reviewer). The identifier 'cls' does not reflect the nature of the object and needs to be replaces with something like: classes, redef_classes or classes_list This impacts the files: jvmtiThreadState.hpp and jvmtiRedefineClasses.cpp. - PR: https://git.openjdk.java.net/jdk/pull/190
Re: RFR: 8241390: 'Deadlock' with VM_RedefineClasses::lock_classes()
On Tue, 15 Sep 2020 20:02:33 GMT, Coleen Phillimore 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.
>
> src/hotspot/share/prims/jvmtiRedefineClasses.cpp line 159:
>
>> 157: if (!cls->contains(def_ik)) {
>> 158: def_ik->set_is_being_redefined(false);
>> 159: }
>
> Ok, so adding the Klass to the thread local list for each recursion works
> like ref counting. Can't think of a simpler
> way to do this. Looks good.
Yes, the same class can be pushed to the list multiple times (not more than
once by each recursive redefinition). It'd
make sense to add a comment about it as it is not obvious.
> test/jdk/java/lang/instrument/MakeAgentJAR.sh line 1:
>
>> 1: #!/bin/sh
>
> There are tests in test/hotspot/jtreg/serviceability/jvmti/RedefineClasses
> that don't use shell scripts that are much
> better. Can you add this test using that framework instead?
I'm second for this suggestion from Coleen to get rid of the shell script in
the test.
-
PR: https://git.openjdk.java.net/jdk/pull/190
Re: RFR: 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 Marked as reviewed by cjplummer (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/142
