Re: RFR JDK-8225474: JDI connector accept fails "Address already in use" with concurrent listeners

2019-06-07 Thread serguei.spit...@oracle.com
Hi Andrew, It looks good to me. Thank you for your investigation and taking care about this! Thanks, Serguei On 6/7/19 07:24, Andrew Leonard wrote: Hi, Please can I request a sponsor and review of thi

Re: RFR JDK-8225474: JDI connector accept fails "Address already in use" with concurrent listeners

2019-06-07 Thread Alex Menkov
Hi Andrew, The fix looks good in general. And I can be the sponsor. I've tested the fix and got 1 test failure (of 400 runs) on Win-x64 and the log looks the same. java.lang.RuntimeException: Failed to accept connector connection at JdwpConcurrentAttachTest.main(JdwpConcurrentAttachTes

Re: RFR(S): 8225388: Running jcmd Compiler.CodeHeap_Analytics all 0 cause crash.

2019-06-07 Thread Leonid Mesnik
Thank you for review. Leonid > On Jun 7, 2019, at 11:29 AM, serguei.spit...@oracle.com wrote: > > On 6/7/19 11:23, Leonid Mesnik wrote: >> I read documentation about CRS on following wiki >> https://wiki.openjdk.java.net/display/csr >> . It says that

Re: RFR(S): 8225388: Running jcmd Compiler.CodeHeap_Analytics all 0 cause crash.

2019-06-07 Thread Leonid Mesnik
Thank you for review. Leonid > On Jun 7, 2019, at 1:07 AM, Tobias Hartmann > wrote: > > Hi Leonid, > > this looks good to me! Please remove the extra whitespace at the end of > compileBroker.hpp:420 before > the ")". No new webrev required. > > Thanks, > Tobias > > On 07.06.19 02:36, Leoni

Re: RFR: 8222828: vmTestbase/nsk/jdi/BScenarios/multithrd/tc02x004/TestDescription.java failed

2019-06-07 Thread Chris Plummer
Looks good. Chris On 6/7/19 11:47 AM, Daniil Titov wrote: Please review the change that fixes an intermittent failure of the test when it is run with Graal on. The test starts a debuggee and sets the method entry breakpoint for tc02x004aClass1 class that has only constructor defined. The deb

Re: RFR: 8222828: vmTestbase/nsk/jdi/BScenarios/multithrd/tc02x004/TestDescription.java failed

2019-06-07 Thread gary.ad...@oracle.com
Looks good to me. Any other graal test timeouts that might be addressed in a similar manner? On 6/7/19 2:47 PM, Daniil Titov wrote: Please review the change that fixes an intermittent failure of the test when it is run with Graal on. The test starts a debuggee and sets the method entry breakp

Re: RFR: 8222828: vmTestbase/nsk/jdi/BScenarios/multithrd/tc02x004/TestDescription.java failed

2019-06-07 Thread Jean Christophe Beyler
Hi Daniil, Looks good to me :) Thanks, Jc On Fri, Jun 7, 2019 at 11:47 AM Daniil Titov wrote: > Please review the change that fixes an intermittent failure of the test > when it is run with Graal on. > > The test starts a debuggee and sets the method entry breakpoint for > tc02x004aClass1 clas

RFR: 8222828: vmTestbase/nsk/jdi/BScenarios/multithrd/tc02x004/TestDescription.java failed

2019-06-07 Thread Daniil Titov
Please review the change that fixes an intermittent failure of the test when it is run with Graal on. The test starts a debuggee and sets the method entry breakpoint for tc02x004aClass1 class that has only constructor defined. The debuggee starts 3 threads and each of them creates a new instance

Re: RFR(S): 8225388: Running jcmd Compiler.CodeHeap_Analytics all 0 cause crash.

2019-06-07 Thread serguei.spit...@oracle.com
On 6/7/19 11:23, Leonid Mesnik wrote: I read documentation about CRS on following wiki https://wiki.openjdk.java.net/display/csr. It says that CRS requires if command-line options are changes. However I believe that fix don't change actual behavior of '

Re: RFR(S): 8225388: Running jcmd Compiler.CodeHeap_Analytics all 0 cause crash.

2019-06-07 Thread Leonid Mesnik
I read documentation about CRS on following wiki https://wiki.openjdk.java.net/display/csr . It says that CRS requires if command-line options are changes. However I believe that fix don't change actual behavior of 'granularity' option so no CSR is nee

Re: RFR(S): 8225388: Running jcmd Compiler.CodeHeap_Analytics all 0 cause crash.

2019-06-07 Thread serguei.spit...@oracle.com
Okay then. I wonder now, if a CSR needs to be filed for this change. Thanks, Serguei On 6/7/19 09:10, Leonid Mesnik wrote: Hi Currently  DCmdArgument is used to parse any numeric values in dcmd framework in

Re: RFR JDK-8225474: JDI connector accept fails "Address already in use" with concurrent listeners

2019-06-07 Thread Robert Field
Thanks Andrew! Can I add my support for this.  I imagine this could be the cause of JShell failures in high-concurrency testing (JShell uses JDI, the failures are in connection). This is not a review, as I haven't been on the JPDA team for a LONG time. Just a request that this be prioritized

Re: RFR(S): 8225388: Running jcmd Compiler.CodeHeap_Analytics all 0 cause crash.

2019-06-07 Thread Leonid Mesnik
Hi Currently DCmdArgument is used to parse any numeric values in dcmd framework including positive integer values for port, ttl etc. Adding new type DCmdArgument requires adding more specialized methods like template <> void DCmdArgument::parse_value(const char* str,

RFR JDK-8225474: JDI connector accept fails "Address already in use" with concurrent listeners

2019-06-07 Thread Andrew Leonard
Hi, Please can I request a sponsor and review of this patch for JDK-8225474. A problem that we have seen intermittently with JDI connector stress tests for quite a while that is caused by an non-thread safe HashMap in the connector class. I've created a standalone testcase that reproduces it re

Re: RFR(S): 8225388: Running jcmd Compiler.CodeHeap_Analytics all 0 cause crash.

2019-06-07 Thread serguei.spit...@oracle.com
Hi Leonid, It looks good to me. One minor comment on the src/hotspot/share/services/diagnosticCommand.?pp + DCmdArgument _granularity; I'm curios if using size_t instead of jlong as in other files would be more unified. Thanks, Serguei

Re: RFR(S): 8225388: Running jcmd Compiler.CodeHeap_Analytics all 0 cause crash.

2019-06-07 Thread Tobias Hartmann
Hi Leonid, this looks good to me! Please remove the extra whitespace at the end of compileBroker.hpp:420 before the ")". No new webrev required. Thanks, Tobias On 07.06.19 02:36, Leonid Mesnik wrote: > Hi > > Could you please review following fix which verify parameter for > Compiler.CodeHeap