Re: RFR: 8218812: vmTestbase/nsk/jvmti/GetAllThreads/allthr001/TestDescription.java failed
This part seems to change the value for the include_jni_attaching_threads arg from true to false: - ThreadsListEnumerator tle(Thread::current(), true); + ThreadsListEnumerator tle(Thread::current(), true, false, JvmtiExport::should_show_compiler_threads()); dl
Re: RFR:8219721: jcmd from earlier release will hang attaching to VM with JDK-8215622 applied
Thanks all for reviewing it. I think it could be pushed now, right? BRs, Lin From: Thomas Stüfe Sent: Thursday, March 7, 2019 3:39:05 PM To: 臧琳 Cc: serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net Subject: Re: RFR:8219721: jcmd from earlier release will hang attaching to VM with JDK-8215622 applied Neat! Both directions are fixed now: I can connect with new jcmd to old VMs and vice versa. Thanks a lot! ..Thomas On Mon, Mar 4, 2019 at 10:14 AM 臧琳 mailto:zangl...@jd.com>> wrote: Dear All, May I ask your help to review the fix of compatibility issue introduced by JDK-8215622 webrev: http://cr.openjdk.java.net/~xiaofeya/8219721/webrev.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8219721 FYI, this webrev is forked from the discussion https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-February/027240.html. Thanks, Lin
Re: RFR: JDK-8218128: vmTestbase/nsk/jvmti/ResourceExhausted/resexhausted003 and 004 use wrong path to test classes
Hi The problemlist http://cr.openjdk.java.net/~gadams/8218128/webrev.00/test/hotspot/jtreg/ProblemList.txt.udiff.html Tests are excluded because of https://bugs.openjdk.java.net/browse/JDK-6606767 which is not fixed yet. Why do you want to remove them from problemlist? Also I think it is a good time to update test desrption in the http://cr.openjdk.java.net/~gadams/8218128/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/ResourceExhausted/resexhausted003/TestDescription.java.udiff.html Currently it says about PermGen OOME. Leonid On Thu, 2019-03-07 at 13:57 -0500, Gary Adams wrote: > This proposed fix will restore the ResourceExhausted tests. Test 3 and 4 were on the ProblemList because of the potential path issues in finding the correct classes. This change searches the test.class.path for the appropriate vmTestbase classes rather than using incorrect settings on the command line. Some clean up has been done to remove quarantine keyword and @ignore directives. Should additional clean up be done to remove bug numbers, etc.? TEST.PROPERTIES were added so test 3 so it is consistent with the other tests in the group. Issue: https://bugs.openjdk.java.net/browse/JDK-8218128 Webrev: http://cr.openjdk.java.net/~gadams/8218128/webrev.00/index.html Local testing has been successful on a linux-x64-debug build. Testing on mach5 for other platforms next.
Re: RFR: JDK-8218128: vmTestbase/nsk/jvmti/ResourceExhausted/resexhausted003 and 004 use wrong path to test classes
Hi Gary, Why did you remove the "nonconcurrent" keyword. I know these are just comments for reference that were added when the test was ported from tonga, but as a comment it is still applicable. The test should not be run concurrent with others (which you have also fixed with the addition of the "exclusiveAccess.dirs=."). Otherwise changes look good. thanks, Chris On 3/7/19 10:57 AM, Gary Adams wrote: This proposed fix will restore the ResourceExhausted tests. Test 3 and 4 were on the ProblemList because of the potential path issues in finding the correct classes. This change searches the test.class.path for the appropriate vmTestbase classes rather than using incorrect settings on the command line. Some clean up has been done to remove quarantine keyword and @ignore directives. Should additional clean up be done to remove bug numbers, etc.? TEST.PROPERTIES were added so test 3 so it is consistent with the other tests in the group. Issue: https://bugs.openjdk.java.net/browse/JDK-8218128 Webrev: http://cr.openjdk.java.net/~gadams/8218128/webrev.00/index.html Local testing has been successful on a linux-x64-debug build. Testing on mach5 for other platforms next.
Re: RFR: 8218812: vmTestbase/nsk/jvmti/GetAllThreads/allthr001/TestDescription.java failed
There are other places where is_hidden_from_external_view() is used. Should is_hidden_from_external_view() also check the new capability? If so, then you can simplify your changes. I'm not sure the new capability is the best choice, however. There is still no way to control whether compiler threads can post events, hit breakpoints, single-step, etc. And "compiler thread" might be too specific. There might be other types of "system thread" that we want to ignore. Since this is a JVMTI spec change, I think it deserves more discussion. For example, an alternative would be a way to set "can debug"/"visible"/"can post events"/etc flags on individual threads. dl On 3/7/19 9:54 AM, Daniil Titov wrote: Please review a change that fixes this test. The problem here is that the test checks the number of threads and with Graal on additional threads the test doesn't expect are started and cause the test fail. The fix introduces a new capability " can_show_compiler_threads" that affects whether Java compiler threads are retuned with JVMTI GetAllThreads call. By default this capability is off. The fix also adds " HotSpotGraalManagement Bean Registration" thread to the list of the threads the tests must ignore. Webrev: http://cr.openjdk.java.net/~dtitov/8218812/webrev.01 Bug: https://bugs.openjdk.java.net/browse/JDK-8218812 Mach5 tier1, tier2 and tier3 tests successfully passed with this change. Thanks! -Daniil
Re: RFR: 8218812: vmTestbase/nsk/jvmti/GetAllThreads/allthr001/TestDescription.java failed
On 3/7/19 12:54 PM, Daniil Titov wrote:
Please review a change that fixes this test.
The problem here is that the test checks the number of threads and
with Graal on additional threads the test doesn't expect are started
and cause the test fail.
The fix introduces a new capability " can_show_compiler_threads" that
affects whether Java compiler threads are retuned with JVMTI
GetAllThreads call. By default this capability is off. The fix also
adds " HotSpotGraalManagement Bean Registration" thread to the list of
the threads the tests must ignore.
Webrev: http://cr.openjdk.java.net/~dtitov/8218812/webrev.01
src/hotspot/share/prims/jvmti.xml
L10382: since="13">
You might want to include a diff between the old generated jvmti.h
and the new one. I expect it to show something like this:
unsigned int can_generate_resource_exhaustion_threads_events : 1;
+ unsigned int can_show_compiler_threads : 1;
- unsigned int : 7;
+ unsigned int : 6;
Also, I think this will need a JVM/TI version bump so that an agent
can conditionally compile in the code that accesses the new
can_show_compiler_threads field.
This also will need a CSR since it is changing an API.
src/hotspot/share/prims/jvmtiEnv.cpp
No comments.
src/hotspot/share/prims/jvmtiExport.cpp
No comments.
src/hotspot/share/prims/jvmtiExport.hpp
No comments.
src/hotspot/share/prims/jvmtiManageCapabilities.cpp
No comments.
src/hotspot/share/services/threadService.cpp
L1048: if (!include_compiler_threads && jt->is_Compiler_thread()) {
L1049: continue;
nit - please reduce indent by two spaces
src/hotspot/share/services/threadService.hpp
No comments.
test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp
No comments.
Thumbs up on the code changes. Double check with Serguei about
the need for a CSR...
Dan
Bug: https://bugs.openjdk.java.net/browse/JDK-8218812
Mach5 tier1, tier2 and tier3 tests successfully passed with this change.
Thanks!
-Daniil
RFR: JDK-8218128: vmTestbase/nsk/jvmti/ResourceExhausted/resexhausted003 and 004 use wrong path to test classes
This proposed fix will restore the ResourceExhausted tests. Test 3 and 4 were on the ProblemList because of the potential path issues in finding the correct classes. This change searches the test.class.path for the appropriate vmTestbase classes rather than using incorrect settings on the command line. Some clean up has been done to remove quarantine keyword and @ignore directives. Should additional clean up be done to remove bug numbers, etc.? TEST.PROPERTIES were added so test 3 so it is consistent with the other tests in the group. Issue: https://bugs.openjdk.java.net/browse/JDK-8218128 Webrev: http://cr.openjdk.java.net/~gadams/8218128/webrev.00/index.html Local testing has been successful on a linux-x64-debug build. Testing on mach5 for other platforms next.
RFR: 8218812: vmTestbase/nsk/jvmti/GetAllThreads/allthr001/TestDescription.java failed
Please review a change that fixes this test. The problem here is that the test checks the number of threads and with Graal on additional threads the test doesn't expect are started and cause the test fail. The fix introduces a new capability " can_show_compiler_threads" that affects whether Java compiler threads are retuned with JVMTI GetAllThreads call. By default this capability is off. The fix also adds " HotSpotGraalManagement Bean Registration" thread to the list of the threads the tests must ignore. Webrev: http://cr.openjdk.java.net/~dtitov/8218812/webrev.01 Bug: https://bugs.openjdk.java.net/browse/JDK-8218812 Mach5 tier1, tier2 and tier3 tests successfully passed with this change. Thanks! -Daniil
Re: 8218812: vmTestbase/nsk/jvmti/GetAllThreads/allthr001/TestDescription.java failed
Please disregard this email, somehow it got a wrong body.. Will resend the request later after dealing with my mail client. Best regards, Danill On 3/7/19, 9:46 AM, "serviceability-dev on behalf of Daniil Titov" wrote: Resending review request with corrected subject... Please review a fix for this test that intermittently fails with Graal on. The problem here is that the test does two consequent calls to VirtualMachine.allThreads() and checks that the number of threads returned by these calls is the same. With Graal on there is a chance that a new JVMCI compiler thread could be started between these calls that makes test to fail. The fix temporary suspends the debuggee VM while these two calls to VirtualMachine.allThreads() are made. Webrev: http://cr.openjdk.java.net/~dtitov/8218464/webrev.01 Bug: https://bugs.openjdk.java.net/browse/JDK-8218464 Thanks! --Daniil
RFR: 8218812: vmTestbase/nsk/jvmti/GetAllThreads/allthr001/TestDescription.java failed
Resending review request with corrected subject... Please review a fix for this test that intermittently fails with Graal on. The problem here is that the test does two consequent calls to VirtualMachine.allThreads() and checks that the number of threads returned by these calls is the same. With Graal on there is a chance that a new JVMCI compiler thread could be started between these calls that makes test to fail. The fix temporary suspends the debuggee VM while these two calls to VirtualMachine.allThreads() are made. Webrev: http://cr.openjdk.java.net/~dtitov/8218464/webrev.01 Bug: https://bugs.openjdk.java.net/browse/JDK-8218464 Thanks! --Daniil
8218812: vmTestbase/nsk/jvmti/GetAllThreads/allthr001/TestDescription.java failed
Please review a change that fixes this test. The problem here is that the test checks the number of threads and with Graal on additional threads the test doesn't expect are started and cause the test fail. The fix introduces a new capability " can_show_compiler_threads" that affects whether Java compiler threads are retuned with JVMTI GetAllThreads call. By default this capability is off. The fix also adds " HotSpotGraalManagement Bean Registration" thread to the list of the threads the tests must ignore. Webrev: http://cr.openjdk.java.net/~dtitov/8218812/webrev.01 Bug: https://bugs.openjdk.java.net/browse/JDK-8218812 Mach5 tier1, tier2 and tier3 tests successfully passed with this change. Thanks! -Daniil
Re: RFR: JDK-8218166: [Graal] com/sun/jdi/SimulResumerTest.java failure
Gary,
Since the 'graal' label was recently removed, I also removed "[Graal]"
from the bug synopsis. Please make sure you update your commit mesg.
On 3/7/19 8:19 AM, Gary Adams wrote:
While trying to reproduce the timeout reported in
JDK-8000669: com/sun/jdi/SimulResumerTest.java times out
I was unable to reproduce the timeout failure, but I did occasionally
see the ObjectCollectedException. The output from the test is very
verbose
and may be the source of the occasional timeout. I'd like to close
JDK-8000669
as cannot reproduce and if it shows up again look into limiting the
amount
of non-essential output from the test.
This is a racy test to begin with and it already is ignoring exceptions
due to unexpected thread states. Adding the ignore for
ObjectCollectedException
allows the test to complete without errors.
The graal label was recently removed. We should also remove it from
the summary.
Proposed changeset:
diff --git a/test/jdk/com/sun/jdi/SimulResumerTest.java
b/test/jdk/com/sun/jdi/SimulResumerTest.java
--- a/test/jdk/com/sun/jdi/SimulResumerTest.java
+++ b/test/jdk/com/sun/jdi/SimulResumerTest.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2015, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 2008, 2019, 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
@@ -211,6 +211,8 @@
} catch (IncompatibleThreadStateException ee) {
// ignore
+ } catch (ObjectCollectedException ee) {
+ // ignore
} catch (VMDisconnectedException ee) {
// This is how we stop. The debuggee runs to completion
// and we get this exception.
There should be some sort of comment explaining why it is okay to ignore
the ObjectCollectedException. When the IncompatibleThreadStateException
was ignored, there should have been a comment added for that also.
Dan
RFR: JDK-8218166: [Graal] com/sun/jdi/SimulResumerTest.java failure
While trying to reproduce the timeout reported in
JDK-8000669: com/sun/jdi/SimulResumerTest.java times out
I was unable to reproduce the timeout failure, but I did occasionally
see the ObjectCollectedException. The output from the test is very verbose
and may be the source of the occasional timeout. I'd like to close
JDK-8000669
as cannot reproduce and if it shows up again look into limiting the amount
of non-essential output from the test.
This is a racy test to begin with and it already is ignoring exceptions
due to unexpected thread states. Adding the ignore for
ObjectCollectedException
allows the test to complete without errors.
The graal label was recently removed. We should also remove it from the
summary.
Proposed changeset:
diff --git a/test/jdk/com/sun/jdi/SimulResumerTest.java
b/test/jdk/com/sun/jdi/SimulResumerTest.java
--- a/test/jdk/com/sun/jdi/SimulResumerTest.java
+++ b/test/jdk/com/sun/jdi/SimulResumerTest.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2015, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 2008, 2019, 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
@@ -211,6 +211,8 @@
} catch (IncompatibleThreadStateException ee) {
// ignore
+} catch (ObjectCollectedException ee) {
+// ignore
} catch (VMDisconnectedException ee) {
// This is how we stop. The debuggee runs to completion
// and we get this exception.
