Re: RFR(L): 8215624: add parallel heap inspection support for jmap histo(G1)(Internet mail)

2020-06-29 Thread 臧琳
Dear All, 
Sorry to bother again, I just want to make sure that is this change 
worth to be continue to work on? If decision is made to not. I think I can drop 
this work and stop asking for help reviewing...
Thanks for all your help about reviewing this previously. 

BRs,
Lin

On 2020/5/9, 3:47 PM, "linzang(臧琳)"  wrote:

Dear All, 
   May I ask your help again for review the latest change?  Thanks!

BRs,
Lin

On 2020/4/28, 1:54 PM, "linzang(臧琳)"  wrote:

Hi Stefan, 
  >>  - Adding Atomic::load/store.
  >>  - Removing the time measurement in the run_task. I renamed G1's 
function 
  >>  to run_task_timed. If we need this outside of G1, we can rethink 
the API 
  >>  at that point.
   >>  - ZGC style cleanups
   Thanks for revising the patch,  they are all good to me, and I have 
made a tiny change based on it: 
   
http://cr.openjdk.java.net/~lzang/jmap-8214535/8215624/webrev_04/ 
   
http://cr.openjdk.java.net/~lzang/jmap-8214535/8215624/webrev_04-delta/
  it reduce the scope of mutex in ParHeapInspectTask, and delete 
unnecessary comments.

BRs,
Lin

On 2020/4/27, 4:34 PM, "Stefan Karlsson"  
wrote:

Hi Lin,

On 2020-04-26 05:10, linzang(臧琳) wrote:
> Hi Stefan and Paul,
>  I have made a new patch based on your comments and Stefan's 
Poc code:
>  Webrev: 
http://cr.openjdk.java.net/~lzang/jmap-8214535/8215624/webrev_03/
>  Delta(based on Stefan's change:) : 
http://cr.openjdk.java.net/~lzang/jmap-8214535/8215624/webrev_03-delta/webrev_03-delta/

Thanks for providing a delta patch. It makes it much easier to look 
at, 
and more likely for reviewers to continue reviewing.

I'm going to continue focusing on the GC parts, and leave the rest 
to 
others to review.

> 
>  And Here are main changed I made and want to discuss with 
you:
>  1.  changed"parallelThreadNum=" to "parallel=" for jmap 
-histo options.
>  2.  Add logic to test where parallelHeapInspection is fail, 
in heapInspection.cpp
>This is because the parHeapInspectTask create thread 
local KlassInfoTable in it's work() method, and this may fail because of native 
OOM, in this case, the parallel should fail and serial heap inspection can be 
tried.
>One more thing I want discuss with you is about the 
member "_success" of parHeapInspectTask, when native OOM happenes, it is set to 
false. And since this "set" operation can be conducted in multiple threads, 
should it be atomic ops?  IMO, this is not necessary because "_success" can 
only be set to false, and there is no way to change it from back to true after 
the ParHeapInspectTask instance is created, so it is save to be non-atomic, do 
you agree with that?

In these situations you should be using the Atomic::load/store 
primitives. We're moving toward a later C++ standard were data 
races are 
considered undefined behavior.

> 3. make CollectedHeap::run_task() be an abstract virtual 
func, so that every subclass of collectedHeap should support it, so later 
implementation of new collectedHeap will not miss the "parallel" features.
>   The problem I want to discuss with you is about 
epsilonHeap and SerialHeap, as they may not need parallel heap iteration, so I 
only make task->work(0), in case the run_task() is invoked someway in future. 
Another way is to left run_task()  unimplemented, which one do you think is 
better?

I don't have a strong opinion about this.

  And also please help take a look at the zHeap, as there is a 
class 
zTask that wrap the abstractGangTask, and the 
collectedHeap::run_task() 
only accept  AbstraceGangTask* as argument, so I made a delegate 
class 
to adapt it , please see src/hotspot/share/gc/z/zHeap.cpp.
> 
>There maybe other better ways to sovle the above problems, 
welcome for any comments, Thanks!

I've created a few cleanups and changes on top of your latest patch:

https://cr.openjdk.java.net/~stefank/8215624/webrev.02.delta
https://cr.openjdk.java.net/~stefank/8215624/webrev.02

- Adding Atomic::load/store.
- Removing the time measurement in the run_task. I renamed G1's 
function 
to run_task_timed. If we need this outside of G1, we can rethink 
the API 
at that point.
- ZGC style cleanups

Thanks,
StefanK

> 
> BRs,
> Lin
> 
> On 2020/4/23, 11:08 AM, "linzang(臧琳)"  wrote:
> 
>  Thanks Paul! I agree with us

Re: RFR (Preliminary): 8248194: Need better support for running SA tests on core files

2020-06-29 Thread Chris Plummer

Hi Leonid,

I'm starting to think that this should all go in a new CoreUtils.java 
file. I experimented with moving getCoreFileLocation() to 
OutputAnalyzer. It worked, but one adjustment I had to make is also 
moving SATestUtils.unzipCores() there also and make it private (no one 
else calls it). But that just got me thinking that maybe CoreUtils.java 
would be a better solution. I think I would put the 
addCoreUlimitCommand() API discussed below there also. What do you think?


thanks,

Chris

On 6/28/20 5:29 PM, Chris Plummer wrote:

Hi Leonid,

I think getCoreFileLocation() can simply move to OutputAnalyzer. No 
need for it to be in SAUtils and be passed the String argument that 
comes from OutputAnalyzer.getOutput().


For the ulimit support, how about if in ProcessTools I add:

    public static ProcessBuilder addCoreUlimitCommand(ProcessBuilder pb);

All the ulimit logic would move there from SATestUtils. It's straight 
forward to use this API in LingeredApp and TestJmapCore. For 
ClhsdbCDSCore I'll need to rework the 
getTestJvmCommandlineWithPrefix() code a bit, since it creates a pb, 
but doesn't save it. It only uses it to get the cmd String.


Also, there's one new finding since I sent out the review. I found the 
following in CiReplayBase.java:


    // lets search few possible locations using process output and 
return existing location

    private String getCoreFileLocation(String crashOutputString) {

This is identical to the code I pulled from ClhsdbCDSCore and is now 
in SATestUtils.parseCoreFileLocationFromOutput(). Although this is in 
the compiler directory, it is in fact an SA test that uses clhsdb, 
although directly via the CLHSDB class rather than through "jhsdb 
clhsdb".


This also explains why ClhsdbCDSCore had some logic to move and rename 
the core file to "cds_core_file". I removed this logic because it 
seemed unnecessary, but for CiReplayBase.java it needs to be in a 
known location so SABase.java can find it. It's still fine for 
ClhsdbCDSCore to not do the rename, and renaming is independent of any 
code that locates the core file.


I'm not going to update CiReplayBase.java as part of these changes 
because the two tests that use it both have issues. TestSAServer is 
problem listed, and when I removed it from the problem list it failed 
with every run on every platform. There's also TestSAClient, but it 
relies on client VM, which we don't support anymore. So with neither 
of these tests running, I'd rather not introduce changes I can't 
really test.


However, there was something good that came out of the 
CiReplayBase.java discovery. I had previously noted that ClhsdbCDSCore 
is excluded from running on windows. When I removed the @requires for 
this, it failed for a reason I didn't quite understand. The complaint 
was about the path to java.exe when running the process that was 
suppose to crash, although the path looked fine. However, I found that 
TestSAServer ran fine on Windows, even though it was basically the 
process launching code for causing the crash. I looked closer and 
found one difference. In getTestJvmCommandlineWithPrefix(), which both 
tests have, the CiReplayBase version had some extra code for Windows:


    return new String[]{"sh", "-c", prefix
    + (Platform.isWindows() ? cmd.replace('\\', 
'/').replace(";", "\\;").replace("|", "\\|") : cmd)};


So on Windows it's doing a path conversion. Once I started doing the 
same with ClhsdbCDSCore, it started to run fine on Windwos also.


thanks,

Chris

On 6/26/20 8:42 PM, Chris Plummer wrote:

Hi Leonid,

On 6/26/20 7:51 PM, Leonid Mesnik wrote:

Hi

The idea basically looks good. I think it just make a sense to 
polish it a little bit to hide "sh" usage from test and get core 
from OutputAnalyzer.

Ok, I'll look into both of those.
  Also, there is a 'CrashApp' in ClhsdbCDSCore.java. Makes it sense 
to unify it with LingeredApp crasher? Currently, it uses Unsafe to 
crash application.
Yes, I purposely didn't not make that change. My main goal with the 
LingeredApp changes is to make it easier to make existing LingeredApp 
SA tests run on both a live process and on a core, and my main goal 
with ClhsdbCDSCore and TestJmapCore was to move the core finding code 
and ulimit code to a common location that could be reused by other 
tests.


Keep in mind that ClhsdbLauncher and LingeredApp are independent of 
each other. You can have a LingeredApp tests that use or don't use 
ClhsdbLauncher, and you can have a non-LingeredApp tests that use or 
don't use ClhsdbLauncher. So I didn't want to go down the path of 
changing ClhsdbCDSCore (a non LingeredApp test) to use LingeredApp. 
Likewise I did not change TestJmapCore to use LingeredApp or 
ClhsdbLauncher. Possibly there is good reason to convert some of the 
tests to start using LingeredApp and/or ClhsdbLauncher, but that 
should be done under a separate RFE.




Also, crashes are used in other tests, I see some implementations in
open/t

Re: RFR: 8242428: JVMTI thread operations should use Thread-Local Handshake

2020-06-29 Thread Yasumasa Suenaga

Hi David, Serguei,

I updated webrev for 8242428. Could you review again?
This change migrate to use direct handshake for GetStackTrace() and 
GetThreadListStackTraces() (when thread_count == 1).

  http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.01/

VM_GetThreadListStackTrace (for GetThreadListStackTraces) and 
VM_GetAllStackTraces (for GetAllStackTraces) have inherited 
VM_GetMultipleStackTraces VM operation which provides the feature to generate 
jvmtiStackInfo. I modified  VM_GetMultipleStackTraces to a normal C++ class to 
share with HandshakeClosure for GetThreadListStackTraces 
(GetSingleStackTraceClosure).

Also I added new testcases which test GetThreadListStackTraces() with 
thread_count == 1 and with all threads.

This change has been tested in serviceability/jvmti serviceability/jdwp 
vmTestbase/nsk/jvmti vmTestbase/nsk/jdi vmTestbase/nsk/jdwp.


Thanks,

Yasumasa


On 2020/06/24 15:50, Yasumasa Suenaga wrote:

Hi all,

Please review this change:

   JBS: https://bugs.openjdk.java.net/browse/JDK-8242428
   webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.00/

This change replace following VM operations to direct handshake.

  - VM_GetFrameCount (GetFrameCount())
  - VM_GetFrameLocation (GetFrameLocation())
  - VM_GetThreadListStackTraces (GetThreadListStackTrace())
  - VM_GetCurrentLocation

GetThreadListStackTrace() uses direct handshake if thread count == 1. In other 
case (thread count > 1), it would be performed as VM operation 
(VM_GetThreadListStackTraces).
Caller of VM_GetCurrentLocation (JvmtiEnvThreadState::reset_current_location()) 
might be called at safepoint. So I added safepoint check in its caller.

This change has been tested in serviceability/jvmti serviceability/jdwp 
vmTestbase/nsk/jvmti vmTestbase/nsk/jdi vmTestbase/ns
k/jdwp.

Also I tested it on submit repo, then it has execution error 
(mach5-one-ysuenaga-JDK-8242428-20200624-0054-12034717) due to dependency 
error. So I think it does not occur by this change.


Thanks,

Yasumasa


Re: RFR(S): 8246493: JDI stress/serial/mixed002 needs to use WhiteBox.deflateIdleMonitors support

2020-06-29 Thread Daniel D. Daugherty

I'll add a note to 8246493.

Dan


On 6/29/20 5:02 PM, Chris Plummer wrote:
Ok, in that case it sounds best not to backport. It would be best to 
make this clear in the bug so there is no future attempt to backport 
this change except to versions that have already done the 
WhiteBox.deflateIdleMonitors() backport.


thanks,

Chris

On 6/29/20 1:53 PM, Daniel D. Daugherty wrote:

The WhiteBox.deflateIdleMonitors() support is not in JDK15. That's
something that added in JDK16 so I'd have to also backport that support.
That support was included with another change (getting rid of the 
special

deflation request mechanism) that is not appropriate for JDK15.

Short version: I don't think we want to back port part of a patch from
JDK16 -> JDK15 in order to fix this test bug.

Dan


On 6/29/20 4:41 PM, Chris Plummer wrote:

Hi Dan,

I think you should push it directly to 15 since it's a new issue.

thanks,

Chris

On 6/29/20 12:53 PM, Daniel D. Daugherty wrote:

Chris,

Thanks. One last thing... since this is a test bug, I wasn't 
planning to
backport the fix to JDK15. The test is ProblemListed there so we 
won't see

the intermittent failures.

Are you and Serguei good with not fixing this test bug in JDK15?

Dan

P.S.
Thanks again for your sleuthing that linked the bug to my
fix for JDK-8153224.


On 6/29/20 3:49 PM, Chris Plummer wrote:

Looks good.

Chris

On 6/29/20 12:45 PM, Daniel D. Daugherty wrote:

Chris and Serguei,

Thanks for the fast reviews!!

I generated the webrev in my "mach5" directory and that was 
baselined
on the jdk-16+3 snapshot and that doesn't include the ProblemList 
change.
Sigh...  I have updated the repo to "current" and regenerated the 
webrev.


test/hotspot/jtreg/ProblemList.txt  now shows:

@@ -126,11 +126,10 @@
 vmTestbase/nsk/monitoring/ThreadMXBean/ThreadInfo/Deadlock/JavaDeadlock001/TestDescription.java 
8060733 generic-all


 vmTestbase/nsk/jdi/ThreadReference/stop/stop001/TestDescription.java 
7034630 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses021/TestDescription.java 
8065773 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses023/TestDescription.java 
8065773 generic-all
-vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 
8246493 generic-all


 vmTestbase/nsk/jdb/eval/eval001/eval001.java 8221503 generic-all

 vmTestbase/metaspace/gc/firstGC_10m/TestDescription.java 8208250 
generic-all
 vmTestbase/metaspace/gc/firstGC_50m/TestDescription.java 8208250 
generic-all


Thanks again for the fast reviews!!

Dan


On 6/29/20 3:41 PM, serguei.spit...@oracle.com wrote:

Hi Dan,

The same as from Chris.
The ProblemList.txt has no changes.
Otherwise, it looks good.

Thanks,
Serguei



On 6/29/20 12:37, Chris Plummer wrote:

Hi Dan,

Something is wrong with ProblemList.txt. It doesn't show any 
changes, but I also don't see mixed002 in the file anymore.


Otherwise the changes look good.

thanks,

Chris

On 6/29/20 12:21 PM, Daniel D. Daugherty wrote:

Greetings,

I have a fix for the following bug:

    JDK-8246493 JDI stress/serial/mixed002 needs to use 
WhiteBox.deflateIdleMonitors support

https://bugs.openjdk.java.net/browse/JDK-8246493

Here's the webrev URL:

http://cr.openjdk.java.net/~dcubed/8246493-webrev/0_for_jdk16/

The test bug that's being fixed:

vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 
fails

    intermittently with the following message:

 nsk.share.TestBug: There are more than one(2) instance of 
'nsk.share.jpda.StateTestThread in debuggee


Summary of the fix:

    Use WhiteBox.deflateIdleMonitors() to make sure that all 
inflated

    ObjectMonitors are deflated after each debuggee has been run.

This fix has been tested with a Mach5 Tier5 test run that 
executes all
of the JDI tests (along with JDWP, JVM/TI and other 
Serviceability tests).
I also did five 100 iteration runs of the failing mix002 test. 
Each Mach5
job set ran the test 100 times on Linux-X64, macOSX, and 
Win-X64 for a
total of (5 * 100 * 3) iterations of 
nsk/jdi/stress/serial/mixed002. There

were no failures.

Thanks, in advance, for any comments, questions or suggestions.

Dan


Gory details:

The primary focus of the fix is in the first three files in 
the webrev:


test/hotspot/jtreg/vmTestbase/nsk/share/jdi/SerialExecutionDebuggee.java 

test/hotspot/jtreg/vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 


test/hotspot/jtreg/ProblemList.txt

nsk.share.jdi.SerialExecutionDebuggee is the class that used 
to serially
execute the debuggee portion of a specific list of tests. 
After this class
is done executing a debuggee class, it needs to deflate idle 
monitors in
order to prevent a StateTestThread object created by one 
debuggee class
from confusing the next debuggee class. Each of the debuggee 
classes that
use StateTestThread expect there to be only one of these 
objects. However,
since we are running multiple debuggee classes serially *in 
the same VM*,
the StateTestThread

Re: RFR(S): 8246493: JDI stress/serial/mixed002 needs to use WhiteBox.deflateIdleMonitors support

2020-06-29 Thread Daniel D. Daugherty

Thanks for review!

Dan


On 6/29/20 5:16 PM, serguei.spit...@oracle.com wrote:

+1

Thanks,
Serguei


On 6/29/20 12:49, Chris Plummer wrote:

Looks good.

Chris

On 6/29/20 12:45 PM, Daniel D. Daugherty wrote:

Chris and Serguei,

Thanks for the fast reviews!!

I generated the webrev in my "mach5" directory and that was baselined
on the jdk-16+3 snapshot and that doesn't include the ProblemList 
change.
Sigh...  I have updated the repo to "current" and regenerated the 
webrev.


test/hotspot/jtreg/ProblemList.txt  now shows:

@@ -126,11 +126,10 @@
 vmTestbase/nsk/monitoring/ThreadMXBean/ThreadInfo/Deadlock/JavaDeadlock001/TestDescription.java 
8060733 generic-all


 vmTestbase/nsk/jdi/ThreadReference/stop/stop001/TestDescription.java 
7034630 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses021/TestDescription.java 
8065773 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses023/TestDescription.java 
8065773 generic-all
-vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 
8246493 generic-all


 vmTestbase/nsk/jdb/eval/eval001/eval001.java 8221503 generic-all

 vmTestbase/metaspace/gc/firstGC_10m/TestDescription.java 8208250 
generic-all
 vmTestbase/metaspace/gc/firstGC_50m/TestDescription.java 8208250 
generic-all


Thanks again for the fast reviews!!

Dan


On 6/29/20 3:41 PM, serguei.spit...@oracle.com wrote:

Hi Dan,

The same as from Chris.
The ProblemList.txt has no changes.
Otherwise, it looks good.

Thanks,
Serguei



On 6/29/20 12:37, Chris Plummer wrote:

Hi Dan,

Something is wrong with ProblemList.txt. It doesn't show any 
changes, but I also don't see mixed002 in the file anymore.


Otherwise the changes look good.

thanks,

Chris

On 6/29/20 12:21 PM, Daniel D. Daugherty wrote:

Greetings,

I have a fix for the following bug:

    JDK-8246493 JDI stress/serial/mixed002 needs to use 
WhiteBox.deflateIdleMonitors support

    https://bugs.openjdk.java.net/browse/JDK-8246493

Here's the webrev URL:

http://cr.openjdk.java.net/~dcubed/8246493-webrev/0_for_jdk16/

The test bug that's being fixed:

vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java fails
    intermittently with the following message:

 nsk.share.TestBug: There are more than one(2) instance of 
'nsk.share.jpda.StateTestThread in debuggee


Summary of the fix:

    Use WhiteBox.deflateIdleMonitors() to make sure that all 
inflated

    ObjectMonitors are deflated after each debuggee has been run.

This fix has been tested with a Mach5 Tier5 test run that 
executes all
of the JDI tests (along with JDWP, JVM/TI and other 
Serviceability tests).
I also did five 100 iteration runs of the failing mix002 test. 
Each Mach5
job set ran the test 100 times on Linux-X64, macOSX, and Win-X64 
for a
total of (5 * 100 * 3) iterations of 
nsk/jdi/stress/serial/mixed002. There

were no failures.

Thanks, in advance, for any comments, questions or suggestions.

Dan


Gory details:

The primary focus of the fix is in the first three files in the 
webrev:


test/hotspot/jtreg/vmTestbase/nsk/share/jdi/SerialExecutionDebuggee.java 

test/hotspot/jtreg/vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 


test/hotspot/jtreg/ProblemList.txt

nsk.share.jdi.SerialExecutionDebuggee is the class that used to 
serially
execute the debuggee portion of a specific list of tests. After 
this class
is done executing a debuggee class, it needs to deflate idle 
monitors in
order to prevent a StateTestThread object created by one debuggee 
class
from confusing the next debuggee class. Each of the debuggee 
classes that
use StateTestThread expect there to be only one of these objects. 
However,
since we are running multiple debuggee classes serially *in the 
same VM*,
the StateTestThread object created in one debuggee can still be 
around

when the next debuggee runs.

The COMMAND_CLEAR_DEBUGGEE implementation clears the 
currentDebuggee variable
which permits the debuggee to be GC'ed and is modified by this 
fix to call
WhiteBox.deflateIdleMonitors() to make sure that all inflated 
ObjectMonitors
are deflated after each debuggee has been run. This takes care of 
any pinned

StateTestThread objects (and any other inflated ObjectMonitors).


vmTestbase/nsk/jdi/stress/serial/mixed002 is a wrapper style 
stress test that
executes the debugger and debuggee parts of a specific list of 
tests serially
*in the same VM*. Several of the tests executed by mixed002 make 
use of the
StateTestThread class. The failure is intermittent because the 
order of test
execution is shuffled automatically and sometimes the 
ServiceThread manages
to execute deflation at the right time to prevent more than one 
StateTestThread

object from existing at the same time.

The additions to vmTestbase/nsk/jdi/stress/serial/mixed002 are 
the standard
boilerplate needed to call WhiteBox functions from test code. The 
actual call
to WhiteBox.deflateIdleMonitors() is made in 
SerialExecutionDebuggee. I did
atte

Re: RFR(S): 8246493: JDI stress/serial/mixed002 needs to use WhiteBox.deflateIdleMonitors support

2020-06-29 Thread serguei.spit...@oracle.com

+1

Thanks,
Serguei


On 6/29/20 12:49, Chris Plummer wrote:

Looks good.

Chris

On 6/29/20 12:45 PM, Daniel D. Daugherty wrote:

Chris and Serguei,

Thanks for the fast reviews!!

I generated the webrev in my "mach5" directory and that was baselined
on the jdk-16+3 snapshot and that doesn't include the ProblemList 
change.
Sigh...  I have updated the repo to "current" and regenerated the 
webrev.


test/hotspot/jtreg/ProblemList.txt  now shows:

@@ -126,11 +126,10 @@
 vmTestbase/nsk/monitoring/ThreadMXBean/ThreadInfo/Deadlock/JavaDeadlock001/TestDescription.java 
8060733 generic-all


 vmTestbase/nsk/jdi/ThreadReference/stop/stop001/TestDescription.java 
7034630 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses021/TestDescription.java 
8065773 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses023/TestDescription.java 
8065773 generic-all
-vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 
8246493 generic-all


 vmTestbase/nsk/jdb/eval/eval001/eval001.java 8221503 generic-all

 vmTestbase/metaspace/gc/firstGC_10m/TestDescription.java 8208250 
generic-all
 vmTestbase/metaspace/gc/firstGC_50m/TestDescription.java 8208250 
generic-all


Thanks again for the fast reviews!!

Dan


On 6/29/20 3:41 PM, serguei.spit...@oracle.com wrote:

Hi Dan,

The same as from Chris.
The ProblemList.txt has no changes.
Otherwise, it looks good.

Thanks,
Serguei



On 6/29/20 12:37, Chris Plummer wrote:

Hi Dan,

Something is wrong with ProblemList.txt. It doesn't show any 
changes, but I also don't see mixed002 in the file anymore.


Otherwise the changes look good.

thanks,

Chris

On 6/29/20 12:21 PM, Daniel D. Daugherty wrote:

Greetings,

I have a fix for the following bug:

    JDK-8246493 JDI stress/serial/mixed002 needs to use 
WhiteBox.deflateIdleMonitors support

    https://bugs.openjdk.java.net/browse/JDK-8246493

Here's the webrev URL:

http://cr.openjdk.java.net/~dcubed/8246493-webrev/0_for_jdk16/

The test bug that's being fixed:

vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java fails
    intermittently with the following message:

 nsk.share.TestBug: There are more than one(2) instance of 
'nsk.share.jpda.StateTestThread in debuggee


Summary of the fix:

    Use WhiteBox.deflateIdleMonitors() to make sure that all inflated
    ObjectMonitors are deflated after each debuggee has been run.

This fix has been tested with a Mach5 Tier5 test run that executes 
all
of the JDI tests (along with JDWP, JVM/TI and other Serviceability 
tests).
I also did five 100 iteration runs of the failing mix002 test. 
Each Mach5
job set ran the test 100 times on Linux-X64, macOSX, and Win-X64 
for a
total of (5 * 100 * 3) iterations of 
nsk/jdi/stress/serial/mixed002. There

were no failures.

Thanks, in advance, for any comments, questions or suggestions.

Dan


Gory details:

The primary focus of the fix is in the first three files in the 
webrev:


test/hotspot/jtreg/vmTestbase/nsk/share/jdi/SerialExecutionDebuggee.java 

test/hotspot/jtreg/vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 


test/hotspot/jtreg/ProblemList.txt

nsk.share.jdi.SerialExecutionDebuggee is the class that used to 
serially
execute the debuggee portion of a specific list of tests. After 
this class
is done executing a debuggee class, it needs to deflate idle 
monitors in
order to prevent a StateTestThread object created by one debuggee 
class
from confusing the next debuggee class. Each of the debuggee 
classes that
use StateTestThread expect there to be only one of these objects. 
However,
since we are running multiple debuggee classes serially *in the 
same VM*,
the StateTestThread object created in one debuggee can still be 
around

when the next debuggee runs.

The COMMAND_CLEAR_DEBUGGEE implementation clears the 
currentDebuggee variable
which permits the debuggee to be GC'ed and is modified by this fix 
to call
WhiteBox.deflateIdleMonitors() to make sure that all inflated 
ObjectMonitors
are deflated after each debuggee has been run. This takes care of 
any pinned

StateTestThread objects (and any other inflated ObjectMonitors).


vmTestbase/nsk/jdi/stress/serial/mixed002 is a wrapper style 
stress test that
executes the debugger and debuggee parts of a specific list of 
tests serially
*in the same VM*. Several of the tests executed by mixed002 make 
use of the
StateTestThread class. The failure is intermittent because the 
order of test
execution is shuffled automatically and sometimes the 
ServiceThread manages
to execute deflation at the right time to prevent more than one 
StateTestThread

object from existing at the same time.

The additions to vmTestbase/nsk/jdi/stress/serial/mixed002 are the 
standard
boilerplate needed to call WhiteBox functions from test code. The 
actual call
to WhiteBox.deflateIdleMonitors() is made in 
SerialExecutionDebuggee. I did
attempt a fix where I modified the StateTestThread class to make 
the call to
WhiteBox.

Re: RFR(S): 8246493: JDI stress/serial/mixed002 needs to use WhiteBox.deflateIdleMonitors support

2020-06-29 Thread Chris Plummer
Ok, in that case it sounds best not to backport. It would be best to 
make this clear in the bug so there is no future attempt to backport 
this change except to versions that have already done the 
WhiteBox.deflateIdleMonitors() backport.


thanks,

Chris

On 6/29/20 1:53 PM, Daniel D. Daugherty wrote:

The WhiteBox.deflateIdleMonitors() support is not in JDK15. That's
something that added in JDK16 so I'd have to also backport that support.
That support was included with another change (getting rid of the special
deflation request mechanism) that is not appropriate for JDK15.

Short version: I don't think we want to back port part of a patch from
JDK16 -> JDK15 in order to fix this test bug.

Dan


On 6/29/20 4:41 PM, Chris Plummer wrote:

Hi Dan,

I think you should push it directly to 15 since it's a new issue.

thanks,

Chris

On 6/29/20 12:53 PM, Daniel D. Daugherty wrote:

Chris,

Thanks. One last thing... since this is a test bug, I wasn't 
planning to
backport the fix to JDK15. The test is ProblemListed there so we 
won't see

the intermittent failures.

Are you and Serguei good with not fixing this test bug in JDK15?

Dan

P.S.
Thanks again for your sleuthing that linked the bug to my
fix for JDK-8153224.


On 6/29/20 3:49 PM, Chris Plummer wrote:

Looks good.

Chris

On 6/29/20 12:45 PM, Daniel D. Daugherty wrote:

Chris and Serguei,

Thanks for the fast reviews!!

I generated the webrev in my "mach5" directory and that was baselined
on the jdk-16+3 snapshot and that doesn't include the ProblemList 
change.
Sigh...  I have updated the repo to "current" and regenerated the 
webrev.


test/hotspot/jtreg/ProblemList.txt  now shows:

@@ -126,11 +126,10 @@
 vmTestbase/nsk/monitoring/ThreadMXBean/ThreadInfo/Deadlock/JavaDeadlock001/TestDescription.java 
8060733 generic-all


 vmTestbase/nsk/jdi/ThreadReference/stop/stop001/TestDescription.java 
7034630 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses021/TestDescription.java 
8065773 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses023/TestDescription.java 
8065773 generic-all
-vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 
8246493 generic-all


 vmTestbase/nsk/jdb/eval/eval001/eval001.java 8221503 generic-all

 vmTestbase/metaspace/gc/firstGC_10m/TestDescription.java 8208250 
generic-all
 vmTestbase/metaspace/gc/firstGC_50m/TestDescription.java 8208250 
generic-all


Thanks again for the fast reviews!!

Dan


On 6/29/20 3:41 PM, serguei.spit...@oracle.com wrote:

Hi Dan,

The same as from Chris.
The ProblemList.txt has no changes.
Otherwise, it looks good.

Thanks,
Serguei



On 6/29/20 12:37, Chris Plummer wrote:

Hi Dan,

Something is wrong with ProblemList.txt. It doesn't show any 
changes, but I also don't see mixed002 in the file anymore.


Otherwise the changes look good.

thanks,

Chris

On 6/29/20 12:21 PM, Daniel D. Daugherty wrote:

Greetings,

I have a fix for the following bug:

    JDK-8246493 JDI stress/serial/mixed002 needs to use 
WhiteBox.deflateIdleMonitors support

    https://bugs.openjdk.java.net/browse/JDK-8246493

Here's the webrev URL:

http://cr.openjdk.java.net/~dcubed/8246493-webrev/0_for_jdk16/

The test bug that's being fixed:

vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 
fails

    intermittently with the following message:

 nsk.share.TestBug: There are more than one(2) instance of 
'nsk.share.jpda.StateTestThread in debuggee


Summary of the fix:

    Use WhiteBox.deflateIdleMonitors() to make sure that all 
inflated

    ObjectMonitors are deflated after each debuggee has been run.

This fix has been tested with a Mach5 Tier5 test run that 
executes all
of the JDI tests (along with JDWP, JVM/TI and other 
Serviceability tests).
I also did five 100 iteration runs of the failing mix002 test. 
Each Mach5
job set ran the test 100 times on Linux-X64, macOSX, and 
Win-X64 for a
total of (5 * 100 * 3) iterations of 
nsk/jdi/stress/serial/mixed002. There

were no failures.

Thanks, in advance, for any comments, questions or suggestions.

Dan


Gory details:

The primary focus of the fix is in the first three files in the 
webrev:


test/hotspot/jtreg/vmTestbase/nsk/share/jdi/SerialExecutionDebuggee.java 

test/hotspot/jtreg/vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 


test/hotspot/jtreg/ProblemList.txt

nsk.share.jdi.SerialExecutionDebuggee is the class that used to 
serially
execute the debuggee portion of a specific list of tests. After 
this class
is done executing a debuggee class, it needs to deflate idle 
monitors in
order to prevent a StateTestThread object created by one 
debuggee class
from confusing the next debuggee class. Each of the debuggee 
classes that
use StateTestThread expect there to be only one of these 
objects. However,
since we are running multiple debuggee classes serially *in the 
same VM*,
the StateTestThread object created in one debuggee can still be 
around

when the next debuggee

Re: RFR(S): 8246493: JDI stress/serial/mixed002 needs to use WhiteBox.deflateIdleMonitors support

2020-06-29 Thread Daniel D. Daugherty

The WhiteBox.deflateIdleMonitors() support is not in JDK15. That's
something that added in JDK16 so I'd have to also backport that support.
That support was included with another change (getting rid of the special
deflation request mechanism) that is not appropriate for JDK15.

Short version: I don't think we want to back port part of a patch from
JDK16 -> JDK15 in order to fix this test bug.

Dan


On 6/29/20 4:41 PM, Chris Plummer wrote:

Hi Dan,

I think you should push it directly to 15 since it's a new issue.

thanks,

Chris

On 6/29/20 12:53 PM, Daniel D. Daugherty wrote:

Chris,

Thanks. One last thing... since this is a test bug, I wasn't planning to
backport the fix to JDK15. The test is ProblemListed there so we 
won't see

the intermittent failures.

Are you and Serguei good with not fixing this test bug in JDK15?

Dan

P.S.
Thanks again for your sleuthing that linked the bug to my
fix for JDK-8153224.


On 6/29/20 3:49 PM, Chris Plummer wrote:

Looks good.

Chris

On 6/29/20 12:45 PM, Daniel D. Daugherty wrote:

Chris and Serguei,

Thanks for the fast reviews!!

I generated the webrev in my "mach5" directory and that was baselined
on the jdk-16+3 snapshot and that doesn't include the ProblemList 
change.
Sigh...  I have updated the repo to "current" and regenerated the 
webrev.


test/hotspot/jtreg/ProblemList.txt  now shows:

@@ -126,11 +126,10 @@
 vmTestbase/nsk/monitoring/ThreadMXBean/ThreadInfo/Deadlock/JavaDeadlock001/TestDescription.java 
8060733 generic-all


 vmTestbase/nsk/jdi/ThreadReference/stop/stop001/TestDescription.java 
7034630 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses021/TestDescription.java 
8065773 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses023/TestDescription.java 
8065773 generic-all
-vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 
8246493 generic-all


 vmTestbase/nsk/jdb/eval/eval001/eval001.java 8221503 generic-all

 vmTestbase/metaspace/gc/firstGC_10m/TestDescription.java 8208250 
generic-all
 vmTestbase/metaspace/gc/firstGC_50m/TestDescription.java 8208250 
generic-all


Thanks again for the fast reviews!!

Dan


On 6/29/20 3:41 PM, serguei.spit...@oracle.com wrote:

Hi Dan,

The same as from Chris.
The ProblemList.txt has no changes.
Otherwise, it looks good.

Thanks,
Serguei



On 6/29/20 12:37, Chris Plummer wrote:

Hi Dan,

Something is wrong with ProblemList.txt. It doesn't show any 
changes, but I also don't see mixed002 in the file anymore.


Otherwise the changes look good.

thanks,

Chris

On 6/29/20 12:21 PM, Daniel D. Daugherty wrote:

Greetings,

I have a fix for the following bug:

    JDK-8246493 JDI stress/serial/mixed002 needs to use 
WhiteBox.deflateIdleMonitors support

    https://bugs.openjdk.java.net/browse/JDK-8246493

Here's the webrev URL:

http://cr.openjdk.java.net/~dcubed/8246493-webrev/0_for_jdk16/

The test bug that's being fixed:

vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 
fails

    intermittently with the following message:

 nsk.share.TestBug: There are more than one(2) instance of 
'nsk.share.jpda.StateTestThread in debuggee


Summary of the fix:

    Use WhiteBox.deflateIdleMonitors() to make sure that all 
inflated

    ObjectMonitors are deflated after each debuggee has been run.

This fix has been tested with a Mach5 Tier5 test run that 
executes all
of the JDI tests (along with JDWP, JVM/TI and other 
Serviceability tests).
I also did five 100 iteration runs of the failing mix002 test. 
Each Mach5
job set ran the test 100 times on Linux-X64, macOSX, and Win-X64 
for a
total of (5 * 100 * 3) iterations of 
nsk/jdi/stress/serial/mixed002. There

were no failures.

Thanks, in advance, for any comments, questions or suggestions.

Dan


Gory details:

The primary focus of the fix is in the first three files in the 
webrev:


test/hotspot/jtreg/vmTestbase/nsk/share/jdi/SerialExecutionDebuggee.java 

test/hotspot/jtreg/vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 


test/hotspot/jtreg/ProblemList.txt

nsk.share.jdi.SerialExecutionDebuggee is the class that used to 
serially
execute the debuggee portion of a specific list of tests. After 
this class
is done executing a debuggee class, it needs to deflate idle 
monitors in
order to prevent a StateTestThread object created by one 
debuggee class
from confusing the next debuggee class. Each of the debuggee 
classes that
use StateTestThread expect there to be only one of these 
objects. However,
since we are running multiple debuggee classes serially *in the 
same VM*,
the StateTestThread object created in one debuggee can still be 
around

when the next debuggee runs.

The COMMAND_CLEAR_DEBUGGEE implementation clears the 
currentDebuggee variable
which permits the debuggee to be GC'ed and is modified by this 
fix to call
WhiteBox.deflateIdleMonitors() to make sure that all inflated 
ObjectMonitors
are deflated after each debuggee has been run. This takes care 
o

Re: RFR (S): 8245129: Enhance jstat gc option output and tests

2020-06-29 Thread Hohensee, Paul
Thanks, Daniil! Pushed.

Paul

On 6/24/20, 4:07 PM, "Daniil Titov"  wrote:

Hi Paul,

The change looks good to me.

Thanks!
--Daniil


On 6/22/20, 8:48 AM, "serviceability-dev on behalf of Hohensee, Paul" 
 
wrote:

Thanks very much for review, Volker. I'll file a follow-up issue.

One more reviewer, please? :)

Paul

On 6/22/20, 8:10 AM, "serviceability-dev on behalf of Volker Simonis" 
 wrote:

Hi Paul,

thanks for fixing jstat for larger heaps. I like that you've added
explicit tests for ParallelGC which hasn't been tested since G1 was
made the default collector.

I also agree that sizes should all be right justified. I only wonder
if the header of a right justified column shouldn't be right 
justified
as well? However, taking into account that this already hasn't been
handled consistently before your change, I'm fine to postpone that 
to
a follow-up cleanup change.

I think the change looks good so thumbs up from me.

Thank you and best regards,
Volker


On Thu, Jun 18, 2020 at 11:53 PM Hohensee, Paul 
 wrote:
>
> Ping. Any takers for this simple patch?
>
>
>
> Thanks,
>
> Paul
>
>
>
> From: serviceability-dev 
 on behalf of "Hohensee, Paul" 

> Date: Monday, May 18, 2020 at 8:25 AM
> To: serviceability-dev 
> Subject: RFR (S): 8245129: Enhance jstat gc option output and 
tests
>
>
>
> Please review an enhancement to the jstat gc option output to 
make the columns wider (for up to a 2TB heap) so one can read the output 
without going cross-eyed.
>
>
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8245129
>
> Webrev: http://cr.openjdk.java.net/~phh/8245129/webrev.00/
>
>
>
> I added tests using ParallelGC since the output can differ for 
non-G1 collectors. Successfully ran the 
test/hotspot/jtreg/serviceability/tmtools/jstat and test/jdk/sun/tools/jstat 
tests. A submit repo run had one failure
>
>
>
> runtime/MemberName/MemberNameLeak.java
>
> tier1
>
> macosx-x64-debug
>
>
>
> but rerunning it on my laptop succeeded, and there’s no 
connection between this test and my patch.
>
>
>
> Thanks,
>
> Paul
>
>
>
>







Re: RFR(S): 8246493: JDI stress/serial/mixed002 needs to use WhiteBox.deflateIdleMonitors support

2020-06-29 Thread Chris Plummer

Hi Dan,

I think you should push it directly to 15 since it's a new issue.

thanks,

Chris

On 6/29/20 12:53 PM, Daniel D. Daugherty wrote:

Chris,

Thanks. One last thing... since this is a test bug, I wasn't planning to
backport the fix to JDK15. The test is ProblemListed there so we won't 
see

the intermittent failures.

Are you and Serguei good with not fixing this test bug in JDK15?

Dan

P.S.
Thanks again for your sleuthing that linked the bug to my
fix for JDK-8153224.


On 6/29/20 3:49 PM, Chris Plummer wrote:

Looks good.

Chris

On 6/29/20 12:45 PM, Daniel D. Daugherty wrote:

Chris and Serguei,

Thanks for the fast reviews!!

I generated the webrev in my "mach5" directory and that was baselined
on the jdk-16+3 snapshot and that doesn't include the ProblemList 
change.
Sigh...  I have updated the repo to "current" and regenerated the 
webrev.


test/hotspot/jtreg/ProblemList.txt  now shows:

@@ -126,11 +126,10 @@
 vmTestbase/nsk/monitoring/ThreadMXBean/ThreadInfo/Deadlock/JavaDeadlock001/TestDescription.java 
8060733 generic-all


 vmTestbase/nsk/jdi/ThreadReference/stop/stop001/TestDescription.java 
7034630 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses021/TestDescription.java 
8065773 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses023/TestDescription.java 
8065773 generic-all
-vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 
8246493 generic-all


 vmTestbase/nsk/jdb/eval/eval001/eval001.java 8221503 generic-all

 vmTestbase/metaspace/gc/firstGC_10m/TestDescription.java 8208250 
generic-all
 vmTestbase/metaspace/gc/firstGC_50m/TestDescription.java 8208250 
generic-all


Thanks again for the fast reviews!!

Dan


On 6/29/20 3:41 PM, serguei.spit...@oracle.com wrote:

Hi Dan,

The same as from Chris.
The ProblemList.txt has no changes.
Otherwise, it looks good.

Thanks,
Serguei



On 6/29/20 12:37, Chris Plummer wrote:

Hi Dan,

Something is wrong with ProblemList.txt. It doesn't show any 
changes, but I also don't see mixed002 in the file anymore.


Otherwise the changes look good.

thanks,

Chris

On 6/29/20 12:21 PM, Daniel D. Daugherty wrote:

Greetings,

I have a fix for the following bug:

    JDK-8246493 JDI stress/serial/mixed002 needs to use 
WhiteBox.deflateIdleMonitors support

    https://bugs.openjdk.java.net/browse/JDK-8246493

Here's the webrev URL:

http://cr.openjdk.java.net/~dcubed/8246493-webrev/0_for_jdk16/

The test bug that's being fixed:

vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java fails
    intermittently with the following message:

 nsk.share.TestBug: There are more than one(2) instance of 
'nsk.share.jpda.StateTestThread in debuggee


Summary of the fix:

    Use WhiteBox.deflateIdleMonitors() to make sure that all 
inflated

    ObjectMonitors are deflated after each debuggee has been run.

This fix has been tested with a Mach5 Tier5 test run that 
executes all
of the JDI tests (along with JDWP, JVM/TI and other 
Serviceability tests).
I also did five 100 iteration runs of the failing mix002 test. 
Each Mach5
job set ran the test 100 times on Linux-X64, macOSX, and Win-X64 
for a
total of (5 * 100 * 3) iterations of 
nsk/jdi/stress/serial/mixed002. There

were no failures.

Thanks, in advance, for any comments, questions or suggestions.

Dan


Gory details:

The primary focus of the fix is in the first three files in the 
webrev:


test/hotspot/jtreg/vmTestbase/nsk/share/jdi/SerialExecutionDebuggee.java 

test/hotspot/jtreg/vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 


test/hotspot/jtreg/ProblemList.txt

nsk.share.jdi.SerialExecutionDebuggee is the class that used to 
serially
execute the debuggee portion of a specific list of tests. After 
this class
is done executing a debuggee class, it needs to deflate idle 
monitors in
order to prevent a StateTestThread object created by one debuggee 
class
from confusing the next debuggee class. Each of the debuggee 
classes that
use StateTestThread expect there to be only one of these objects. 
However,
since we are running multiple debuggee classes serially *in the 
same VM*,
the StateTestThread object created in one debuggee can still be 
around

when the next debuggee runs.

The COMMAND_CLEAR_DEBUGGEE implementation clears the 
currentDebuggee variable
which permits the debuggee to be GC'ed and is modified by this 
fix to call
WhiteBox.deflateIdleMonitors() to make sure that all inflated 
ObjectMonitors
are deflated after each debuggee has been run. This takes care of 
any pinned

StateTestThread objects (and any other inflated ObjectMonitors).


vmTestbase/nsk/jdi/stress/serial/mixed002 is a wrapper style 
stress test that
executes the debugger and debuggee parts of a specific list of 
tests serially
*in the same VM*. Several of the tests executed by mixed002 make 
use of the
StateTestThread class. The failure is intermittent because the 
order of test
execution is shuffled automatically and sometimes the

Re: RFR(S): 8246493: JDI stress/serial/mixed002 needs to use WhiteBox.deflateIdleMonitors support

2020-06-29 Thread Daniel D. Daugherty

Chris,

Thanks. One last thing... since this is a test bug, I wasn't planning to
backport the fix to JDK15. The test is ProblemListed there so we won't see
the intermittent failures.

Are you and Serguei good with not fixing this test bug in JDK15?

Dan

P.S.
Thanks again for your sleuthing that linked the bug to my
fix for JDK-8153224.


On 6/29/20 3:49 PM, Chris Plummer wrote:

Looks good.

Chris

On 6/29/20 12:45 PM, Daniel D. Daugherty wrote:

Chris and Serguei,

Thanks for the fast reviews!!

I generated the webrev in my "mach5" directory and that was baselined
on the jdk-16+3 snapshot and that doesn't include the ProblemList 
change.
Sigh...  I have updated the repo to "current" and regenerated the 
webrev.


test/hotspot/jtreg/ProblemList.txt  now shows:

@@ -126,11 +126,10 @@
 vmTestbase/nsk/monitoring/ThreadMXBean/ThreadInfo/Deadlock/JavaDeadlock001/TestDescription.java 
8060733 generic-all


 vmTestbase/nsk/jdi/ThreadReference/stop/stop001/TestDescription.java 
7034630 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses021/TestDescription.java 
8065773 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses023/TestDescription.java 
8065773 generic-all
-vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 
8246493 generic-all


 vmTestbase/nsk/jdb/eval/eval001/eval001.java 8221503 generic-all

 vmTestbase/metaspace/gc/firstGC_10m/TestDescription.java 8208250 
generic-all
 vmTestbase/metaspace/gc/firstGC_50m/TestDescription.java 8208250 
generic-all


Thanks again for the fast reviews!!

Dan


On 6/29/20 3:41 PM, serguei.spit...@oracle.com wrote:

Hi Dan,

The same as from Chris.
The ProblemList.txt has no changes.
Otherwise, it looks good.

Thanks,
Serguei



On 6/29/20 12:37, Chris Plummer wrote:

Hi Dan,

Something is wrong with ProblemList.txt. It doesn't show any 
changes, but I also don't see mixed002 in the file anymore.


Otherwise the changes look good.

thanks,

Chris

On 6/29/20 12:21 PM, Daniel D. Daugherty wrote:

Greetings,

I have a fix for the following bug:

    JDK-8246493 JDI stress/serial/mixed002 needs to use 
WhiteBox.deflateIdleMonitors support

    https://bugs.openjdk.java.net/browse/JDK-8246493

Here's the webrev URL:

http://cr.openjdk.java.net/~dcubed/8246493-webrev/0_for_jdk16/

The test bug that's being fixed:

vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java fails
    intermittently with the following message:

 nsk.share.TestBug: There are more than one(2) instance of 
'nsk.share.jpda.StateTestThread in debuggee


Summary of the fix:

    Use WhiteBox.deflateIdleMonitors() to make sure that all inflated
    ObjectMonitors are deflated after each debuggee has been run.

This fix has been tested with a Mach5 Tier5 test run that executes 
all
of the JDI tests (along with JDWP, JVM/TI and other Serviceability 
tests).
I also did five 100 iteration runs of the failing mix002 test. 
Each Mach5
job set ran the test 100 times on Linux-X64, macOSX, and Win-X64 
for a
total of (5 * 100 * 3) iterations of 
nsk/jdi/stress/serial/mixed002. There

were no failures.

Thanks, in advance, for any comments, questions or suggestions.

Dan


Gory details:

The primary focus of the fix is in the first three files in the 
webrev:


test/hotspot/jtreg/vmTestbase/nsk/share/jdi/SerialExecutionDebuggee.java 

test/hotspot/jtreg/vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 


test/hotspot/jtreg/ProblemList.txt

nsk.share.jdi.SerialExecutionDebuggee is the class that used to 
serially
execute the debuggee portion of a specific list of tests. After 
this class
is done executing a debuggee class, it needs to deflate idle 
monitors in
order to prevent a StateTestThread object created by one debuggee 
class
from confusing the next debuggee class. Each of the debuggee 
classes that
use StateTestThread expect there to be only one of these objects. 
However,
since we are running multiple debuggee classes serially *in the 
same VM*,
the StateTestThread object created in one debuggee can still be 
around

when the next debuggee runs.

The COMMAND_CLEAR_DEBUGGEE implementation clears the 
currentDebuggee variable
which permits the debuggee to be GC'ed and is modified by this fix 
to call
WhiteBox.deflateIdleMonitors() to make sure that all inflated 
ObjectMonitors
are deflated after each debuggee has been run. This takes care of 
any pinned

StateTestThread objects (and any other inflated ObjectMonitors).


vmTestbase/nsk/jdi/stress/serial/mixed002 is a wrapper style 
stress test that
executes the debugger and debuggee parts of a specific list of 
tests serially
*in the same VM*. Several of the tests executed by mixed002 make 
use of the
StateTestThread class. The failure is intermittent because the 
order of test
execution is shuffled automatically and sometimes the 
ServiceThread manages
to execute deflation at the right time to prevent more than one 
StateTestThread

object from existing at the same time.

RFR: 8205467: javax/management/remote/mandatory/connection/MultiThreadDeadLockTest.java possible deadlock

2020-06-29 Thread Daniil Titov
Please review a tiny change that adjusts the wait timeout the test uses for 
"test.timeout.factor" system property.

Please note that a trivial merge with fix [4] that is currently on review [3] 
will be required. Since issues [2] and [4] describe different problems I 
decided to not combine these both changes in the single fix.

Testing: Mach5 tests tier1-tier3 successfully passed.

[1] Web rev:  https://cr.openjdk.java.net/~dtitov/8205467/webrev.01/ 
[2] Jira issue:  https://bugs.openjdk.java.net/browse/JDK-8205467 
[3] 
https://mail.openjdk.java.net/pipermail/serviceability-dev/2020-June/032098.html
 
[4] https://bugs.openjdk.java.net/browse/JDK-8227337 

Thank you,
Daniil




Re: [15] RFR(XXS): 7107012: sun.jvm.hostspot.code.CompressedReadStream readDouble() conversion to long mishandled

2020-06-29 Thread Daniel D. Daugherty

Hi Chris,

This one caught my eye just because it was such an old bug number... :-)


On 6/26/20 7:03 PM, Chris Plummer wrote:

Hello,

Please help review the following:

http://cr.openjdk.java.net/~cjplummer/7107012/webrev.00/index.html


src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/CompressedReadStream.java
    Nice catch!

Thumbs up.

Dan



https://bugs.openjdk.java.net/browse/JDK-7107012

This bug is filed as confidential, although the issue is trivial. In 
the following line of code:


    return Double.longBitsToDouble((h << 32) | ((long)l & 
0xL));


Since h is an int, it's subject to the following:

https://docs.oracle.com/javase/specs/jls/se14/html/jls-15.html#jls-15.19

"If the promoted type of the left-hand operand is int, then only the 
five lowest-order bits of the right-hand operand are used as the shift 
distance. It is as if the right-hand operand were subjected to a 
bitwise logical AND operator & (§15.22.1) with the mask value 0x1f 
(0b1). The shift distance actually used is therefore always in the 
range 0 to 31, inclusive."


So (h << 32) is the same as (h << 0), which is not what was intended. 
The spec also calls out another issue:


"The type of the shift expression is the promoted type of the 
left-hand operand."


So even if it did left shift 32 bits, the result would have been 
truncated to an int, meaning the result would always be 0. The fix is 
to first cast h to a long. Doing this addresses both these problems, 
allowing a full 32 bit left shift to be done, and leaving the result 
as an untruncated long.


I was unable to trigger use of this code in SA. It seems to be used to 
pull locals out of a CompiledVFrame. I don't see any clhsdb paths to 
this code. It appears the GUI hsdb uses it via a complex call path I 
could not fully decipher, but I could not trigger its use from hsdb. 
In any case, the fix is straight forward and trivial, so I'd rather 
not have to spend more time digging deeper into its use and providing 
a test case.


thanks,

Chris





Re: RFR(S): 8246493: JDI stress/serial/mixed002 needs to use WhiteBox.deflateIdleMonitors support

2020-06-29 Thread Chris Plummer

Looks good.

Chris

On 6/29/20 12:45 PM, Daniel D. Daugherty wrote:

Chris and Serguei,

Thanks for the fast reviews!!

I generated the webrev in my "mach5" directory and that was baselined
on the jdk-16+3 snapshot and that doesn't include the ProblemList change.
Sigh...  I have updated the repo to "current" and regenerated the webrev.

test/hotspot/jtreg/ProblemList.txt  now shows:

@@ -126,11 +126,10 @@
 vmTestbase/nsk/monitoring/ThreadMXBean/ThreadInfo/Deadlock/JavaDeadlock001/TestDescription.java 
8060733 generic-all


 vmTestbase/nsk/jdi/ThreadReference/stop/stop001/TestDescription.java 
7034630 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses021/TestDescription.java 
8065773 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses023/TestDescription.java 
8065773 generic-all
-vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 
8246493 generic-all


 vmTestbase/nsk/jdb/eval/eval001/eval001.java 8221503 generic-all

 vmTestbase/metaspace/gc/firstGC_10m/TestDescription.java 8208250 
generic-all
 vmTestbase/metaspace/gc/firstGC_50m/TestDescription.java 8208250 
generic-all


Thanks again for the fast reviews!!

Dan


On 6/29/20 3:41 PM, serguei.spit...@oracle.com wrote:

Hi Dan,

The same as from Chris.
The ProblemList.txt has no changes.
Otherwise, it looks good.

Thanks,
Serguei



On 6/29/20 12:37, Chris Plummer wrote:

Hi Dan,

Something is wrong with ProblemList.txt. It doesn't show any 
changes, but I also don't see mixed002 in the file anymore.


Otherwise the changes look good.

thanks,

Chris

On 6/29/20 12:21 PM, Daniel D. Daugherty wrote:

Greetings,

I have a fix for the following bug:

    JDK-8246493 JDI stress/serial/mixed002 needs to use 
WhiteBox.deflateIdleMonitors support

    https://bugs.openjdk.java.net/browse/JDK-8246493

Here's the webrev URL:

http://cr.openjdk.java.net/~dcubed/8246493-webrev/0_for_jdk16/

The test bug that's being fixed:

vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java fails
    intermittently with the following message:

 nsk.share.TestBug: There are more than one(2) instance of 
'nsk.share.jpda.StateTestThread in debuggee


Summary of the fix:

    Use WhiteBox.deflateIdleMonitors() to make sure that all inflated
    ObjectMonitors are deflated after each debuggee has been run.

This fix has been tested with a Mach5 Tier5 test run that executes all
of the JDI tests (along with JDWP, JVM/TI and other Serviceability 
tests).
I also did five 100 iteration runs of the failing mix002 test. Each 
Mach5

job set ran the test 100 times on Linux-X64, macOSX, and Win-X64 for a
total of (5 * 100 * 3) iterations of 
nsk/jdi/stress/serial/mixed002. There

were no failures.

Thanks, in advance, for any comments, questions or suggestions.

Dan


Gory details:

The primary focus of the fix is in the first three files in the 
webrev:


test/hotspot/jtreg/vmTestbase/nsk/share/jdi/SerialExecutionDebuggee.java 

test/hotspot/jtreg/vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 


test/hotspot/jtreg/ProblemList.txt

nsk.share.jdi.SerialExecutionDebuggee is the class that used to 
serially
execute the debuggee portion of a specific list of tests. After 
this class
is done executing a debuggee class, it needs to deflate idle 
monitors in
order to prevent a StateTestThread object created by one debuggee 
class
from confusing the next debuggee class. Each of the debuggee 
classes that
use StateTestThread expect there to be only one of these objects. 
However,
since we are running multiple debuggee classes serially *in the 
same VM*,

the StateTestThread object created in one debuggee can still be around
when the next debuggee runs.

The COMMAND_CLEAR_DEBUGGEE implementation clears the 
currentDebuggee variable
which permits the debuggee to be GC'ed and is modified by this fix 
to call
WhiteBox.deflateIdleMonitors() to make sure that all inflated 
ObjectMonitors
are deflated after each debuggee has been run. This takes care of 
any pinned

StateTestThread objects (and any other inflated ObjectMonitors).


vmTestbase/nsk/jdi/stress/serial/mixed002 is a wrapper style stress 
test that
executes the debugger and debuggee parts of a specific list of 
tests serially
*in the same VM*. Several of the tests executed by mixed002 make 
use of the
StateTestThread class. The failure is intermittent because the 
order of test
execution is shuffled automatically and sometimes the ServiceThread 
manages
to execute deflation at the right time to prevent more than one 
StateTestThread

object from existing at the same time.

The additions to vmTestbase/nsk/jdi/stress/serial/mixed002 are the 
standard
boilerplate needed to call WhiteBox functions from test code. The 
actual call
to WhiteBox.deflateIdleMonitors() is made in 
SerialExecutionDebuggee. I did
attempt a fix where I modified the StateTestThread class to make 
the call to
WhiteBox.deflateIdleMonitors() after the internal waitOnObject is 
no longer

Re: RFR(S): 8246493: JDI stress/serial/mixed002 needs to use WhiteBox.deflateIdleMonitors support

2020-06-29 Thread Daniel D. Daugherty

Chris and Serguei,

Thanks for the fast reviews!!

I generated the webrev in my "mach5" directory and that was baselined
on the jdk-16+3 snapshot and that doesn't include the ProblemList change.
Sigh...  I have updated the repo to "current" and regenerated the webrev.

test/hotspot/jtreg/ProblemList.txt  now shows:

@@ -126,11 +126,10 @@
 
vmTestbase/nsk/monitoring/ThreadMXBean/ThreadInfo/Deadlock/JavaDeadlock001/TestDescription.java
 8060733 generic-all

 vmTestbase/nsk/jdi/ThreadReference/stop/stop001/TestDescription.java 
7034630 generic-all

 
vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses021/TestDescription.java
 8065773 generic-all
 
vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses023/TestDescription.java
 8065773 generic-all
-vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 8246493 
generic-all


 vmTestbase/nsk/jdb/eval/eval001/eval001.java 8221503 generic-all

 vmTestbase/metaspace/gc/firstGC_10m/TestDescription.java 8208250 
generic-all
 vmTestbase/metaspace/gc/firstGC_50m/TestDescription.java 8208250 
generic-all


Thanks again for the fast reviews!!

Dan


On 6/29/20 3:41 PM, serguei.spit...@oracle.com wrote:

Hi Dan,

The same as from Chris.
The ProblemList.txt has no changes.
Otherwise, it looks good.

Thanks,
Serguei



On 6/29/20 12:37, Chris Plummer wrote:

Hi Dan,

Something is wrong with ProblemList.txt. It doesn't show any changes, 
but I also don't see mixed002 in the file anymore.


Otherwise the changes look good.

thanks,

Chris

On 6/29/20 12:21 PM, Daniel D. Daugherty wrote:

Greetings,

I have a fix for the following bug:

    JDK-8246493 JDI stress/serial/mixed002 needs to use 
WhiteBox.deflateIdleMonitors support

    https://bugs.openjdk.java.net/browse/JDK-8246493

Here's the webrev URL:

http://cr.openjdk.java.net/~dcubed/8246493-webrev/0_for_jdk16/

The test bug that's being fixed:

vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java fails
    intermittently with the following message:

 nsk.share.TestBug: There are more than one(2) instance of 
'nsk.share.jpda.StateTestThread in debuggee


Summary of the fix:

    Use WhiteBox.deflateIdleMonitors() to make sure that all inflated
    ObjectMonitors are deflated after each debuggee has been run.

This fix has been tested with a Mach5 Tier5 test run that executes all
of the JDI tests (along with JDWP, JVM/TI and other Serviceability 
tests).
I also did five 100 iteration runs of the failing mix002 test. Each 
Mach5

job set ran the test 100 times on Linux-X64, macOSX, and Win-X64 for a
total of (5 * 100 * 3) iterations of nsk/jdi/stress/serial/mixed002. 
There

were no failures.

Thanks, in advance, for any comments, questions or suggestions.

Dan


Gory details:

The primary focus of the fix is in the first three files in the webrev:

test/hotspot/jtreg/vmTestbase/nsk/share/jdi/SerialExecutionDebuggee.java 

test/hotspot/jtreg/vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 


test/hotspot/jtreg/ProblemList.txt

nsk.share.jdi.SerialExecutionDebuggee is the class that used to 
serially
execute the debuggee portion of a specific list of tests. After this 
class
is done executing a debuggee class, it needs to deflate idle 
monitors in

order to prevent a StateTestThread object created by one debuggee class
from confusing the next debuggee class. Each of the debuggee classes 
that
use StateTestThread expect there to be only one of these objects. 
However,
since we are running multiple debuggee classes serially *in the same 
VM*,

the StateTestThread object created in one debuggee can still be around
when the next debuggee runs.

The COMMAND_CLEAR_DEBUGGEE implementation clears the currentDebuggee 
variable
which permits the debuggee to be GC'ed and is modified by this fix 
to call
WhiteBox.deflateIdleMonitors() to make sure that all inflated 
ObjectMonitors
are deflated after each debuggee has been run. This takes care of 
any pinned

StateTestThread objects (and any other inflated ObjectMonitors).


vmTestbase/nsk/jdi/stress/serial/mixed002 is a wrapper style stress 
test that
executes the debugger and debuggee parts of a specific list of tests 
serially
*in the same VM*. Several of the tests executed by mixed002 make use 
of the
StateTestThread class. The failure is intermittent because the order 
of test
execution is shuffled automatically and sometimes the ServiceThread 
manages
to execute deflation at the right time to prevent more than one 
StateTestThread

object from existing at the same time.

The additions to vmTestbase/nsk/jdi/stress/serial/mixed002 are the 
standard
boilerplate needed to call WhiteBox functions from test code. The 
actual call
to WhiteBox.deflateIdleMonitors() is made in 
SerialExecutionDebuggee. I did
attempt a fix where I modified the StateTestThread class to make the 
call to
WhiteBox.deflateIdleMonitors() after the internal waitOnObject is no 
longer
contended or waited on. That fix reduced the frequency of the 
fail

Re: RFR(S): 8246493: JDI stress/serial/mixed002 needs to use WhiteBox.deflateIdleMonitors support

2020-06-29 Thread serguei.spit...@oracle.com

Hi Dan,

The same as from Chris.
The ProblemList.txt has no changes.
Otherwise, it looks good.

Thanks,
Serguei



On 6/29/20 12:37, Chris Plummer wrote:

Hi Dan,

Something is wrong with ProblemList.txt. It doesn't show any changes, 
but I also don't see mixed002 in the file anymore.


Otherwise the changes look good.

thanks,

Chris

On 6/29/20 12:21 PM, Daniel D. Daugherty wrote:

Greetings,

I have a fix for the following bug:

    JDK-8246493 JDI stress/serial/mixed002 needs to use 
WhiteBox.deflateIdleMonitors support

    https://bugs.openjdk.java.net/browse/JDK-8246493

Here's the webrev URL:

http://cr.openjdk.java.net/~dcubed/8246493-webrev/0_for_jdk16/

The test bug that's being fixed:

vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java fails
    intermittently with the following message:

 nsk.share.TestBug: There are more than one(2) instance of 
'nsk.share.jpda.StateTestThread in debuggee


Summary of the fix:

    Use WhiteBox.deflateIdleMonitors() to make sure that all inflated
    ObjectMonitors are deflated after each debuggee has been run.

This fix has been tested with a Mach5 Tier5 test run that executes all
of the JDI tests (along with JDWP, JVM/TI and other Serviceability 
tests).
I also did five 100 iteration runs of the failing mix002 test. Each 
Mach5

job set ran the test 100 times on Linux-X64, macOSX, and Win-X64 for a
total of (5 * 100 * 3) iterations of nsk/jdi/stress/serial/mixed002. 
There

were no failures.

Thanks, in advance, for any comments, questions or suggestions.

Dan


Gory details:

The primary focus of the fix is in the first three files in the webrev:

test/hotspot/jtreg/vmTestbase/nsk/share/jdi/SerialExecutionDebuggee.java
test/hotspot/jtreg/vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 


test/hotspot/jtreg/ProblemList.txt

nsk.share.jdi.SerialExecutionDebuggee is the class that used to serially
execute the debuggee portion of a specific list of tests. After this 
class

is done executing a debuggee class, it needs to deflate idle monitors in
order to prevent a StateTestThread object created by one debuggee class
from confusing the next debuggee class. Each of the debuggee classes 
that
use StateTestThread expect there to be only one of these objects. 
However,
since we are running multiple debuggee classes serially *in the same 
VM*,

the StateTestThread object created in one debuggee can still be around
when the next debuggee runs.

The COMMAND_CLEAR_DEBUGGEE implementation clears the currentDebuggee 
variable
which permits the debuggee to be GC'ed and is modified by this fix to 
call
WhiteBox.deflateIdleMonitors() to make sure that all inflated 
ObjectMonitors
are deflated after each debuggee has been run. This takes care of any 
pinned

StateTestThread objects (and any other inflated ObjectMonitors).


vmTestbase/nsk/jdi/stress/serial/mixed002 is a wrapper style stress 
test that
executes the debugger and debuggee parts of a specific list of tests 
serially
*in the same VM*. Several of the tests executed by mixed002 make use 
of the
StateTestThread class. The failure is intermittent because the order 
of test
execution is shuffled automatically and sometimes the ServiceThread 
manages
to execute deflation at the right time to prevent more than one 
StateTestThread

object from existing at the same time.

The additions to vmTestbase/nsk/jdi/stress/serial/mixed002 are the 
standard
boilerplate needed to call WhiteBox functions from test code. The 
actual call
to WhiteBox.deflateIdleMonitors() is made in SerialExecutionDebuggee. 
I did
attempt a fix where I modified the StateTestThread class to make the 
call to
WhiteBox.deflateIdleMonitors() after the internal waitOnObject is no 
longer
contended or waited on. That fix reduced the frequency of the 
failures by
about half, but it didn't solve the test bug entirely. So I had to 
make the

fix in SerialExecutionDebuggee instead.


test/hotspot/jtreg/ProblemList.txt is modified to re-enable the 
mix002 test.



The remaining nine files are also wrapper style stress tests that 
execute

the debugger and debuggee parts of a specific list of tests serially *in
the same VM*. Because these tests also use SerialExecutionDebuggee, they
also need the boilerplate changes so that WhiteBox.deflateIdleMonitors()
can be called.







RFR: 8227337: javax/management/remote/mandatory/connection/ReconnectTest.java NoSuchObjectException no such object in table

2020-06-29 Thread Daniil Titov
Please review the change that fixes an intermittent tests failure.

The tests javax/management/remote/mandatory/connection/ReconnectTest.java and
 javax/management/remote/mandatory/connection/MultiThreadDeadLockTest.java  use 
specific settings for server timeout that in some cases  (e.g. when the test is 
run with -Xcomp)
 result in  JMX server connection timeout thread unexports the remote object 
while the client
 connection is still in the progress.  Below is an example of a such stacktrace:

java.rmi.NoSuchObjectException: no such object in table
at 
java.rmi/sun.rmi.transport.StreamRemoteCall.exceptionReceivedFromServer(StreamRemoteCall.java:303)
at 
java.rmi/sun.rmi.transport.StreamRemoteCall.executeCall(StreamRemoteCall.java:279)
at java.rmi/sun.rmi.server.UnicastRef.invoke(UnicastRef.java:164)
at jdk.remoteref/jdk.jmx.remote.internal.rmi.PRef.invoke(Unknown Source)
at 
java.management.rmi/javax.management.remote.rmi.RMIConnectionImpl_Stub.getConnectionId(RMIConnectionImpl_Stub.java:318)
at 
java.management.rmi/javax.management.remote.rmi.RMIConnector.getConnectionId(RMIConnector.java:385)
at 
java.management.rmi/javax.management.remote.rmi.RMIConnector.connect(RMIConnector.java:347)
at 
java.management/javax.management.remote.JMXConnectorFactory.connect(JMXConnectorFactory.java:270)
at MultiThreadDeadLockTest.main(MultiThreadDeadLockTest.java:87)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64)
at 
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:564)
at 
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
at java.base/java.lang.Thread.run(Thread.java:832)



The fix adjusts the server timeout  the tests use for "test.timeout.factor" 
system property.

Testing: Mach5 tests are in the progress.

[1] https://cr.openjdk.java.net/~dtitov/8227337/webrev.01/ 
[2] https://bugs.openjdk.java.net/browse/JDK-8227337 

Thanks,
Daniil





Re: RFR(S): 8246493: JDI stress/serial/mixed002 needs to use WhiteBox.deflateIdleMonitors support

2020-06-29 Thread Chris Plummer

Hi Dan,

Something is wrong with ProblemList.txt. It doesn't show any changes, 
but I also don't see mixed002 in the file anymore.


Otherwise the changes look good.

thanks,

Chris

On 6/29/20 12:21 PM, Daniel D. Daugherty wrote:

Greetings,

I have a fix for the following bug:

    JDK-8246493 JDI stress/serial/mixed002 needs to use 
WhiteBox.deflateIdleMonitors support

    https://bugs.openjdk.java.net/browse/JDK-8246493

Here's the webrev URL:

http://cr.openjdk.java.net/~dcubed/8246493-webrev/0_for_jdk16/

The test bug that's being fixed:

    vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java fails
    intermittently with the following message:

 nsk.share.TestBug: There are more than one(2) instance of 
'nsk.share.jpda.StateTestThread in debuggee


Summary of the fix:

    Use WhiteBox.deflateIdleMonitors() to make sure that all inflated
    ObjectMonitors are deflated after each debuggee has been run.

This fix has been tested with a Mach5 Tier5 test run that executes all
of the JDI tests (along with JDWP, JVM/TI and other Serviceability 
tests).

I also did five 100 iteration runs of the failing mix002 test. Each Mach5
job set ran the test 100 times on Linux-X64, macOSX, and Win-X64 for a
total of (5 * 100 * 3) iterations of nsk/jdi/stress/serial/mixed002. 
There

were no failures.

Thanks, in advance, for any comments, questions or suggestions.

Dan


Gory details:

The primary focus of the fix is in the first three files in the webrev:

test/hotspot/jtreg/vmTestbase/nsk/share/jdi/SerialExecutionDebuggee.java
test/hotspot/jtreg/vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java 


test/hotspot/jtreg/ProblemList.txt

nsk.share.jdi.SerialExecutionDebuggee is the class that used to serially
execute the debuggee portion of a specific list of tests. After this 
class

is done executing a debuggee class, it needs to deflate idle monitors in
order to prevent a StateTestThread object created by one debuggee class
from confusing the next debuggee class. Each of the debuggee classes that
use StateTestThread expect there to be only one of these objects. 
However,

since we are running multiple debuggee classes serially *in the same VM*,
the StateTestThread object created in one debuggee can still be around
when the next debuggee runs.

The COMMAND_CLEAR_DEBUGGEE implementation clears the currentDebuggee 
variable
which permits the debuggee to be GC'ed and is modified by this fix to 
call
WhiteBox.deflateIdleMonitors() to make sure that all inflated 
ObjectMonitors
are deflated after each debuggee has been run. This takes care of any 
pinned

StateTestThread objects (and any other inflated ObjectMonitors).


vmTestbase/nsk/jdi/stress/serial/mixed002 is a wrapper style stress 
test that
executes the debugger and debuggee parts of a specific list of tests 
serially
*in the same VM*. Several of the tests executed by mixed002 make use 
of the
StateTestThread class. The failure is intermittent because the order 
of test
execution is shuffled automatically and sometimes the ServiceThread 
manages
to execute deflation at the right time to prevent more than one 
StateTestThread

object from existing at the same time.

The additions to vmTestbase/nsk/jdi/stress/serial/mixed002 are the 
standard
boilerplate needed to call WhiteBox functions from test code. The 
actual call
to WhiteBox.deflateIdleMonitors() is made in SerialExecutionDebuggee. 
I did
attempt a fix where I modified the StateTestThread class to make the 
call to
WhiteBox.deflateIdleMonitors() after the internal waitOnObject is no 
longer

contended or waited on. That fix reduced the frequency of the failures by
about half, but it didn't solve the test bug entirely. So I had to 
make the

fix in SerialExecutionDebuggee instead.


test/hotspot/jtreg/ProblemList.txt is modified to re-enable the mix002 
test.



The remaining nine files are also wrapper style stress tests that execute
the debugger and debuggee parts of a specific list of tests serially *in
the same VM*. Because these tests also use SerialExecutionDebuggee, they
also need the boilerplate changes so that WhiteBox.deflateIdleMonitors()
can be called.





RFR(S): 8246493: JDI stress/serial/mixed002 needs to use WhiteBox.deflateIdleMonitors support

2020-06-29 Thread Daniel D. Daugherty

Greetings,

I have a fix for the following bug:

    JDK-8246493 JDI stress/serial/mixed002 needs to use 
WhiteBox.deflateIdleMonitors support

    https://bugs.openjdk.java.net/browse/JDK-8246493

Here's the webrev URL:

http://cr.openjdk.java.net/~dcubed/8246493-webrev/0_for_jdk16/

The test bug that's being fixed:

    vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java fails
    intermittently with the following message:

 nsk.share.TestBug: There are more than one(2) instance of 
'nsk.share.jpda.StateTestThread in debuggee


Summary of the fix:

    Use WhiteBox.deflateIdleMonitors() to make sure that all inflated
    ObjectMonitors are deflated after each debuggee has been run.

This fix has been tested with a Mach5 Tier5 test run that executes all
of the JDI tests (along with JDWP, JVM/TI and other Serviceability tests).
I also did five 100 iteration runs of the failing mix002 test. Each Mach5
job set ran the test 100 times on Linux-X64, macOSX, and Win-X64 for a
total of (5 * 100 * 3) iterations of nsk/jdi/stress/serial/mixed002. There
were no failures.

Thanks, in advance, for any comments, questions or suggestions.

Dan


Gory details:

The primary focus of the fix is in the first three files in the webrev:

test/hotspot/jtreg/vmTestbase/nsk/share/jdi/SerialExecutionDebuggee.java
test/hotspot/jtreg/vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java
test/hotspot/jtreg/ProblemList.txt

nsk.share.jdi.SerialExecutionDebuggee is the class that used to serially
execute the debuggee portion of a specific list of tests. After this class
is done executing a debuggee class, it needs to deflate idle monitors in
order to prevent a StateTestThread object created by one debuggee class
from confusing the next debuggee class. Each of the debuggee classes that
use StateTestThread expect there to be only one of these objects. However,
since we are running multiple debuggee classes serially *in the same VM*,
the StateTestThread object created in one debuggee can still be around
when the next debuggee runs.

The COMMAND_CLEAR_DEBUGGEE implementation clears the currentDebuggee 
variable

which permits the debuggee to be GC'ed and is modified by this fix to call
WhiteBox.deflateIdleMonitors() to make sure that all inflated ObjectMonitors
are deflated after each debuggee has been run. This takes care of any pinned
StateTestThread objects (and any other inflated ObjectMonitors).


vmTestbase/nsk/jdi/stress/serial/mixed002 is a wrapper style stress test 
that
executes the debugger and debuggee parts of a specific list of tests 
serially

*in the same VM*. Several of the tests executed by mixed002 make use of the
StateTestThread class. The failure is intermittent because the order of test
execution is shuffled automatically and sometimes the ServiceThread manages
to execute deflation at the right time to prevent more than one 
StateTestThread

object from existing at the same time.

The additions to vmTestbase/nsk/jdi/stress/serial/mixed002 are the standard
boilerplate needed to call WhiteBox functions from test code. The actual 
call

to WhiteBox.deflateIdleMonitors() is made in SerialExecutionDebuggee. I did
attempt a fix where I modified the StateTestThread class to make the call to
WhiteBox.deflateIdleMonitors() after the internal waitOnObject is no longer
contended or waited on. That fix reduced the frequency of the failures by
about half, but it didn't solve the test bug entirely. So I had to make the
fix in SerialExecutionDebuggee instead.


test/hotspot/jtreg/ProblemList.txt is modified to re-enable the mix002 test.


The remaining nine files are also wrapper style stress tests that execute
the debugger and debuggee parts of a specific list of tests serially *in
the same VM*. Because these tests also use SerialExecutionDebuggee, they
also need the boilerplate changes so that WhiteBox.deflateIdleMonitors()
can be called.


Re: [15] RFR(XXS): 7107012: sun.jvm.hostspot.code.CompressedReadStream readDouble() conversion to long mishandled

2020-06-29 Thread Chris Plummer

Thanks Serguei!

Can I get one more reviewer please? The change is very simple.

thanks,

Chris

On 6/26/20 5:51 PM, serguei.spit...@oracle.com wrote:

Hi Chris,

The fix looks good.
I would most likely overlook such a bug with my eyes. :)


Thanks,
Serguei


On 6/26/20 16:03, Chris Plummer wrote:

Hello,

Please help review the following:

http://cr.openjdk.java.net/~cjplummer/7107012/webrev.00/index.html
https://bugs.openjdk.java.net/browse/JDK-7107012

This bug is filed as confidential, although the issue is trivial. In 
the following line of code:


    return Double.longBitsToDouble((h << 32) | ((long)l & 
0xL));


Since h is an int, it's subject to the following:

https://docs.oracle.com/javase/specs/jls/se14/html/jls-15.html#jls-15.19

"If the promoted type of the left-hand operand is int, then only the 
five lowest-order bits of the right-hand operand are used as the 
shift distance. It is as if the right-hand operand were subjected to 
a bitwise logical AND operator & (§15.22.1) with the mask value 0x1f 
(0b1). The shift distance actually used is therefore always in 
the range 0 to 31, inclusive."


So (h << 32) is the same as (h << 0), which is not what was intended. 
The spec also calls out another issue:


"The type of the shift expression is the promoted type of the 
left-hand operand."


So even if it did left shift 32 bits, the result would have been 
truncated to an int, meaning the result would always be 0. The fix is 
to first cast h to a long. Doing this addresses both these problems, 
allowing a full 32 bit left shift to be done, and leaving the result 
as an untruncated long.


I was unable to trigger use of this code in SA. It seems to be used 
to pull locals out of a CompiledVFrame. I don't see any clhsdb paths 
to this code. It appears the GUI hsdb uses it via a complex call path 
I could not fully decipher, but I could not trigger its use from 
hsdb. In any case, the fix is straight forward and trivial, so I'd 
rather not have to spend more time digging deeper into its use and 
providing a test case.


thanks,

Chris








Re: 15 RFR(XS): 8165276: Spec states that invoke the premain method in an agent class if it's public but implementation differs

2020-06-29 Thread serguei.spit...@oracle.com

Thanks a lot for review, Mandy!
I also, asked Leonid to look if the test changes can be simplified

Thanks,
Serguei


On 6/29/20 09:46, Mandy Chung wrote:



On 6/27/20 12:23 AM, Alan Bateman wrote:

On 27/06/2020 01:40, serguei.spit...@oracle.com wrote:


The implementation has this order of lookup:

    // The agent class must have a premain or agentmain method that
    // has 1 or 2 arguments. We check in the following order:
    //
    // 1) declared with a signature of (String, Instrumentation)
    // 2) declared with a signature of (String)
    // 3) inherited with a signature of (String, Instrumentation)
    // 4) inherited with a signature of (String)

The declared methods are gotten with the getDeclaredMethod and 
inherited with the getMethod.
This works for both 1-arg and 2-arg premain methods, so I'm not sure 
what is inconsistent.

Or you have a concern there can be a non-nice NoSuchMethodException?

In fact, I don't understand why there is a need to use the 
getDeclaredMethod.
As I see, the getMethod should return a declared method first, and 
only if it is absent then it checks for a inherited one.
The JPLIS agent used getMethod when it was originally created in JDK 
5 so it would only find public methods. I haven't studied the 
intervening history too closely but I assume JDK-6289149 (in JDK 7) 
created the inconsistency between the spec and implementation when it 
explored the scenario of premain declared in a super class with 
different arity and/or modifiers to the premain in the sub-class. I 
assume the tests that you've been forced to change are related to 
this same issue.




Thanks for digging up the history.

So given where we are, and given the statement "The JVM first 
attempts to invoke the following method on the agent class" in the 
spec then I guess it's okay to keep the getDeclaredMethod to deal 
with "whacky" case where a super class of the agent class has a 
public premain method.




I also think it's okay to get a different exception message in this case.

Serguie - I reviewed this version.  It looks okay.

New wevrev version is:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/instr-setAccessible.2/


Mandy







Re: 15 RFR(XS): 8165276: Spec states that invoke the premain method in an agent class if it's public but implementation differs

2020-06-29 Thread serguei.spit...@oracle.com

Hi David,

Thank you a lot for review and tweaking the bug title.
I've re-targeted this to 16 as was suggested by Joe.

Thanks,
Serguei


On 6/28/20 19:37, David Holmes wrote:

Hi Serguei,

These changes look good to me.

Note that I tweaked the bug synopsis to make it slightly more 
grammatically correct: that invoke -> to invoke


Thanks,
David

On 26/06/2020 2:17 am, serguei.spit...@oracle.com wrote:


New wevrev version is:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/instr-setAccessible.2/

Now the InstrumentationImpl.java has this new check to throw IAE with 
the meaningful error message:


+ // reject non-public premain or agentmain method
+ if (!m.canAccess(null)) {
+ String msg = "method " + classname + "." + methodname + " must be 
declared public";

+ throw new IllegalAccessException(msg);
+ }


It also includes a new negative test for non-public premain method:
test/jdk/java/lang/instrument/NonPublicPremainAgent.java

I've tested the non-public agentmain as well with one of the hacked 
JVMTI aod tests.
But I gave up to make it a stand alone test as this testing framework 
is tricky to use for negative testing.
The implementation is common for premain and agentmain cases, so 
probably, one test



Also, I had to fix all impacted java/lang/instrument tests to make 
the Agent classes public.

The following tests required a refactoring:

|| test/jdk/java/lang/instrument/PremainClass/InheritAgent0100.java
test/jdk/java/lang/instrument/PremainClass/InheritAgent1000.java
test/jdk/java/lang/instrument/PremainClass/InheritAgent1100.java


They define an agent as extending an agent super class which has the 
premain methods defined:


   37 class InheritAgent0101 extends InheritAgent0101Super {
   38
   39 //
   40 // This agent has a single argument premain() method which
   41 // is the one that should be called.
   42 //
   43 public static void premain (String agentArgs) {
   44 System.out.println("Hello from Single-Arg 
InheritAgent0101!");

   45 }
   46
   47 // This agent does NOT have a double argument premain() 
method.

   48 }
   49
   50 class InheritAgent0101Super {
   51
   52 //
   53 // This agent has a single argument premain() method which
   54 // is NOT the one that should be called.
   55 //
   56 public static void premain (String agentArgs) {
   57 System.out.println("Hello from Single-Arg 
InheritAgent0101Super!");
   58 throw new Error("ERROR: THIS AGENT SHOULD NOT HAVE BEEN 
CALLED.");

   59 }
   60
   61 // This agent does NOT have a double argument premain() 
method.

   62 }


Above, just one class can be made public.
But the InheritAgent0101Super has to be public as well as has the 
premain method defined.

These agent super classes are separated to their own files.
To make this refactoring to work new || customized script is introduced:
   test/jdk/java/lang/instrument/PremainClass/MakeJAR.sh

The java/lang/instrument tests are passed locally.
I'll submit a mach5 test jobs to make sure nothing is broken.

Thanks,
Serguei



On 6/24/20 13:07, serguei.spit...@oracle.com wrote:

On 6/24/20 12:44, Mandy Chung wrote:



On 6/24/20 12:26 PM, serguei.spit...@oracle.com wrote:

On 6/24/20 05:25, David Holmes wrote:


Ah! The test class SimpleAgent is what is not public. That seems 
a bug in the test.


There are many such tests.
We can break some of the existing agents by rejecting non-public 
agent classes.
I'm inclined to continue using the setAccessible and just add an 
extra check for non-public premain/agentmain methods.


There is only one non-public SimpleAgent which is shared by 
j.l.instrument tests.

  test/jdk/java/lang/instrument/SimpleAgent.java


There are many such tests:

test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/TestLambdaFormRetransformation.java:class 
Agent implements ClassFileTransformer {


test/jdk/java/lang/instrument/NativeMethodPrefixAgent.java:class 
NativeMethodPrefixAgent {
test/jdk/java/lang/instrument/PremainClass/NoPremainAgent.java:class 
NoPremainAgent {

test/jdk/java/lang/instrument/SimpleAgent.java:class SimpleAgent {
test/jdk/java/lang/instrument/RetransformAgent.java:class 
RetransformAgent {
test/jdk/java/lang/instrument/PremainClass/InheritAgent0001.java:class 
InheritAgent0001 extends InheritAgent0001Super {
test/jdk/java/lang/instrument/PremainClass/InheritAgent0001.java:class 
InheritAgent0001Super {
test/jdk/java/lang/instrument/PremainClass/InheritAgent0010.java:class 
InheritAgent0010 extends InheritAgent0010Super {
test/jdk/java/lang/instrument/PremainClass/InheritAgent0010.java:class 
InheritAgent0010Super {
test/jdk/java/lang/instrument/PremainClass/InheritAgent0011.java:class 
InheritAgent0011 extends InheritAgent0011Super {
test/jdk/java/lang/instrument/PremainClass/InheritAgent0011.java:class 
InheritAgent0011Super {
test/jdk/java/lang/instrument/PremainClass/InheritAgent0100.java:class 
InheritAgent0100 extends InheritAgent0100S

Re: 15 RFR(XS): 8165276: Spec states that invoke the premain method in an agent class if it's public but implementation differs

2020-06-29 Thread serguei.spit...@oracle.com

On 6/27/20 00:23, Alan Bateman wrote:

On 27/06/2020 01:40, serguei.spit...@oracle.com wrote:


The implementation has this order of lookup:

    // The agent class must have a premain or agentmain method that
    // has 1 or 2 arguments. We check in the following order:
    //
    // 1) declared with a signature of (String, Instrumentation)
    // 2) declared with a signature of (String)
    // 3) inherited with a signature of (String, Instrumentation)
    // 4) inherited with a signature of (String)

The declared methods are gotten with the getDeclaredMethod and 
inherited with the getMethod.
This works for both 1-arg and 2-arg premain methods, so I'm not sure 
what is inconsistent.

Or you have a concern there can be a non-nice NoSuchMethodException?

In fact, I don't understand why there is a need to use the 
getDeclaredMethod.
As I see, the getMethod should return a declared method first, and 
only if it is absent then it checks for a inherited one.
The JPLIS agent used getMethod when it was originally created in JDK 5 
so it would only find public methods. I haven't studied the 
intervening history too closely but I assume JDK-6289149 (in JDK 7) 
created the inconsistency between the spec and implementation when it 
explored the scenario of premain declared in a super class with 
different arity and/or modifiers to the premain in the sub-class. I 
assume the tests that you've been forced to change are related to this 
same issue.


So given where we are, and given the statement "The JVM first attempts 
to invoke the following method on the agent class" in the spec then I 
guess it's okay to keep the getDeclaredMethod to deal with "whacky" 
case where a super class of the agent class has a public premain method.


Thank you for clarification, Alan.

Thanks,
Serguei



-Alan.









Re: 15 RFR(XS): 8165276: Spec states that invoke the premain method in an agent class if it's public but implementation differs

2020-06-29 Thread Mandy Chung



On 6/27/20 12:23 AM, Alan Bateman wrote:

On 27/06/2020 01:40, serguei.spit...@oracle.com wrote:


The implementation has this order of lookup:

    // The agent class must have a premain or agentmain method that
    // has 1 or 2 arguments. We check in the following order:
    //
    // 1) declared with a signature of (String, Instrumentation)
    // 2) declared with a signature of (String)
    // 3) inherited with a signature of (String, Instrumentation)
    // 4) inherited with a signature of (String)

The declared methods are gotten with the getDeclaredMethod and 
inherited with the getMethod.
This works for both 1-arg and 2-arg premain methods, so I'm not sure 
what is inconsistent.

Or you have a concern there can be a non-nice NoSuchMethodException?

In fact, I don't understand why there is a need to use the 
getDeclaredMethod.
As I see, the getMethod should return a declared method first, and 
only if it is absent then it checks for a inherited one.
The JPLIS agent used getMethod when it was originally created in JDK 5 
so it would only find public methods. I haven't studied the 
intervening history too closely but I assume JDK-6289149 (in JDK 7) 
created the inconsistency between the spec and implementation when it 
explored the scenario of premain declared in a super class with 
different arity and/or modifiers to the premain in the sub-class. I 
assume the tests that you've been forced to change are related to this 
same issue.




Thanks for digging up the history.

So given where we are, and given the statement "The JVM first attempts 
to invoke the following method on the agent class" in the spec then I 
guess it's okay to keep the getDeclaredMethod to deal with "whacky" 
case where a super class of the agent class has a public premain method.




I also think it's okay to get a different exception message in this case.

Serguie - I reviewed this version.  It looks okay.

New wevrev version is:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/instr-setAccessible.2/


Mandy





RFR(s): 8247863: Unreachable code in OperatingSystemImpl.getTotalSwapSpaceSize()

2020-06-29 Thread Severin Gehwolf
Hi,

Could I please get a review of this dead-code removal? During review of
JDK-8244500 it was discovered that with the new cgroups implementation
supporting v1 and v2 Metrics.getMemoryAndSwapLimit() will never return
0 when relevant cgroup files are missing. E.g. on a system where the
kernel doesn't support swap limit capabilities. Therefore this code
introduced with JDK-8236617 can no longer be reached and should get
removed.

Bug: https://bugs.openjdk.java.net/browse/JDK-8247863
webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8247863/01/webrev/

Testing: Matthias tested this on the affected system and it did pass
for him. Docker tests on cgroup v1 and cgroup v2.

Thanks,
Severin