Re: RFR: (S): JDK-8200613: SA: jstack throws UnmappedAddressException with a CDS core file

2018-12-10 Thread Jini George
Thank you very much, Coleen. I have converted the flag into a diagnostic flag. The revised webrev is at: http://cr.openjdk.java.net/~jgeorge/8200613/webrev.03/ A plain diff between this patch and the earlier one is at: http://cr.openjdk.java.net/~jgeorge/8200613/incremental_diff I will withdr

Re: RFR 8215050: [TESTBUG] serviceability/tmtools/jstack/WaitNotifyThreadTest.java fails when run with flag -Xcomp

2018-12-10 Thread David Holmes
Sorry please ignore - "off by one error" checking the CI results. This bug was fixed in 1097 and I was looking at 1096. David On 11/12/2018 2:50 pm, David Holmes wrote: This is still failing in tier4: Error: incorrect monitor info: locked, a java.lang.Object, 0x00078653eeb0 Expected: l

Re: RFR 8215050: [TESTBUG] serviceability/tmtools/jstack/WaitNotifyThreadTest.java fails when run with flag -Xcomp

2018-12-10 Thread David Holmes
This is still failing in tier4: Error: incorrect monitor info: locked, a java.lang.Object, 0x00078653eeb0 Expected: locked, a java.lang.Object, no object reference available Looks like the presence/absence of an address is contingent on other things ?? "MyWaitingThread" #13 prio=5 os_p

Re: RFR (L) 8215160: Normalize spaces for remaining vmTestbase tests

2018-12-10 Thread Alex Menkov
Sorry, it was review for 8215161 Some minor notes for this webrev: - nsk/jvmti/scenarios/jni_interception/JI05/ji05t001/ji05t001.cpp for some reason there are missed spaces before "?" in statements like (indx == 0)? "A" : "B" see lines 190, 227, 243 Also there is some inconsistency with spaces

Re: RFR (L) 8215160: Normalize spaces for remaining vmTestbase tests

2018-12-10 Thread Alex Menkov
+1 One minor request. Could you please fix nsk/jvmti/NativeMethodBind/nativemethbind002/nativemethbind002.cpp replacing if (!(methNam == NULL)) and if (!(methSig == NULL)) with if (methNam != NULL) and if (methSig != NULL) No need for new round of the review. --alex On 12/10/2018 13:54, ser

Re: [PATCH] JDK-8025886: Replace [[ and == bash extensions in tests

2018-12-10 Thread Sergei Ustimenko
Martin, >you probably won't be able to build openjdk on a system without bash Most certainly not, it appears I really wonder now if there's any policy/rule of thumb to not to use /bin/sh. I don't feel really comfortable if there would be such a policy. I've checked build docs and it seems that

Re: RFR (L) 8215161: Normalize spaces for vmTestbase/[a-j]

2018-12-10 Thread Martin Buchholz
Looks good to me. On Mon, Dec 10, 2018 at 1:49 PM JC Beyler wrote: > Hi all, > > Let's try this again; my apologies for the spam. > > Could I get a review that normalizes spaces around comparisons and the > ternary operator? This is the first of two webrevs to handle this. > > Webrev: http://cr.

Re: [PATCH] JDK-8025886: Replace [[ and == bash extensions in tests

2018-12-10 Thread David Holmes
On 11/12/2018 7:27 am, Martin Buchholz wrote: I don't know if there's an official policy on how ultra-portable tests are supposed to be.  In practice, you probably won't be able to build openjdk on a system without bash. configure must be run with bash; so no bash, no build. David On Mon, De

Re: RFR (L) 8215161: Normalize spaces for vmTestbase/[a-j]

2018-12-10 Thread serguei.spit...@oracle.com
Hi Jc, LGTM Thanks, Serguei On 12/10/18 13:48, JC Beyler wrote: Hi all, Let's try this again; my apologies for the spam. Could I get a review that

Re: RFR (L) 8215160: Normalize spaces for remaining vmTestbase tests

2018-12-10 Thread serguei.spit...@oracle.com
Hi Jc, LGTM Thank you for the re-post! Serguei On 12/10/18 13:46, JC Beyler wrote: Hi all, Let's try this again; my apologies for the spam. Could I get a review that no

Re: RFR (L) 8215161: Remove spaces in assignments for vmTestbase/[a-j]

2018-12-10 Thread Martin Buchholz
Looks good to me. Make sure to "test" using hg diff -wbB On Mon, Dec 10, 2018 at 1:04 PM JC Beyler wrote: > Hi all, > > Could I get a review that adds a space around comparisons for the > vmTestbase? This is the first of two webrevs to handle this. > > Webrev: http://cr.openjdk.java.net/~jcbeyl

Re: RFR(L) 8212960: Remove spaces in assignments for remaining vmTestbase

2018-12-10 Thread serguei.spit...@oracle.com
Hi Jc, It looks good. The same comment from Martin applies to this. The bug title does not match the webrev update. One of the options is fix the bug title and re-post the RFR. Thanks, Serguei On 12/10/18 13:02

Re: RFR (L) 8215161: Remove spaces in assignments for vmTestbase/[a-j]

2018-12-10 Thread serguei.spit...@oracle.com
I'd suggest to say: "normalize spaces" or something like this. Thanks, Serguei On 12/10/18 13:31, Martin Buchholz wrote: On Mon, Dec 10, 2018 at 1:28 PM JC Beyler w

Re: RFR (L) 8215161: Remove spaces in assignments for vmTestbase/[a-j]

2018-12-10 Thread Martin Buchholz
On Mon, Dec 10, 2018 at 1:28 PM JC Beyler wrote: > > I know, Claes noticed it too and I modified the title in the bug and the > webrevs but the RFR is out. > You should modify the bug Description to agree with the bug title.

Re: RFR (L) 8215161: Remove spaces in assignments for vmTestbase/[a-j]

2018-12-10 Thread JC Beyler
Hi Martin, I know, Claes noticed it too and I modified the title in the bug and the webrevs but the RFR is out. Should I send a new email thread for 's/Remove/Add' ? Jc On Mon, Dec 10, 2018 at 1:23 PM Martin Buchholz wrote: > The bug title says > Add spaces > but the description says > Remove s

Re: [PATCH] JDK-8025886: Replace [[ and == bash extensions in tests

2018-12-10 Thread Martin Buchholz
I don't know if there's an official policy on how ultra-portable tests are supposed to be. In practice, you probably won't be able to build openjdk on a system without bash. On Mon, Dec 10, 2018 at 1:12 PM Sergei Ustimenko wrote: > Hi Martin, > > That sounds good! > > I've counted all the sh-sh

Re: RFR (L) 8215161: Remove spaces in assignments for vmTestbase/[a-j]

2018-12-10 Thread Martin Buchholz
The bug title says Add spaces but the description says Remove spaces On Mon, Dec 10, 2018 at 1:04 PM JC Beyler wrote: > Hi all, > > Could I get a review that adds a space around comparisons for the > vmTestbase? This is the first of two webrevs to handle this. > > Webrev: http://cr.openjdk.java.

Re: [PATCH] JDK-8214122 Prevent name mangling of jdwpTransport_OnLoad in Windows 32-bit DLL name decoration

2018-12-10 Thread Ali İnce
Hi Alexey, I’ve searched for |GetProcAddress| usages across source code and couldn’t find (hopefully tbh) other occurrences of such mismatches. Regards, Ali > On 7 Dec 2018, at 20:24, Alexey Ivanov wrote: > > Hi Ali, > > On 06/12/2018 22:49, Ali İnce wrote: >> Hi Magnus, Alexey, >> >> I be

Re: [PATCH] JDK-8025886: Replace [[ and == bash extensions in tests

2018-12-10 Thread Sergei Ustimenko
Hi Martin, That sounds good! I've counted all the sh-shebangs and it appears that there are at least 66 of them inside the test/ directory, where only 12 bashes. I've also ran the search in order to identify all the occurrences that use either [[ or == and found only three of them that use "=="

Re: [PATCH] Windows 32-bit DLL name decoration

2018-12-10 Thread Ali İnce
> On 10 Dec 2018, at 15:11, Magnus Ihse Bursie > wrote: > > > > On 2018-12-10 16:02, Alexey Ivanov wrote: >> >> >> On 10/12/2018 10:41, Magnus Ihse Bursie wrote: >>> >>> >>> On 2018-12-07 22:18, Simon Tooke wrote: Looking at the code, it seems (to me) the "correct" thing to do is t

RFR (L) 8215161: Remove spaces in assignments for vmTestbase/[a-j]

2018-12-10 Thread JC Beyler
Hi all, Could I get a review that adds a space around comparisons for the vmTestbase? This is the first of two webrevs to handle this. Webrev: http://cr.openjdk.java.net/~jcbeyler/8215161/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8215161 Thanks, Jc

RFR(L) 8212960: Remove spaces in assignments for remaining vmTestbase

2018-12-10 Thread JC Beyler
Hi all, Could I get a review that adds a space around comparisons for the vmTestbase? This is the second of two webrevs to handle this. Webrev: http://cr.openjdk.java.net/~jcbeyler/8215160/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8215160 Thanks, Jc

Re: [PATCH] JDK-8025886: Replace [[ and == bash extensions in tests

2018-12-10 Thread Martin Buchholz
I would not try to remove all bash-isms from openjdk. Instead I would find instances of /bin/sh that need to be changed to /bin/bash. (Ubuntu's use of /bin/sh -> /bin/dash is technically correct, but caused much suffering https://bugs.launchpad.net/ubuntu/+source/dash/+bug/61463 )

Re: RFR: JDK-8210106: sun/tools/jps/TestJps.java timed out

2018-12-10 Thread Daniel D. Daugherty
On 12/10/18 2:55 AM, David Holmes wrote: Hi Dan, Gary saw a longest time value of 280 seconds in his testing with fastdebug bits. The default timeout value is 120 seconds with a default timeout factor of 4 in Mach5 gives a total timeout of 480 seconds (8 minutes). With Gary's new default timeou

Re: optimize KlassInfoTable size to power of 2

2018-12-10 Thread Andrew Haley
On 12/10/18 2:46 PM, 臧琳 wrote: > Would you like to give more details about the benefit of using hash table > with prime > size? Really? OK. Imagine that your hash table is an exact power of two in size. Reducing the index mod any power of two means using only the lower n bits of the hash value.

Re: [PATCH] Windows 32-bit DLL name decoration

2018-12-10 Thread Magnus Ihse Bursie
On 2018-12-10 16:02, Alexey Ivanov wrote: On 10/12/2018 10:41, Magnus Ihse Bursie wrote: On 2018-12-07 22:18, Simon Tooke wrote: Looking at the code, it seems (to me) the "correct" thing to do is to is to create a Windows-specific version of dbgsysBuildFunName() in linker_md.c, and have i

RE: optimize KlassInfoTable size to power of 2

2018-12-10 Thread 臧琳
Hi Andrew, Thanks for your comments. I think the reason that idiv is issued is that the constant _num_buckets is not used directly for the hash idx calculation. It is the _size which is initialized to be _num_buckets. And I am also puzzling about why the _size is necessary since there is no

RE: RFR (M) 8214892: Delayed starting of debugging via jcmd

2018-12-10 Thread Schmelter, Ralf
Hi Christoph, > I have thought about a more generic name for the option rather than > "onjcmd". What about "standby=y". That would indicate that the > debugging agent is on standby and can be triggered by whatever > means. What do others think? I'm not sure making the name more generic is the rig

Fwd: [PATCH] JDK-8025886: Replace [[ and == bash extensions in tests

2018-12-10 Thread Sergei Ustimenko
Hi, I've recently been working on bug https://bugs.openjdk.java.net/browse/JDK-8214052 that is mainly about cleaning up the [[ and == bash extensions from the tests. I've discovered, that there's a leftover after hg forest consolidation. GeneratePropertyPassword.sh script still uses beforemention

Re: optimize KlassInfoTable size to power of 2

2018-12-10 Thread Andrew Haley
On 12/10/18 6:24 AM, 臧琳 wrote: > My observation is that when “jmap histo” iterating objects , the > object’s klass >is identified and then hash idx in KlassInfoTable’s buckets[] is > calculated by mod > of _num_bucktes, which would issue a heavy instruction idiv on x86 platform. > It >

Re: RFR: (S): JDK-8200613: SA: jstack throws UnmappedAddressException with a CDS core file

2018-12-10 Thread Jini George
Thank you very much, Kevin! - Jini. On 12/10/2018 3:09 PM, Kevin Walls wrote: Thanks for adding the flag Jini, all looks good to me too, Thanks Kevin On 10/12/2018 06:44, Jini George wrote: Thank you very much, Ioi! - Jini. On 12/10/2018 12:01 PM, Ioi Lam wrote: Hi Jini, These changes l

Re: [PATCH] JDK-8214122 Prevent name mangling of jdwpTransport_OnLoad in Windows 32-bit DLL name decoration

2018-12-10 Thread Magnus Ihse Bursie
On 2018-12-07 21:24, Alexey Ivanov wrote: Hi Ali, On 06/12/2018 22:49, Ali İnce wrote: Hi Magnus, Alexey, I believe we won’t be able to get further opinions from serviceability-dev. Unfortunately, no one has replied as of now. Have you found any issues with jdwpTransport_OnLoad after remo

Re: RFR: (S): JDK-8200613: SA: jstack throws UnmappedAddressException with a CDS core file

2018-12-10 Thread Kevin Walls
Thanks for adding the flag Jini, all looks good to me too, Thanks Kevin On 10/12/2018 06:44, Jini George wrote: Thank you very much, Ioi! - Jini. On 12/10/2018 12:01 PM, Ioi Lam wrote: Hi Jini, These changes look good to me. Thanks! - Ioi On 12/7/18 11:22 AM, Jini George wrote: I have

RE: optimize KlassInfoTable size to power of 2

2018-12-10 Thread 臧琳
Our preliminary measurement show the patch introduced about ~5% speed up for jmap �Chisto when iterate ~13GB committed heap. ( with �CXX:+UseG1GC �CXmx180g configured) Just FYI. Cheers, Lin From: 臧琳 Sent: Monday, December 10, 2018 2:24 PM To: serviceability-dev@openjdk.java.net Subject: optimiz