Re: RFR: JDK-8068225: nsk/jdi/EventQueue/remove_l/remove_l005 intermittently times out

[Chris Plummer]

Ok. I think the fix is fine then if this is the only test that was 
problematic.


Chris

On 2/6/19 11:02 AM, Gary Adams wrote:

Yes, the test was hung in waitFor.

The debugger process made some calls after the VMStart event
was observed, but never resumed the debuggee.

Here's a simple change that makes test run to completion cleanly.


diff --git 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/canBeModified/canbemodified001.java 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/canBeModified/canbemodified001.java 

--- 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/canBeModified/canbemodified001.java
+++ 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/canBeModified/canbemodified001.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights 
reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -78,6 +78,7 @@
 exitStatus = Consts.TEST_FAILED;
 e.printStackTrace();
 } finally {
+    debugee.resume();
 debugee.endDebugee();
 }
 display("Test finished. exitStatus = " + exitStatus);

On 2/6/19, 1:16 PM, Chris Plummer wrote:

Does it timeout in waitFor()?

Chris

On 2/6/19 10:01 AM, Gary Adams wrote:
So far the only test that has an issue with the change in waitFor 
order is
nsk/jdi/VirtualMachine/canBeModified, which does not appear to 
synchronize

with the debuggee.

On 2/6/19, 12:15 PM, Chris Plummer wrote:
What happens if endDebugee() is called when the caller is not 
expecting a clean exit of the debugeee? Won't waitFor() wait 
forever in that case? I think by having the vm.dispose() first, you 
avoid that issue.


Chris

On 2/6/19 8:04 AM, Gary Adams wrote:
Testing an alternate fix that will wait for the debugee process to 
terminate after processing the "quit"

command before doing the vm tear down.


diff --git 
a/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java 
b/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java

--- a/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java
+++ b/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All 
rights reserved.
+ * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All 
rights reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or 
modify it

@@ -557,6 +557,7 @@
  * exit status code.
  */
 public int endDebugee() {
+    int status = waitFor();
 if (vm != null) {
 try {
 vm.dispose();
@@ -564,7 +565,7 @@
 }
 vm = null;
 }
-    return waitFor();
+    return status;
 }


On 2/6/19, 8:14 AM, Gary Adams wrote:

Just a quick update on the failure on shutdown ...
I've come across a couple of other tests with a similar failure 
mode.


I'm attempting to get some additional diagnostic information from 
these
tests with "-jdi.trace=all", but the diagnostic output may be 
interfering

with the timing.

My current investigation is around a possible race condition
in the Debgugee.quit() processing.
   - the "quit" string is sent to the deuggee via the IOpipe
   - the vm.dispose() is handled from the JDWP communication

If the context switching is not quick enough, the debugee
may not have read and processed the "quit" command before
tear down is handled.

From earlier comments in the bug report the debuggee was observed
still at the breakpoint, so the resume and the disabling of the 
breakpoint

had not been processed, yet.

It might be worth having an "ack" returned when the "quit" is
processed in the debugee.

...

On 2/1/19, 2:15 PM, serguei.spit...@oracle.com wrote:

Hi Gary,

The debugee.quit() is to complete the session.
It makes this call: sendSignal(SGNL_QUIT);

A call to the debugee.quit() is at the end of execTest() method:

    display("");
    }

    display("");
    display("=");
    display("TEST FINISHES\n");

    brkpReq.disable();
    debugee.resume();
    debugee.quit();
    }

Thanks,
Serguei


On 2/1/19 10:00, Gary Adams wrote:
When the remove_l005 runs to completion, it never signals the 
debuggee

that all iterations have completed.

  Issue: https://bugs.openjdk.java.net/browse/JDK-8068225

diff --git a/test/hotspot/jtreg/ProblemList.txt 
b/test/hotspot/jtreg/ProblemList.txt

--- a/test/hotspot/jtreg/ProblemList.txt
+++ b/test/hotspot/jtreg/ProblemList.txt
@@ -163,7 +163,6 @@
 vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003/TestDescription.java 
8066993 generic-all
 vmTestbase/nsk/jdi/VirtualMachine/redefineClasses/redefineclasses021/TestDescription.java 
8065773 generic-all
 vmTestbase/nsk

Re: RFR: JDK-8068225: nsk/jdi/EventQueue/remove_l/remove_l005 intermittently times out

[Gary Adams]

Yes, the test was hung in waitFor.

The debugger process made some calls after the VMStart event
was observed, but never resumed the debuggee.

Here's a simple change that makes test run to completion cleanly.


diff --git 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/canBeModified/canbemodified001.java 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/canBeModified/canbemodified001.java
--- 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/canBeModified/canbemodified001.java
+++ 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/canBeModified/canbemodified001.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights 
reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -78,6 +78,7 @@
 exitStatus = Consts.TEST_FAILED;
 e.printStackTrace();
 } finally {
+debugee.resume();
 debugee.endDebugee();
 }
 display("Test finished. exitStatus = " + exitStatus);

On 2/6/19, 1:16 PM, Chris Plummer wrote:

Does it timeout in waitFor()?

Chris

On 2/6/19 10:01 AM, Gary Adams wrote:
So far the only test that has an issue with the change in waitFor 
order is
nsk/jdi/VirtualMachine/canBeModified, which does not appear to 
synchronize

with the debuggee.

On 2/6/19, 12:15 PM, Chris Plummer wrote:
What happens if endDebugee() is called when the caller is not 
expecting a clean exit of the debugeee? Won't waitFor() wait forever 
in that case? I think by having the vm.dispose() first, you avoid 
that issue.


Chris

On 2/6/19 8:04 AM, Gary Adams wrote:
Testing an alternate fix that will wait for the debugee process to 
terminate after processing the "quit"

command before doing the vm tear down.


diff --git 
a/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java 
b/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java

--- a/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java
+++ b/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All 
rights reserved.
+ * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All 
rights reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or 
modify it

@@ -557,6 +557,7 @@
  * exit status code.
  */
 public int endDebugee() {
+int status = waitFor();
 if (vm != null) {
 try {
 vm.dispose();
@@ -564,7 +565,7 @@
 }
 vm = null;
 }
-return waitFor();
+return status;
 }


On 2/6/19, 8:14 AM, Gary Adams wrote:

Just a quick update on the failure on shutdown ...
I've come across a couple of other tests with a similar failure mode.

I'm attempting to get some additional diagnostic information from 
these
tests with "-jdi.trace=all", but the diagnostic output may be 
interfering

with the timing.

My current investigation is around a possible race condition
in the Debgugee.quit() processing.
   - the "quit" string is sent to the deuggee via the IOpipe
   - the vm.dispose() is handled from the JDWP communication

If the context switching is not quick enough, the debugee
may not have read and processed the "quit" command before
tear down is handled.

From earlier comments in the bug report the debuggee was observed
still at the breakpoint, so the resume and the disabling of the 
breakpoint

had not been processed, yet.

It might be worth having an "ack" returned when the "quit" is
processed in the debugee.

...

On 2/1/19, 2:15 PM, serguei.spit...@oracle.com wrote:

Hi Gary,

The debugee.quit() is to complete the session.
It makes this call: sendSignal(SGNL_QUIT);

A call to the debugee.quit() is at the end of execTest() method:

display("");
}

display("");
display("=");
display("TEST FINISHES\n");

brkpReq.disable();
debugee.resume();
debugee.quit();
}

Thanks,
Serguei


On 2/1/19 10:00, Gary Adams wrote:
When the remove_l005 runs to completion, it never signals the 
debuggee

that all iterations have completed.

  Issue: https://bugs.openjdk.java.net/browse/JDK-8068225

diff --git a/test/hotspot/jtreg/ProblemList.txt 
b/test/hotspot/jtreg/ProblemList.txt

--- a/test/hotspot/jtreg/ProblemList.txt
+++ b/test/hotspot/jtreg/ProblemList.txt
@@ -163,7 +163,6 @@
 vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003/TestDescription.java 
8066993 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/EventQueue/remo

Re: [aarch64-port-dev ] RFR: 8209413: AArch64: NPE in clhsdb jstack command

[Andrew Haley]

On 2/6/19 10:54 AM, Nick Gasson (Arm Technology China) wrote:
> Hi Andrew
> 
>> Here's the test that reveals the problem: it seems that you need an entry 
>> frame which calls compiled Java code.
> 
> This seems slightly different to the original problem, although
> maybe related. Because here the top-most frame is a compiled Java
> frame we'll take the vm.isJavaPCDbg and !vm.isClientCompiler
> branches of AARCH64CurrentFrameGuess::run which as far as I can tell
> always gets the PC from the thread context. The original crash I was
> looking at happened when the top frame was native (else branch on
> line 183) where the PC is set to null which causes the two-argument
> AARCH64Frame constructor to be used.

OK.

> Unfortunately I'm on holiday until next Thursday so can't test
> anything. Did you try Thread.sleep? That's what LingeredApp that the
> jtreg tests is trying to get a stack trace of calls.

Well, that was fun. When generally assume that we can find a saved PC
in a fixed place in the frame, and this is always the word above the
FP. However, when we're in JNI native code, as you've noticed the
C[++] compiler isn't helpful enough to lave the caller PC in a
well-known place. When the frame is created by our AArch64 assembly-
langage code we'll be fine because we always do the right thing.

The problem is that when we save the Java frame state in the Thread,
we're doing it for the sake of the runtime, not for debuggers, so we
don't always save all the information that a debugger might need.

This patch should work for compiled native methods, but I'm not at all
sure about all of the other places where we call out from runtime
stubs to the VM. We perhaps check that the PC returned by
raw_sp.getAddressAt(-1 * VM.getVM().getAddressSize()) is in the code
cache before we use it.

Anyway, try this: it should fix your immediate bug.

diff -r 3954d70e1c50 src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp
--- a/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp Wed Feb 06 08:31:27 
2019 +0100
+++ b/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp Wed Feb 06 18:22:09 
2019 +
@@ -1916,10 +1916,20 @@
 default:
   ShouldNotReachHere();
 }
+
+// Leave a breadcrumb for
+// sun.jvm.hotspot.runtime.aarch64.AARCH64Frame(sp, fp)
+Label retaddr;
+__ adr(rscratch2, retaddr);
+__ stp(zr, rscratch2, Address(__ pre(sp, -2 * wordSize)));
+
 rt_call(masm, native_func,
 int_args + 2, // AArch64 passes up to 8 args in int registers
 float_args,   // and up to 8 float args
 return_type);
+
+__ bind(retaddr);
+__ add(sp, sp, 2 * wordSize);
   }

   // Unpack native results.


-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. 
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: RFR: JDK-8068225: nsk/jdi/EventQueue/remove_l/remove_l005 intermittently times out

[Chris Plummer]

Does it timeout in waitFor()?

Chris

On 2/6/19 10:01 AM, Gary Adams wrote:
So far the only test that has an issue with the change in waitFor 
order is
nsk/jdi/VirtualMachine/canBeModified, which does not appear to 
synchronize

with the debuggee.

On 2/6/19, 12:15 PM, Chris Plummer wrote:
What happens if endDebugee() is called when the caller is not 
expecting a clean exit of the debugeee? Won't waitFor() wait forever 
in that case? I think by having the vm.dispose() first, you avoid 
that issue.


Chris

On 2/6/19 8:04 AM, Gary Adams wrote:
Testing an alternate fix that will wait for the debugee process to 
terminate after processing the "quit"

command before doing the vm tear down.


diff --git 
a/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java 
b/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java

--- a/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java
+++ b/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All 
rights reserved.
+ * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All 
rights reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or 
modify it

@@ -557,6 +557,7 @@
  * exit status code.
  */
 public int endDebugee() {
+    int status = waitFor();
 if (vm != null) {
 try {
 vm.dispose();
@@ -564,7 +565,7 @@
 }
 vm = null;
 }
-    return waitFor();
+    return status;
 }


On 2/6/19, 8:14 AM, Gary Adams wrote:

Just a quick update on the failure on shutdown ...
I've come across a couple of other tests with a similar failure mode.

I'm attempting to get some additional diagnostic information from 
these
tests with "-jdi.trace=all", but the diagnostic output may be 
interfering

with the timing.

My current investigation is around a possible race condition
in the Debgugee.quit() processing.
   - the "quit" string is sent to the deuggee via the IOpipe
   - the vm.dispose() is handled from the JDWP communication

If the context switching is not quick enough, the debugee
may not have read and processed the "quit" command before
tear down is handled.

From earlier comments in the bug report the debuggee was observed
still at the breakpoint, so the resume and the disabling of the 
breakpoint

had not been processed, yet.

It might be worth having an "ack" returned when the "quit" is
processed in the debugee.

...

On 2/1/19, 2:15 PM, serguei.spit...@oracle.com wrote:

Hi Gary,

The debugee.quit() is to complete the session.
It makes this call: sendSignal(SGNL_QUIT);

A call to the debugee.quit() is at the end of execTest() method:

    display("");
    }

    display("");
    display("=");
    display("TEST FINISHES\n");

    brkpReq.disable();
    debugee.resume();
    debugee.quit();
    }

Thanks,
Serguei


On 2/1/19 10:00, Gary Adams wrote:
When the remove_l005 runs to completion, it never signals the 
debuggee

that all iterations have completed.

  Issue: https://bugs.openjdk.java.net/browse/JDK-8068225

diff --git a/test/hotspot/jtreg/ProblemList.txt 
b/test/hotspot/jtreg/ProblemList.txt

--- a/test/hotspot/jtreg/ProblemList.txt
+++ b/test/hotspot/jtreg/ProblemList.txt
@@ -163,7 +163,6 @@
 vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003/TestDescription.java 
8066993 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/EventQueue/remove_l/remove_l005/TestDescription.java 
8068225 generic-all


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



diff --git 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java 

--- 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java
+++ 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2018, Oracle and/or its affiliates. All 
rights reserved.
+ * Copyright (c) 2002, 2019, Oracle and/or its affiliates. All 
rights reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or 
modify it

@@ -134,6 +134,7 @@
 }
 display("");
 }
+    debugee.sendSignal(SGNL_QUIT);

 display("");
 display("=");

Re: RFR: JDK-8068225: nsk/jdi/EventQueue/remove_l/remove_l005 intermittently times out

[Gary Adams]

So far the only test that has an issue with the change in waitFor order is
nsk/jdi/VirtualMachine/canBeModified, which does not appear to synchronize
with the debuggee.

On 2/6/19, 12:15 PM, Chris Plummer wrote:
What happens if endDebugee() is called when the caller is not 
expecting a clean exit of the debugeee? Won't waitFor() wait forever 
in that case? I think by having the vm.dispose() first, you avoid that 
issue.


Chris

On 2/6/19 8:04 AM, Gary Adams wrote:
Testing an alternate fix that will wait for the debugee process to 
terminate after processing the "quit"

command before doing the vm tear down.


diff --git a/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java 
b/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java

--- a/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java
+++ b/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All 
rights reserved.
+ * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All 
rights reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -557,6 +557,7 @@
  * exit status code.
  */
 public int endDebugee() {
+int status = waitFor();
 if (vm != null) {
 try {
 vm.dispose();
@@ -564,7 +565,7 @@
 }
 vm = null;
 }
-return waitFor();
+return status;
 }


On 2/6/19, 8:14 AM, Gary Adams wrote:

Just a quick update on the failure on shutdown ...
I've come across a couple of other tests with a similar failure mode.

I'm attempting to get some additional diagnostic information from these
tests with "-jdi.trace=all", but the diagnostic output may be 
interfering

with the timing.

My current investigation is around a possible race condition
in the Debgugee.quit() processing.
   - the "quit" string is sent to the deuggee via the IOpipe
   - the vm.dispose() is handled from the JDWP communication

If the context switching is not quick enough, the debugee
may not have read and processed the "quit" command before
tear down is handled.

From earlier comments in the bug report the debuggee was observed
still at the breakpoint, so the resume and the disabling of the 
breakpoint

had not been processed, yet.

It might be worth having an "ack" returned when the "quit" is
processed in the debugee.

...

On 2/1/19, 2:15 PM, serguei.spit...@oracle.com wrote:

Hi Gary,

The debugee.quit() is to complete the session.
It makes this call: sendSignal(SGNL_QUIT);

A call to the debugee.quit() is at the end of execTest() method:

display("");
}

display("");
display("=");
display("TEST FINISHES\n");

brkpReq.disable();
debugee.resume();
debugee.quit();
}

Thanks,
Serguei


On 2/1/19 10:00, Gary Adams wrote:
When the remove_l005 runs to completion, it never signals the 
debuggee

that all iterations have completed.

  Issue: https://bugs.openjdk.java.net/browse/JDK-8068225

diff --git a/test/hotspot/jtreg/ProblemList.txt 
b/test/hotspot/jtreg/ProblemList.txt

--- a/test/hotspot/jtreg/ProblemList.txt
+++ b/test/hotspot/jtreg/ProblemList.txt
@@ -163,7 +163,6 @@
 vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003/TestDescription.java 
8066993 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/EventQueue/remove_l/remove_l005/TestDescription.java 
8068225 generic-all


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



diff --git 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java 

--- 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java
+++ 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2018, Oracle and/or its affiliates. All 
rights reserved.
+ * Copyright (c) 2002, 2019, Oracle and/or its affiliates. All 
rights reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or 
modify it

@@ -134,6 +134,7 @@
 }
 display("");
 }
+debugee.sendSignal(SGNL_QUIT);

 display("");
 display("=");

Re: RFR: JDK-8068225: nsk/jdi/EventQueue/remove_l/remove_l005 intermittently times out

[Chris Plummer]

What happens if endDebugee() is called when the caller is not expecting 
a clean exit of the debugeee? Won't waitFor() wait forever in that case? 
I think by having the vm.dispose() first, you avoid that issue.


Chris

On 2/6/19 8:04 AM, Gary Adams wrote:
Testing an alternate fix that will wait for the debugee process to 
terminate after processing the "quit"

command before doing the vm tear down.


diff --git a/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java 
b/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java

--- a/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java
+++ b/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights 
reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -557,6 +557,7 @@
  * exit status code.
  */
 public int endDebugee() {
+    int status = waitFor();
 if (vm != null) {
 try {
 vm.dispose();
@@ -564,7 +565,7 @@
 }
 vm = null;
 }
-    return waitFor();
+    return status;
 }


On 2/6/19, 8:14 AM, Gary Adams wrote:

Just a quick update on the failure on shutdown ...
I've come across a couple of other tests with a similar failure mode.

I'm attempting to get some additional diagnostic information from these
tests with "-jdi.trace=all", but the diagnostic output may be 
interfering

with the timing.

My current investigation is around a possible race condition
in the Debgugee.quit() processing.
   - the "quit" string is sent to the deuggee via the IOpipe
   - the vm.dispose() is handled from the JDWP communication

If the context switching is not quick enough, the debugee
may not have read and processed the "quit" command before
tear down is handled.

From earlier comments in the bug report the debuggee was observed
still at the breakpoint, so the resume and the disabling of the 
breakpoint

had not been processed, yet.

It might be worth having an "ack" returned when the "quit" is
processed in the debugee.

...

On 2/1/19, 2:15 PM, serguei.spit...@oracle.com wrote:

Hi Gary,

The debugee.quit() is to complete the session.
It makes this call: sendSignal(SGNL_QUIT);

A call to the debugee.quit() is at the end of execTest() method:

    display("");
    }

    display("");
    display("=");
    display("TEST FINISHES\n");

    brkpReq.disable();
    debugee.resume();
    debugee.quit();
    }

Thanks,
Serguei


On 2/1/19 10:00, Gary Adams wrote:

When the remove_l005 runs to completion, it never signals the debuggee
that all iterations have completed.

  Issue: https://bugs.openjdk.java.net/browse/JDK-8068225

diff --git a/test/hotspot/jtreg/ProblemList.txt 
b/test/hotspot/jtreg/ProblemList.txt

--- a/test/hotspot/jtreg/ProblemList.txt
+++ b/test/hotspot/jtreg/ProblemList.txt
@@ -163,7 +163,6 @@
 vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003/TestDescription.java 
8066993 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/EventQueue/remove_l/remove_l005/TestDescription.java 
8068225 generic-all


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



diff --git 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java 

--- 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java
+++ 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2018, Oracle and/or its affiliates. All 
rights reserved.
+ * Copyright (c) 2002, 2019, Oracle and/or its affiliates. All 
rights reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or 
modify it

@@ -134,6 +134,7 @@
 }
 display("");
 }
+    debugee.sendSignal(SGNL_QUIT);

 display("");
 display("=");

Re: RFR: JDK-8068225: nsk/jdi/EventQueue/remove_l/remove_l005 intermittently times out

[Gary Adams]

Testing an alternate fix that will wait for the debugee process to 
terminate after processing the "quit"

command before doing the vm tear down.


diff --git a/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java 
b/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java

--- a/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java
+++ b/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Debugee.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights 
reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -557,6 +557,7 @@
  * exit status code.
  */
 public int endDebugee() {
+int status = waitFor();
 if (vm != null) {
 try {
 vm.dispose();
@@ -564,7 +565,7 @@
 }
 vm = null;
 }
-return waitFor();
+return status;
 }


On 2/6/19, 8:14 AM, Gary Adams wrote:

Just a quick update on the failure on shutdown ...
I've come across a couple of other tests with a similar failure mode.

I'm attempting to get some additional diagnostic information from these
tests with "-jdi.trace=all", but the diagnostic output may be interfering
with the timing.

My current investigation is around a possible race condition
in the Debgugee.quit() processing.
   - the "quit" string is sent to the deuggee via the IOpipe
   - the vm.dispose() is handled from the JDWP communication

If the context switching is not quick enough, the debugee
may not have read and processed the "quit" command before
tear down is handled.

From earlier comments in the bug report the debuggee was observed
still at the breakpoint, so the resume and the disabling of the 
breakpoint

had not been processed, yet.

It might be worth having an "ack" returned when the "quit" is
processed in the debugee.

...

On 2/1/19, 2:15 PM, serguei.spit...@oracle.com wrote:

Hi Gary,

The debugee.quit() is to complete the session.
It makes this call: sendSignal(SGNL_QUIT);

A call to the debugee.quit() is at the end of execTest() method:

display("");
}

display("");
display("=");
display("TEST FINISHES\n");

brkpReq.disable();
debugee.resume();
debugee.quit();
}

Thanks,
Serguei


On 2/1/19 10:00, Gary Adams wrote:

When the remove_l005 runs to completion, it never signals the debuggee
that all iterations have completed.

  Issue: https://bugs.openjdk.java.net/browse/JDK-8068225

diff --git a/test/hotspot/jtreg/ProblemList.txt 
b/test/hotspot/jtreg/ProblemList.txt

--- a/test/hotspot/jtreg/ProblemList.txt
+++ b/test/hotspot/jtreg/ProblemList.txt
@@ -163,7 +163,6 @@
 vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003/TestDescription.java 
8066993 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/EventQueue/remove_l/remove_l005/TestDescription.java 
8068225 generic-all


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



diff --git 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java 

--- 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java
+++ 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2018, Oracle and/or its affiliates. All 
rights reserved.
+ * Copyright (c) 2002, 2019, Oracle and/or its affiliates. All 
rights reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or 
modify it

@@ -134,6 +134,7 @@
 }
 display("");
 }
+debugee.sendSignal(SGNL_QUIT);

 display("");
 display("=");

Re: RFR(S) 8212127: Cleanup TLAB fast refill statistics, perf counters and etc.

[zgu]

On Wed, 2019-02-06 at 15:48 +0100, Aleksey Shipilev wrote:
> On 2/6/19 3:34 PM, z...@redhat.com wrote:
> > > *) In src/hotspot/share/gc/shared/threadLocalAllocBuffer.hpp, why
> > > this whole thing is removed? I
> > > would expect "just" the rename of slow_refill_waste() to
> > > refill_waste() and removing
> > > fast_refill_waste() here, leaving everything else untouched.
> > > 
> > >  105   // statistics
> > >  106
> > >  107   int number_of_refills() const { return _number_of_refills;
> > > }
> > >  108   int fast_refill_waste() const { return _fast_refill_waste;
> > > }
> > >  109   int slow_refill_waste() const { return _slow_refill_waste;
> > > }
> > >  110   int gc_waste() const  { return _gc_waste; }
> > >  111   int slow_allocations() const  { return _slow_allocations;
> > > }
> > >  112
> > 
> > They are dead code. Do they worth for another cleanup RFE for the
> > trivial cleanup?
> 
> Okay. I thought the patch was about fast refills only. But indeed, we
> can clean these unused
> definitions in the same patch. On one hand, removing trivial unused
> getters would require us to add
> them back when they are needed. On the other hand, it seems that TLAB
> does every statistic
> internally, and we should instead force callers to use higher-level
> TLAB methods to access and
> interpret them.
> 
> Code looks good! I think we still need Serviceability to acknowledge
> this is okay to do.

Serguei okayed.

Serguei, does it still okay with you?

Thanks,

-Zhengyu

> 
> -Aleksey
> 
> 
> 
> 
> 
> 
> 

Re: RFR(S) 8212127: Cleanup TLAB fast refill statistics, perf counters and etc.

[Aleksey Shipilev]

On 2/6/19 3:34 PM, z...@redhat.com wrote:
>> *) In src/hotspot/share/gc/shared/threadLocalAllocBuffer.hpp, why
>> this whole thing is removed? I
>> would expect "just" the rename of slow_refill_waste() to
>> refill_waste() and removing
>> fast_refill_waste() here, leaving everything else untouched.
>>
>>  105   // statistics
>>  106
>>  107   int number_of_refills() const { return _number_of_refills; }
>>  108   int fast_refill_waste() const { return _fast_refill_waste; }
>>  109   int slow_refill_waste() const { return _slow_refill_waste; }
>>  110   int gc_waste() const  { return _gc_waste; }
>>  111   int slow_allocations() const  { return _slow_allocations; }
>>  112
> 
> They are dead code. Do they worth for another cleanup RFE for the
> trivial cleanup?

Okay. I thought the patch was about fast refills only. But indeed, we can clean 
these unused
definitions in the same patch. On one hand, removing trivial unused getters 
would require us to add
them back when they are needed. On the other hand, it seems that TLAB does 
every statistic
internally, and we should instead force callers to use higher-level TLAB 
methods to access and
interpret them.

Code looks good! I think we still need Serviceability to acknowledge this is 
okay to do.

-Aleksey









signature.asc
Description: OpenPGP digital signature

Re: RFR(S) 8212127: Cleanup TLAB fast refill statistics, perf counters and etc.

[zgu]

Thanks, Aleksey.

On Wed, 2019-02-06 at 15:23 +0100, Aleksey Shipilev wrote:
> On 2/3/19 10:43 PM, z...@redhat.com wrote:
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8212127
> > Webrev: http://cr.openjdk.java.net/~zgu/JDK-8212127/webrev.00/
> 
> Looks fine code-wise.
> 
> *) In src/hotspot/share/gc/shared/threadLocalAllocBuffer.hpp, why
> this whole thing is removed? I
> would expect "just" the rename of slow_refill_waste() to
> refill_waste() and removing
> fast_refill_waste() here, leaving everything else untouched.
> 
>  105   // statistics
>  106
>  107   int number_of_refills() const { return _number_of_refills; }
>  108   int fast_refill_waste() const { return _fast_refill_waste; }
>  109   int slow_refill_waste() const { return _slow_refill_waste; }
>  110   int gc_waste() const  { return _gc_waste; }
>  111   int slow_allocations() const  { return _slow_allocations; }
>  112

They are dead code. Do they worth for another cleanup RFE for the
trivial cleanup?

-Zhengyu

> 
> 
> Thanks,
> -Aleksey
> 

Re: RFR(S) 8212127: Cleanup TLAB fast refill statistics, perf counters and etc.

[Aleksey Shipilev]

On 2/3/19 10:43 PM, z...@redhat.com wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8212127
> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8212127/webrev.00/

Looks fine code-wise.

*) In src/hotspot/share/gc/shared/threadLocalAllocBuffer.hpp, why this whole 
thing is removed? I
would expect "just" the rename of slow_refill_waste() to refill_waste() and 
removing
fast_refill_waste() here, leaving everything else untouched.

 105   // statistics
 106
 107   int number_of_refills() const { return _number_of_refills; }
 108   int fast_refill_waste() const { return _fast_refill_waste; }
 109   int slow_refill_waste() const { return _slow_refill_waste; }
 110   int gc_waste() const  { return _gc_waste; }
 111   int slow_allocations() const  { return _slow_allocations; }
 112


Thanks,
-Aleksey



signature.asc
Description: OpenPGP digital signature

Re: RFR: JDK-8068225: nsk/jdi/EventQueue/remove_l/remove_l005 intermittently times out

[Gary Adams]

Just a quick update on the failure on shutdown ...
I've come across a couple of other tests with a similar failure mode.

I'm attempting to get some additional diagnostic information from these
tests with "-jdi.trace=all", but the diagnostic output may be interfering
with the timing.

My current investigation is around a possible race condition
in the Debgugee.quit() processing.
   - the "quit" string is sent to the deuggee via the IOpipe
   - the vm.dispose() is handled from the JDWP communication

If the context switching is not quick enough, the debugee
may not have read and processed the "quit" command before
tear down is handled.

From earlier comments in the bug report the debuggee was observed
still at the breakpoint, so the resume and the disabling of the breakpoint
had not been processed, yet.

It might be worth having an "ack" returned when the "quit" is
processed in the debugee.

...

On 2/1/19, 2:15 PM, serguei.spit...@oracle.com wrote:

Hi Gary,

The debugee.quit() is to complete the session.
It makes this call: sendSignal(SGNL_QUIT);

A call to the debugee.quit() is at the end of execTest() method:

display("");
}

display("");
display("=");
display("TEST FINISHES\n");

brkpReq.disable();
debugee.resume();
debugee.quit();
}

Thanks,
Serguei


On 2/1/19 10:00, Gary Adams wrote:

When the remove_l005 runs to completion, it never signals the debuggee
that all iterations have completed.

  Issue: https://bugs.openjdk.java.net/browse/JDK-8068225

diff --git a/test/hotspot/jtreg/ProblemList.txt 
b/test/hotspot/jtreg/ProblemList.txt

--- a/test/hotspot/jtreg/ProblemList.txt
+++ b/test/hotspot/jtreg/ProblemList.txt
@@ -163,7 +163,6 @@
 vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003/TestDescription.java 
8066993 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/EventQueue/remove_l/remove_l005/TestDescription.java 
8068225 generic-all


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



diff --git 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java 

--- 
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java
+++ 
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l005.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2018, Oracle and/or its affiliates. All 
rights reserved.
+ * Copyright (c) 2002, 2019, Oracle and/or its affiliates. All 
rights reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -134,6 +134,7 @@
 }
 display("");
 }
+debugee.sendSignal(SGNL_QUIT);

 display("");
 display("=");

Re: [aarch64-port-dev ] RFR: 8209413: AArch64: NPE in clhsdb jstack command

[Nick Gasson (Arm Technology China)]

Hi Andrew

> Here's the test that reveals the problem: it seems that you need an entry 
> frame which calls compiled Java code.

This seems slightly different to the original problem, although maybe related. 
Because here the top-most frame is a compiled Java frame we'll take the 
vm.isJavaPCDbg and !vm.isClientCompiler branches of 
AARCH64CurrentFrameGuess::run which as far as I can tell always gets the PC 
from the thread context. The original crash I was looking at happened when the 
top frame was native (else branch on line 183) where the PC is set to null 
which causes the two-argument AARCH64Frame constructor to be used.

Unfortunately I'm on holiday until next Thursday so can't test anything. Did 
you try Thread.sleep? That's what LingeredApp that the jtreg tests is trying to 
get a stack trace of calls.

Thanks,
Nick

From: Andrew Haley 
Sent: 06 February 2019 18:00:16
To: Nick Gasson (Arm Technology China); serviceability-dev@openjdk.java.net
Cc: nd; aarch64-port-...@openjdk.java.net
Subject: Re: [aarch64-port-dev ] RFR: 8209413: AArch64: NPE in clhsdb jstack 
command

On 2/5/19 5:57 PM, Andrew Haley wrote:
> On 2/5/19 5:55 PM, Nick Gasson (Arm Technology China) wrote:
>> The stack trace you sent below is incomplete, it's missing the frame for 
>> Spinner.run (i.e. the AARCH64Frame object corresponding to the native 
>> wrapper frame). Did you try on x86 or with the patch I posted? I'm not able 
>> to test at the moment but I think you will get one exra frame in the output. 
>> I don't know why you don't get the NPE, I guess it depends on what's in that 
>> stack slot.
>
> Never mind, found it. A more complex test is needed.

Here's the test that reveals the problem: it seems that you need an entry frame
which calls compiled Java code.

[aph@merino ~]$ cat Spinner.c
#include 
#include 

/*
 * Class: Spinner
 * Method:run
 * Signature: ()V
 */
JNIEXPORT void JNICALL Java_Spinner_run(JNIEnv *env,  jclass cls) {
  jmethodID mid = (*env)->GetStaticMethodID(env, cls, "test", "()V");
  (*env)->CallStaticVoidMethod(env, cls, mid);
  // printf("Spinning...\n");
  // for(;;);
}


[aph@merino ~]$ cat Spinner.java
public class Spinner {
static native void run();

static {
System.loadLibrary("Spinner");
}

static volatile int nn;

public static void test() {
System.out.println("Spinning");
for(int i = 1; i !=0; i = (i + 1) | 1)
nn = (int)Math.log(i);
}

public static void main(String[] args) {
run();
}
}

--
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. 
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: [aarch64-port-dev ] RFR: 8209413: AArch64: NPE in clhsdb jstack command

[Andrew Haley]

On 2/5/19 5:57 PM, Andrew Haley wrote:
> On 2/5/19 5:55 PM, Nick Gasson (Arm Technology China) wrote:
>> The stack trace you sent below is incomplete, it's missing the frame for 
>> Spinner.run (i.e. the AARCH64Frame object corresponding to the native 
>> wrapper frame). Did you try on x86 or with the patch I posted? I'm not able 
>> to test at the moment but I think you will get one exra frame in the output. 
>> I don't know why you don't get the NPE, I guess it depends on what's in that 
>> stack slot.
> 
> Never mind, found it. A more complex test is needed.

Here's the test that reveals the problem: it seems that you need an entry frame
which calls compiled Java code.

[aph@merino ~]$ cat Spinner.c
#include 
#include 

/*
 * Class: Spinner
 * Method:run
 * Signature: ()V
 */
JNIEXPORT void JNICALL Java_Spinner_run(JNIEnv *env,  jclass cls) {
  jmethodID mid = (*env)->GetStaticMethodID(env, cls, "test", "()V");
  (*env)->CallStaticVoidMethod(env, cls, mid);
  // printf("Spinning...\n");
  // for(;;);
}


[aph@merino ~]$ cat Spinner.java
public class Spinner {
static native void run();

static {
System.loadLibrary("Spinner");
}

static volatile int nn;

public static void test() {
System.out.println("Spinning");
for(int i = 1; i !=0; i = (i + 1) | 1)
nn = (int)Math.log(i);
}

public static void main(String[] args) {
run();
}
}

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. 
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: [RFR]8215622: Add dump to file support for jmap histo

[Hohensee, Paul]

Thanks, Serguei (and David)!. I’ve put the CSR into finalized state.

Paul

From: serviceability-dev  on 
behalf of "serguei.spit...@oracle.com" 
Date: Tuesday, February 5, 2019 at 9:23 AM
To: "serviceability-dev@openjdk.java.net" 
Subject: Re: [RFR]8215622: Add dump to file support for jmap histo

Hi Paul,

I'll review the CSR.

Thanks,
Serguei


On 2/5/19 06:02, Hohensee, Paul wrote:

Looks good to me. David, who should review the CSR? Is there someone who "owns" 
jmap or perhaps the serviceability group lead if there is such a person?



Thanks,



Paul



On 1/30/19, 10:43 PM, "臧琳"  wrote:



Hi David, Paul,

I have uploaded the refined webrev at

http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.06/

And submitted a CSR at https://bugs.openjdk.java.net/browse/JDK-8218131

May I ask your help to review?

Thanks!



BRs,

Lin



From: David Holmes 

Sent: Thursday, January 31, 2019 9:19:31 AM

To: Hohensee, Paul; 臧琳; JC Beyler

Cc: 
serviceability-dev@openjdk.java.net

Subject: Re: [RFR]8215622: Add dump to file support for jmap histo



On 31/01/2019 10:41 am, Hohensee, Paul wrote:

> Also, I suspect that this change needs a CSR, since we're adding 
functionality to jmap's histo switch. Opinions?



Yes. Changes to command-line flags need a CSR.



Thanks,

David



> Thanks,

>

> Paul

>

> On 1/30/19, 9:33 AM, "serviceability-dev on behalf of Hohensee, Paul" 

 wrote:

>

>  A nit in TestLoggerWeakRefLeak.java, separate the 2 arguments to 
heapHisto() by a space.

>

>  vm.heapHisto("", "-live")

>

>  Also, the header comment line for getInstanceCountFromHeapHisto() 
should be changed to

>

>   * 'vm.heapHisto("", "-live")' will request a full GC

>

>  In attachListener.cpp, the line

>

>out->print_cr("Invalid argument to dumpheap operation: %s", 
arg1);

>

>  should be

>

>out->print_cr("Invalid argument to inspectheap operation: %s", 
arg1);

>

>  Otherwise looks fine. The two failing tests (the other one was 
test/jdk/java/util/concurrent/locks/Lock/TimedAcquireLeak.java) now pass on my 
mac laptop.

>

>  Paul

>

>  On 1/30/19, 12:18 AM, "臧琳"  
wrote:

>

>  Dear All,

>  This issue mentioned is that test case 
"java/util/logging/TestLoggerWeakRefLeak.java" Failed after applied the webrev.

>  I have identified the root cause of the issue. it is caused 
by 2 problems.

>   1. The path processing in heap_inspection() at 
attachListener.cpp.

>   The problem is that when use jmap -histo:live , the 
path is actually set to "" when it is transfer to socket. so it cause JNI_ERR.

>   I need to modify the logic here that if path[0] == '\0' 
, fall through to the next argument parsing, rather than return JNI_ERR.

>

>   2. Another problem is HotSpotVirtualMachine.java, it has a 
heapHisto() method, and the testcase use vm.heapHisto("-live"), And after the 
patch applied, it should be vm.heapHisto(""/*filepath*/, "-live"), seems we 
need to change the API for handling it.

>

>   I have upload a webrev05:

>   http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.05/

>

>  May I ask your help for review?

>

>  Thanks,

>  Lin

>  

>  From: 臧琳

>  Sent: Monday, January 28, 2019 5:49:42 PM

>  To: Hohensee, Paul; JC Beyler

>  Cc: 
serviceability-dev@openjdk.java.net

>  Subject: Re: [RFR]8215622: Add dump to file support for jmap 
histo

>

>  Dear all,

>   I have found there is problem for handling the "filepath" 
and it cause test "java/util/logging/TestLoggerWeakRefLeak.java" fail, I have 
identified the root cause, please wait for the new update.

>  Thanks!

>

>  BRs,

>  Lin

>  

>  From: 臧琳

>  Sent: Friday, January 25, 2019 9:00:19 AM

>  To: Hohensee, Paul; JC Beyler

>  Cc: 
serviceability-dev@openjdk.java.net

>  Subject: Re: [RFR]8215622: Add dump to file support for jmap 
histo

>

>  Dear All,

>  May I get more review about this webrev?

>