Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-02 Thread David Holmes
/ On 4/1/20 11:21 PM, David Holmes wrote: Hi Mandy, On 2/04/2020 3:17 pm, Mandy Chung wrote: Hi David, Thanks for the edits to the comments.   "weak hidden" were missed to be changed to "non-strong hidden".  Here is the patch fixing the comments. There are 33 cases of "

Re: RFR(S): 8241638: launcher time metrics alway report 1 on Linux when _JAVA_LAUNCHER_DEBUG set(Internet mail)

2020-04-02 Thread David Holmes
that provide gethrtime() (like Solaris). It is * also subject to time-of-day changes, but alternatives may not be * known to be available at either build time or run time. No need for updated webrev. Thanks, David BRs, Lin On 2020/4/2, 2:54 PM, "David Holmes" wrote: Hi Lin,

Re: RFR(S): 8241638: launcher time metrics alway report 1 on Linux when _JAVA_LAUNCHER_DEBUG set(Internet mail)

2020-04-02 Thread David Holmes
ut the new function can be called whatever you like as you are defining CounterGet, so calling it gethrtime() is a bit misleading - I suggest getTimeMicros(). Thanks, David BRs, Lin >On 2020/3/31, 8:05 AM, "David Holmes" wrote: On 31/03/2020 4:08 am, Henry Jen wrote: >

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-02 Thread David Holmes
quot;; ",   (same_module) ? "" : sel_klass->class_in_module_of_loader()   ); -    // For private access check if there was a problem with nest host +    // For private access see if there was a problem with nest host // resolution, and if so report that as part of the m

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-01 Thread David Holmes
Hi Mandy, On 1/04/2020 1:01 pm, Mandy Chung wrote: Thanks to the review feedbacks. Updated webrev: http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.04/ I've had a good look through all the hotspot related changes. All looks good. A few minor comments:

Re: RFR(XS): 8174768: Make ProcessTools print executed process output into a separate file

2020-04-01 Thread David Holmes
Thanks for sharing this Igor! I'm not at all sure this is generally what we want for every single test that uses ProcessTools! But I'm willing it to see it trialed. Evgeny: Please run full tier testing at least to tier 6 and ideally beyond before pushing this. There are potential

Re: RFR(S): 8241638: launcher time metrics alway report 1 on Linux when _JAVA_LAUNCHER_DEBUG set

2020-03-30 Thread David Holmes
On 31/03/2020 4:08 am, Henry Jen wrote: Based on my understanding to gethrtime(), the main benefit is not to be affected by settimeofday or adjtime. I think it is probably better to use clock_gettime(CLOCK_MONOTONIC_RAW, ts); which I checked seems to be available on both Linux and Mac.

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-30 Thread David Holmes
Sorry to jump in on this but it caught my eye though I may be missing a larger context ... On 30/03/2020 7:30 pm, serguei.spit...@oracle.com wrote: Hi Mandy, I have just one comment so far.

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-27 Thread David Holmes
Hi Mandy, On 28/03/2020 9:46 am, Mandy Chung wrote: On 3/27/20 4:01 PM, David Holmes wrote: Hi Mandy, On 28/03/2020 8:29 am, Mandy Chung wrote: Hi Vicente, hasNestmateAccess is about VM supports static nestmates on JDK release  >= 11. However this is about javac --release

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-27 Thread David Holmes
Hi Mandy, On 28/03/2020 8:29 am, Mandy Chung wrote: Hi Vicente, hasNestmateAccess is about VM supports static nestmates on JDK release >= 11. However this is about javac --release 14 and the compiled classes may run on JDK 14 that lambda and string concat spin classes that are not

Re: RFR: (T) 8241144 Javadoc is not generated for new module jdk.nio.mapmode

2020-03-24 Thread David Holmes
On 25/03/2020 3:49 am, Florian Weimer wrote: * Magnus Ihse Bursie: On 2020-03-24 09:59, Andrew Dinn wrote: On 23/03/2020 18:38, Erik Joelsson wrote: Looks good. Thanks for the review, Erik. I'm assuming that also implies it is trivial (because, copyright update a side, it really is a

Re: Sponsor Request: 8241100: Make Boolean, Character, Byte, and Short implement Constable

2020-03-18 Thread David Holmes
Hi Jorn, This needs a CSR request before it can be pushed. Thanks, David On 19/03/2020 12:08 am, Jorn Vernee wrote: Hi, Can someone please sponsor this patch that makes Boolean, Character, Byte, and Short implement Constable? Bug: https://bugs.openjdk.java.net/browse/JDK-8241100 Webrev:

Re: Review Request JDK-8228336: Refactor native library loading implementation

2020-03-10 Thread David Holmes
Hi Mandy, Updates seem fine to me. Thanks, David On 10/03/2020 2:53 pm, Mandy Chung wrote: Hi David, On 3/9/20 7:41 PM, David Holmes wrote: That's a core-libs decision but I'm not sure that's a namespace we want to increase usage of. I'm open to other suggestion.  This helper method

Re: Review Request JDK-8228336: Refactor native library loading implementation

2020-03-09 Thread David Holmes
Hi Mandy, On 10/03/2020 9:33 am, Mandy Chung wrote: This patch refactors the native library loading implementation out of ClassLoader and move them to jdk.internal.loader package. This introduces a new NativeLibraries abstraction which loads and registers the loaded native libraries. 

Re: RFR(S): 8232081: Try to link all classes during dynamic CDS dump

2020-03-04 Thread David Holmes
for cleaning that up. David thanks, Calvin On 3/1/20 10:14 PM, David Holmes wrote: Hi Calvin, On 28/02/2020 4:12 pm, Calvin Cheung wrote: Hi David, On 2/27/20 5:40 PM, David Holmes wrote: Hi Calvin, Ioi, Looking good - comments below. A meta-question: normal application classes are rarely

Re: RFR(S): 8232081: Try to link all classes during dynamic CDS dump

2020-03-01 Thread David Holmes
Hi Calvin, On 28/02/2020 4:12 pm, Calvin Cheung wrote: Hi David, On 2/27/20 5:40 PM, David Holmes wrote: Hi Calvin, Ioi, Looking good - comments below. A meta-question: normal application classes are rarely loaded but not linked so I'm a little surprised this is an issue. What is the main

Re: RFR(S): 8232081: Try to link all classes during dynamic CDS dump

2020-02-27 Thread David Holmes
hive, so that we can speedup the subsequent link time of LinkClassApp. (I am adding this info to the bug report for clarification). Thanks for doing that. David Thanks - Ioi On 2/27/20 5:40 PM, David Holmes wrote: Hi Calvin, Ioi, Looking good - comments below. A meta-question: normal

Re: RFR(S): 8232081: Try to link all classes during dynamic CDS dump

2020-02-27 Thread David Holmes
from the archive if they were never linked when the application actually executed ?? On 28/02/2020 10:48 am, Calvin Cheung wrote: Hi David, Ioi, Thanks for your review and suggestions. On 2/26/20 9:46 PM, Ioi Lam wrote: On 2/26/20 7:50 PM, David Holmes wrote: Hi Calvin, Adding core-libs

Re: RFR(S): 8232081: Try to link all classes during dynamic CDS dump

2020-02-26 Thread David Holmes
Hi Calvin, Adding core-libs-dev as you are messing with their code :) On 27/02/2020 1:19 pm, Calvin Cheung wrote: JBS: https://bugs.openjdk.java.net/browse/JDK-8232081 webrev: http://cr.openjdk.java.net/~ccheung/jdk15/8232081/webrev.00/ The proposed changeset for this RFE adds a

Re: RFR(T): 8240134: ProblemList javax/script/Test7.java

2020-02-26 Thread David Holmes
Looks good. Thanks, David On 27/02/2020 9:03 am, Daniel D. Daugherty wrote: Greetings, I'm trying to reduce the noise in the jdk/jdk CI. I'm ProblemListing javax/script/Test7.java on all platforms due to this bug:     JDK-8239361 javax/script/Test7.java failed due to ScriptException    

Re: RFR: 8239563 - Reduce public exports in dynamic libraries built from static JDK libraries

2020-02-26 Thread David Holmes
Bursie <mailto:magnus.ihse.bur...@oracle.com>> wrote: On 2020-02-26 14:31, Bob Vandette wrote: On Feb 26, 2020, at 7:31 AM, Magnus Ihse Bursie <mailto:magnus.ihse.bur...@oracle.com>> wrote: On 2020-02-26 08:41, David Holmes wrote: Hi Bob, Adding build-dev. Thanks for noticing t

Re: [TRIVIAL] Fast-path for String.subsring(n,n)

2020-02-26 Thread David Holmes
On 26/02/2020 11:25 pm, Claes Redestad wrote: On 2020-02-26 14:19, Claes Redestad wrote: I don't think a CSR is required, but bringing it up since others think otherwise. s/since/in case/ An observable change in behaviour of a public API definitely needs a CSR request. Thanks, David (CSR

Re: RFR: 8239563 - Reduce public exports in dynamic libraries built from static JDK libraries

2020-02-25 Thread David Holmes
Hi Bob, Adding build-dev. On 26/02/2020 6:37 am, Bob Vandette wrote: Please review this RFE that alters the visibility of JNI entrypoints to hidden when a shared library is created using static JDK libraries. RFE: https://bugs.openjdk.java.net/browse/JDK-8239563 WEBREV:

Re: VM crashed at StringTable expansion

2020-02-25 Thread David Holmes
Hi, cc'ing core-libs-dev as this seems to me like a case of "don't do that!". Reflection can be used to "shoot oneself in the foot" and I think that is the case here. That said, if we can make the VM more resilient without penalizing all the well behaved code, then we should probably do

Re: JDK-8237818: Typo in Unsafe: resposibility

2020-02-16 Thread David Holmes
Hi Yasumasa, On 17/02/2020 11:38 am, Yasumasa Suenaga wrote: Hi David, On 2020/02/17 8:10, David Holmes wrote: On 17/02/2020 12:40 am, Aya Ebata wrote: Hi, I sent OCA today. So it hasn't been approved yet. I'm waiting for approval. For this trivial contribution an OCA is a not required

Re: JDK-8237818: Typo in Unsafe: resposibility

2020-02-16 Thread David Holmes
On 17/02/2020 12:40 am, Aya Ebata wrote: Hi, I sent OCA today. So it hasn't been approved yet. I'm waiting for approval. For this trivial contribution an OCA is a not required, so this can be sponsored before the OCA goes through processing. Cheers, David regards, Aya Ebata

Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

2020-02-07 Thread David Holmes
Hi Martin, On 8/02/2020 3:10 am, Martin Buchholz wrote: I explored System.identityHashCode (see below). System.identityHashCode is a VM call that is potentially very expensive. In the worst-case it triggers monitor inflation. Cheers, David They appear to be high quality, pre-mixed ints

Re: RFR: 8237521: Memory Access API fixes for 32-bit

2020-01-28 Thread David Holmes
Hi Nick, On 27/01/2020 4:41 pm, Nick Gasson wrote: Hi David, On 01/25/20 06:34 am, David Holmes wrote: I've done this here: http://cr.openjdk.java.net/~ngasson/8237521/webrev.02/ Need to check bytes >= 0 before aligning up so that allocateMemory(-1) still throws an IllegalArgumentExcept

Fwd: New candidate JEP: 371: Hidden Classes

2020-01-24 Thread David Holmes
FYI. Cheers, David --- Begin Message --- https://openjdk.java.net/jeps/371 - Mark --- End Message ---

Re: RFR: 8237521: Memory Access API fixes for 32-bit

2020-01-24 Thread David Holmes
On 24/01/2020 7:58 pm, Nick Gasson wrote: Hi David, On 01/24/20 14:35 pm, David Holmes wrote: How about we align the size up to ADDRESS_SIZE (== HeapWordSize) in Unsafe.allocateMemory() before the call to allocateMemoryChecks(). Like: bytes = ((bytes + ADDRESS_SIZE - 1) & ~(ADDRESS_

Re: RFR: 8237521: Memory Access API fixes for 32-bit

2020-01-23 Thread David Holmes
Hi Nick, On 24/01/2020 3:32 pm, Nick Gasson wrote: Hi David, On 01/24/20 12:17 pm, David Holmes wrote: That said, memory segments are not the only clients of Unsafe::allocateMemory - so if we decided that overflow is an issue that should be handled in clients, fine, but at least it should

Re: RFR: 8237521: Memory Access API fixes for 32-bit

2020-01-23 Thread David Holmes
Hi Maurizio, On 24/01/2020 9:27 am, Maurizio Cimadamore wrote: On 23/01/2020 22:59, David Holmes wrote: On 24/01/2020 12:41 am, Andrew Haley wrote: On 1/23/20 10:22 AM, David Holmes wrote: That aside IIRC the overflow check is not ideal here because we already enter undefined behaviour

Re: RFR: 8237521: Memory Access API fixes for 32-bit

2020-01-23 Thread David Holmes
On 24/01/2020 12:41 am, Andrew Haley wrote: On 1/23/20 10:22 AM, David Holmes wrote: That aside IIRC the overflow check is not ideal here because we already enter undefined behaviour territory inside align_up if we actually overflow. How is that possible? size_t is an unsigned type

Re: RFR: 8237521: Memory Access API fixes for 32-bit

2020-01-23 Thread David Holmes
Hi Nick, I've cc'd Kim as our C++ expert. Kim I have a query below if we're entering UB territory ... On 23/01/2020 6:02 pm, Nick Gasson wrote: Hi David, On 01/23/20 15:46 pm, David Holmes wrote: So on 32-bit size_t is 32-bit and so may have ~half the capacity allowed for by the jlong size

Re: RFR: 8237521: Memory Access API fixes for 32-bit

2020-01-22 Thread David Holmes
Hi Nick :) On 23/01/2020 5:25 pm, David Holmes wrote: On 23/01/2020 5:08 pm, Nick Gasson wrote: Hi Maurizio, Yes, for jdk/jdk. Do I need another Reviewer? Yes - specifically you need a reviewer from Hotspot! cc'ing hotspot-runtime-dev :) So on 32-bit size_t is 32-bit and so may have

Re: RFR(S): 8236111 : narrow allowSmartActionArgs disabling

2019-12-23 Thread David Holmes
Hi Igor, Hotspot changes seem fine. Can't comment on jdk tests. Thanks, David On 24/12/2019 6:42 am, Igor Ignatyev wrote: ping? On Dec 17, 2019, at 11:30 AM, Igor Ignatyev wrote: http://cr.openjdk.java.net/~iignatyev/8236111/webrev.00/ 31 lines changed: 20 ins; 11 del; 0 mod; Hi all,

Re: [15] Review Request: 8235975 Update copyright year to match last edit in jdk repository for 2014/15/16/17/18

2019-12-22 Thread David Holmes
Hi Sergey, I've looked at the hotspot files. Changes seem okay, though for the files with dual-copyrights it isn't clear whether the Oracle copyright or other copyright should be updated for any given change. But it seems to me that if anyone wants their specific copyright updated then it is

Re: RFR: [XS]: 8236183: cleanup Java_jdk_internal_reflect_Reflection_getCallerClass naming - was : RE: running java with LD_DEBUG-tracing

2019-12-18 Thread David Holmes
Hi Matthias, Looks good. Thanks for fixing and finding. :) David On 19/12/2019 12:33 am, Baesken, Matthias wrote: Hello David, Here is a change that adjusts the naming of Java_jdk_internal_reflect_Reflection_getCallerClass - With this change, I checked the output of LD_DEBUG=libs

Re: running java with LD_DEBUG-tracing

2019-12-18 Thread David Holmes
On 18/12/2019 10:43 pm, David Holmes wrote: On 18/12/2019 7:43 pm, Baesken, Matthias wrote: Hello, I  recently worked a bit with  the  "verbose debugging information"  output  about operations of the   dynamic linker  (to sort out some lib loading issues) . See https://docs.ora

Re: running java with LD_DEBUG-tracing

2019-12-18 Thread David Holmes
On 18/12/2019 7:43 pm, Baesken, Matthias wrote: Hello, I recently worked a bit with the "verbose debugging information" output about operations of the dynamic linker (to sort out some lib loading issues) . See https://docs.oracle.com/cd/E19683-01/816-1386/chapter3-33/index.html

Re: [14] RFR(S/T) : 8235866 : bump jtreg requiredVersion to 4.2b16

2019-12-12 Thread David Holmes
Hi Igor, On 13/12/2019 3:19 pm, Igor Ignatyev wrote: http://cr.openjdk.java.net/~iignatyev//8235866/webrev.00 5 lines changed: 0 ins; 0 del; 5 mod Hi all, could you please review this small patch which updates TEST.ROOT in all test suites to require jtreg4.2b16? 8230067[1] updated profiles

Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"

2019-12-07 Thread David Holmes
Looks good! Thanks, David On 7/12/2019 11:34 pm, gerard ziemski wrote: Hi all, Please review this revision 2 of a cleanup, where we remove JDK_GetVersionInfo0 and related code, since we can get build versions directly from within the VM itself. The rev2 differs from rev1 in that it's

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-12-05 Thread David Holmes
Hi Christoph, On 6/12/2019 2:12 am, Langer, Christoph wrote: Hi David, I prepared an updated webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.3/ src/java.base/windows/native/libjava/canonicalize_md.c +// We can't include jdk_util.h here because the file is used in Oracle in

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-12-05 Thread David Holmes
On 6/12/2019 2:06 am, Daniel D. Daugherty wrote: On 12/5/19 8:00 AM, David Holmes wrote: Hi Christoph, On 5/12/2019 9:55 pm, Langer, Christoph wrote: Hi David, thanks again for your efforts. Here is a version that ran successfully through jdk-submit (mach5-one-clanger-JDK-8234185-3

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-12-05 Thread David Holmes
ond review is still needed for the final version of this. Thanks, David Somebody in Oracle could then eventually clean up things by decoupling the installer from OpenJDK's canonicalize_md.c. I'd open a bug for this. Thanks Christoph -Original Message----- From: David Holmes Sent: D

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-12-03 Thread David Holmes
an issue for our install team to pick up. I would ask that this not be pushed for the moment while others figure out how best to handle this. Thanks, David - Cheers Christoph -Original Message----- From: David Holmes Sent: Dienstag, 3. Dezember 2019 03:13 To: Langer, Christ

Re: RFR: JEP 359: Records (Preview) (full code)

2019-12-02 Thread David Holmes
On 3/12/2019 11:08 am, John Rose wrote: On Dec 2, 2019, at 4:36 PM, David Holmes <mailto:david.hol...@oracle.com>> wrote: You are using CHECK_0 in RecordComponent::create but that method returns oop. That seems wrong to me, but I see many other oop returning methods also use CHECK_

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-12-02 Thread David Holmes
Hi Christoph, Can you please post your updated patch for review and I will use it to check the fix for our internal build. Once you have your fix reviewed I will push both fixes together. Thanks, David On 25/11/2019 10:41 pm, David Holmes wrote: Hi Christoph, On 25/11/2019 10:38 pm

Re: RFR: JEP 359: Records (Preview) (full code)

2019-12-02 Thread David Holmes
Hi Vicente, I have also re-examined all the hotspot related code and everything looks fine - and very neat (kudos to you and Harold!). A couple of nits: src/hotspot/share/classfile/classFileParser.cpp src/hotspot/share/prims/jvmtiClassFileReconstituter.cpp Typo: + //u2 attributs_count;

Re: RFR: 8235169: [TESTBUG] java/nio/channels/DatagramChannel/ManySenders.java fails on Ubuntu18.04

2019-12-01 Thread David Holmes
Hi Jie, I think this should be reviewed on net-dev. Cheers, David On 2/12/2019 4:00 pm, Jie Fu wrote: Hi all, The failure was caused by InetAddress.getLocalHost() which returns "127.0.1.1" on our Ubuntu18.04 machines. JBS:    https://bugs.openjdk.java.net/browse/JDK-8235169 Webrev:

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-11-25 Thread David Holmes
? That should be simple enough to do. David Thanks Christoph -Original Message- From: David Holmes Sent: Montag, 25. November 2019 01:02 To: Langer, Christoph ; Alan Bateman ; gerard ziemski Cc: core-libs-dev@openjdk.java.net; hotspot-runtime- d...@openjdk.java.net Subject: Re: RFR

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-11-24 Thread David Holmes
On 25/11/2019 8:45 am, David Holmes wrote: On 25/11/2019 7:49 am, David Holmes wrote: On 25/11/2019 7:33 am, David Holmes wrote: Hi Christoph, On 23/11/2019 12:04 am, Langer, Christoph wrote: Hi, I'd like to push this change. However, running it through jdk-submit shows reproducible

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-11-24 Thread David Holmes
On 25/11/2019 7:49 am, David Holmes wrote: On 25/11/2019 7:33 am, David Holmes wrote: Hi Christoph, On 23/11/2019 12:04 am, Langer, Christoph wrote: Hi, I'd like to push this change. However, running it through jdk-submit shows reproducible errors: Job: mach5-one-clanger-JDK-8234185-1

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-11-24 Thread David Holmes
On 25/11/2019 7:33 am, David Holmes wrote: Hi Christoph, On 23/11/2019 12:04 am, Langer, Christoph wrote: Hi, I'd like to push this change. However, running it through jdk-submit shows reproducible errors: Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189 BuildId: 2019-11-22

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-11-24 Thread David Holmes
Hi Christoph, On 23/11/2019 12:04 am, Langer, Christoph wrote: Hi, I'd like to push this change. However, running it through jdk-submit shows reproducible errors: Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189 BuildId: 2019-11-22-0926373.christoph.langer.source No failed tests

Re: RFR: JEP 367: Remove the Pack200 Tools and API

2019-11-20 Thread David Holmes
Correction ... On 21/11/2019 9:10 am, David Holmes wrote: Hi Vicente, Not sure the best mailing list for this review ... jdk-dev may not be well monitored. Is there a separate review thread for the actual tool removal (jdk.pack) I overlooked the removal of jdk.pack (scrolling too fast

Re: RFR: JEP 367: Remove the Pack200 Tools and API

2019-11-20 Thread David Holmes
ith a help text at: src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties Thanks, Vicente [1] http://cr.openjdk.java.net/~vromero/8234542/webrev.01/ On 11/20/19 6:26 PM, David Holmes wrote: Hi Vicente, On 21/11/2019 9:22 am, Vicente Romero wrote: Hi David, This is the list th

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-11-17 Thread David Holmes
Hi Christoph, This all seems fine to me. One clarification: - /* The appropriate location of getPrefixed() should be io_util_md.c, but -java.lang.instrument package has hardwired canonicalize_md.c into their -dll, to avoid complicate solution such as including io_util_md.c into -

Re: RFR 8233272 : The Class.forName specification should be updated to match the long-standing implementation with respect to class linking

2019-11-17 Thread David Holmes
On 16/11/2019 4:38 am, Brent Christian wrote: On 11/14/19 4:46 PM, Mandy Chung wrote: On 11/14/19 4:42 PM, David Holmes wrote: If you really want to test both positive and negative cases from a clean slate then I would suggest modifying the test slightly and using two @run commands - one

Re: RFR JDK-8233527: Update Lookup::hasPrivateAccess and Lookup::defineClass spec w.r.t. full power lookup

2019-11-14 Thread David Holmes
Hi Mandy, On 15/11/2019 10:51 am, Mandy Chung wrote: On 11/14/19 8:04 AM, Mandy Chung wrote: On 11/14/19 2:33 AM, Alan Bateman wrote: On 14/11/2019 04:57, Mandy Chung wrote: Updated in place: http://cr.openjdk.java.net/~mchung/jdk14/8233527/webrev.02/

Re: RFR 8233272 : The Class.forName specification should be updated to match the long-standing implementation with respect to class linking

2019-11-14 Thread David Holmes
On 15/11/2019 10:33 am, Brent Christian wrote: On 11/14/19 4:12 PM, David Holmes wrote: On 15/11/2019 9:58 am, Brent Christian wrote: http://cr.openjdk.java.net/~bchristi/8233272/webrev-03/ Test is fine. Just one note/clarification:   63 // Loading (but not linking) Container

Re: RFR 8233272 : The Class.forName specification should be updated to match the long-standing implementation with respect to class linking

2019-11-14 Thread David Holmes
Hi Brent, On 15/11/2019 9:58 am, Brent Christian wrote: On 11/14/19 8:22 AM, Mandy Chung wrote: On 11/13/19 10:37 AM, Brent Christian wrote: The spec change looks fine. OK, thanks. +1 from me on spec changes. As for the test, I expect that it simply calls Class.forName("Provider",

Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"

2019-11-14 Thread David Holmes
/Windows were not. The .c file can go. The .h file wouldn't be empty as it still has the other includes. David cheers On 11/13/19 9:21 PM, David Holmes wrote: Hi Gerard, On 14/11/2019 6:05 am, Langer, Christoph wrote: Hi Gerard, generally it looks like a nice cleanup. I've got a patch

Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"

2019-11-13 Thread David Holmes
Hi Gerard, On 14/11/2019 6:05 am, Langer, Christoph wrote: Hi Gerard, generally it looks like a nice cleanup. I've got a patch prepared though, which I was planning on posting tomorrow. It is about cleanup for the canonicalize function in libjava. I wanted to use jdk_util.h for the function

Re: RFR 8233091 : Backout JDK-8212117: Class.forName loads a class but not linked if class is not initialized

2019-10-31 Thread David Holmes
Hi Brent, This looks like an accurate backout to me. Thanks, David - On 1/11/2019 8:50 am, Brent Christian wrote: Hi, Please review my change to backout JDK-8212117: http://cr.openjdk.java.net/~bchristi/8233091/webrev-revert-01/ Background: A couple months ago, JDK-8212117[1] was

Re: RFR: 8229516: Thread.isInterrupted() always returns false after thread termination

2019-10-31 Thread David Holmes
Thanks Doug! Appreciate the help getting the Graal bits correct. David On 31/10/2019 5:47 pm, Doug Simon wrote: On 31 Oct 2019, at 05:30, David Holmes <mailto:david.hol...@oracle.com>> wrote: Hi Doug, On 30/10/2019 10:06 pm, Doug Simon wrote: On 30 Oct 2019, at 05:28, Dav

Re: RFR: 8229516: Thread.isInterrupted() always returns false after thread termination

2019-10-30 Thread David Holmes
Hi Doug, On 30/10/2019 10:06 pm, Doug Simon wrote: On 30 Oct 2019, at 05:28, David Holmes wrote: Hi Doug, Your proposed patch wasn't quite right. I made some adjustments but I'm still having issues with the test, HotSpotMethodSubstitutionTest.testThreadSubstitutions, as I don't see how

Re: RFR: 8229516: Thread.isInterrupted() always returns false after thread termination

2019-10-30 Thread David Holmes
Thanks Serguei! David On 31/10/2019 10:36 am, serguei.spit...@oracle.com wrote: Hi David, The update looks good. Thanks, Serguei On 10/29/19 9:28 PM, David Holmes wrote: Hi Doug, Your proposed patch wasn't quite right. I made some adjustments but I'm still having issues with the test

Re: RFR: 8229516: Thread.isInterrupted() always returns false after thread termination

2019-10-30 Thread David Holmes
Hi Alan, Thanks for taking a look at this. On 31/10/2019 1:33 am, Alan Bateman wrote: On 29/10/2019 07:42, David Holmes wrote: Bug: https://bugs.openjdk.java.net/browse/JDK-8229516 CSR: https://bugs.openjdk.java.net/browse/JDK-8232676 (already approved) webrev: http://cr.openjdk.java.net

Re: 8205132: Remove Thread.countStackFrames()

2019-10-29 Thread David Holmes
On 30/10/2019 2:26 am, Alan Bateman wrote: On 23/10/2019 08:25, Alan Bateman wrote: Thread::countStackFrames has been deprecated for 20+ years and has been marked for-removal since Java SE 9. I'd like to remove it for Java SE 14. It's was never a well-defined method and I've been unable to

Re: RFR JDK-8232724: Remove indirection with calling JNU_NewStringPlatform

2019-10-29 Thread David Holmes
LGTM. Thanks, David On 30/10/2019 1:04 am, Alexey Ivanov wrote: Hi David, Christoph, Thank you for your reviews. On 29/10/2019 07:49, David Holmes wrote: Shall I remove one of them? The one in jvm.h because it's unused? Yes please remove the unused one in jvm.h. The updated webrev

Re: RFR: 8229516: Thread.isInterrupted() always returns false after thread termination

2019-10-29 Thread David Holmes
/~dholmes/8229516/webrev.v2/ Thanks, David - On 29/10/2019 7:14 pm, Doug Simon wrote: On 29 Oct 2019, at 10:12, David Holmes wrote: Hi Doug, Thanks for taking a look so quickly! :) On 29/10/2019 6:55 pm, Doug Simon wrote: Hi David, Since Graal needs to work against multiple JVMCI

Re: RFR: 8229516: Thread.isInterrupted() always returns false after thread termination

2019-10-29 Thread David Holmes
er. Hope that clarifies things. Thanks, David - Rémi - Mail original ----- De: "David Holmes" À: "hotspot-runtime-dev" , "hotspot compiler" , "core-libs-dev" , "Doug Simon" , "TOM.RODRIGUEZ" , "serviceability-dev&q

Re: RFR: 8229516: Thread.isInterrupted() always returns false after thread termination

2019-10-29 Thread David Holmes
Thanks for the review Dan! Fixed the nit in thread.c Thanks, David On 30/10/2019 6:51 am, Daniel D. Daugherty wrote: On 10/29/19 3:42 AM, David Holmes wrote: Bug: https://bugs.openjdk.java.net/browse/JDK-8229516 CSR: https://bugs.openjdk.java.net/browse/JDK-8232676 (already approved) webrev

Re: RFR: 8229516: Thread.isInterrupted() always returns false after thread termination

2019-10-29 Thread David Holmes
. Thanks, Serguei On 10/29/19 00:42, David Holmes wrote: Bug: https://bugs.openjdk.java.net/browse/JDK-8229516 CSR: https://bugs.openjdk.java.net/browse/JDK-8232676 (already approved) webrev: http://cr.openjdk.java.net/~dholmes/8229516/webrev/ This cross-cuts core-libs, hotspot-runtime and the JIT

Re: RFR: 8229516: Thread.isInterrupted() always returns false after thread termination

2019-10-29 Thread David Holmes
otspot/src/org/graalvm/compiler/hotspot/replacements/HotSpotReplacementsUtil.java @@ -308,6 +308,7 @@ public class HotSpotReplacementsUtil { @Fold public static int osThreadOffset(@InjectedParameter GraalHotSpotVMConfig config) { +assert config.instanceKlassInitThreadOffset

Re: RFR JDK-8232724: Remove indirection with calling JNU_NewStringPlatform

2019-10-29 Thread David Holmes
Hi Alexey, On 29/10/2019 4:46 am, Alexey Ivanov wrote: Hello, Please review the following fix which removes indirection in calling JNU_NewStringPlatform via NewStringPlatform. bug: https://bugs.openjdk.java.net/browse/JDK-8232724 webrev:

RFR: 8229516: Thread.isInterrupted() always returns false after thread termination

2019-10-29 Thread David Holmes
Bug: https://bugs.openjdk.java.net/browse/JDK-8229516 CSR: https://bugs.openjdk.java.net/browse/JDK-8232676 (already approved) webrev: http://cr.openjdk.java.net/~dholmes/8229516/webrev/ This cross-cuts core-libs, hotspot-runtime and the JIT compilers, but only in small pieces each. There is

Re: RFR: JDK-8232773: ClassLoading Debug Output for Specific Classes

2019-10-23 Thread David Holmes
Hi Martin, This is going a bit OT I think ... On 24/10/2019 1:24 pm, Martin Buchholz wrote: On Tue, Oct 22, 2019 at 3:58 PM David Holmes <mailto:david.hol...@oracle.com>> wrote: Perhaps one aspect of the class loading/resolution/initialization process that can lead to

Re: 8205132: Remove Thread.countStackFrames()

2019-10-23 Thread David Holmes
Looks good Alan! I'm guessing there may be a JCK update required somewhere to go with this. Thanks, David On 23/10/2019 5:25 pm, Alan Bateman wrote: Thread::countStackFrames has been deprecated for 20+ years and has been marked for-removal since Java SE 9. I'd like to remove it for Java SE

Re: RFR: JDK-8232773: ClassLoading Debug Output for Specific Classes

2019-10-22 Thread David Holmes
the "loading the wrong version of a class" problem, but does not seem to give us information in the case of class loading failure. Also, thanks for moving the bug to the correct component. :) Best Regards Adam Farley IBM Runtimes David Holmes wrote on 22/10/2019 14:12:55: From: D

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread David Holmes
On 23/10/2019 7:52 am, Mandy Chung wrote: On 10/22/19 8:03 AM, Claes Redestad wrote: http://cr.openjdk.java.net/~redestad/8232613/open.03 Looks good to me. Filed and drafted a CSR: https://bugs.openjdk.java.net/browse/JDK-8232801 I think it'd be good to include the IAE message as an

Re: 8231602: Deprecate Thread.suspend/resume for removal

2019-10-22 Thread David Holmes
All looks good to me! :) Thanks, David On 23/10/2019 4:25 am, Alan Bateman wrote: I brought up the issue of deprecating, for removal, Thread.suspend/resume a few weeks ago [1]. Mark said "Got for it", no other replies or objections. Here's the webrev with the changes to

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread David Holmes
On 22/10/2019 11:41 pm, Claes Redestad wrote: Hi Lois, to make sure noone misses it, here's the new webrev: http://cr.openjdk.java.net/~redestad/8232613/open.03 I like the code placement in this version - Coleen's suggestion was better than mine :) Comments inline.. Comments inline ...

Re: RFR: JDK-8232773: ClassLoading Debug Output for Specific Classes

2019-10-22 Thread David Holmes
Hi Adam, Did you look at the logging available from the VM: -Xlog:class+load? BTW I moved the bug you filed to the correct component. Cheers, David On 22/10/2019 8:40 pm, Adam Farley8 wrote: Hey All, This one goes out to anyone who's struggled to figure out why OpenJDK isn't loading their

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread David Holmes
Hi Claes, On 22/10/2019 8:38 pm, Claes Redestad wrote: Hi David, On 2019-10-22 01:08, David Holmes wrote: Hi Claes, My only nit with this is that I don't think register_native and friends belongs in the SystemDictionary class as it has nothing to do with the SD. This code seems to be all

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread David Holmes
Hi Andrew, On 22/10/2019 6:11 pm, Andrew Dinn wrote: Hi Claes/David, On 22/10/2019 00:08, David Holmes wrote: My only nit with this is that I don't think register_native and friends belongs in the SystemDictionary class as it has nothing to do with the SD. This code seems to be all about

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-21 Thread David Holmes
Hi Claes, My only nit with this is that I don't think register_native and friends belongs in the SystemDictionary class as it has nothing to do with the SD. This code seems to be all about Methods so that seems like the place to put this. Thanks, David On 22/10/2019 12:55 am, Claes

Re: RFR JDK-8232624: Java cannot start: NewStringPlatform missing

2019-10-21 Thread David Holmes
+1 On 22/10/2019 4:03 am, Claes Redestad wrote: On 2019-10-21 19:58, Alexey Ivanov wrote: Just to confirm: Is the patch http://cr.openjdk.java.net/~aivanov/8232624/webrev.00/ approved? Ship it! /Claes

Re: Review Request JDK-8232617: Update the outdated code comments in java.lang.System class

2019-10-21 Thread David Holmes
+1 Thanks, David On 22/10/2019 6:33 am, Brent Christian wrote: Looks great. -B On 10/21/19 10:10 AM, Mandy Chung wrote: > Thanks.   Updated:      /* Register the natives via the static initializer.   *   * The VM will invoke the initPhase1 method to complete the initialization    

Re: RFR JDK-8232624: Java cannot start: NewStringPlatform missing

2019-10-21 Thread David Holmes
On 21/10/2019 10:14 pm, Alexey Ivanov wrote: Hi David, On 21/10/2019 02:19, David Holmes wrote: Hi Alexey, On 21/10/2019 9:37 am, Alexey Ivanov wrote: Hi David, On 20/10/2019 23:59, David Holmes wrote: Hi Alexey, On 21/10/2019 2:20 am, Alexey Ivanov wrote: Hello, Please review

Re: RFR JDK-8232624: Java cannot start: NewStringPlatform missing

2019-10-20 Thread David Holmes
Hi Alexey, On 21/10/2019 9:37 am, Alexey Ivanov wrote: Hi David, On 20/10/2019 23:59, David Holmes wrote: Hi Alexey, On 21/10/2019 2:20 am, Alexey Ivanov wrote: Hello, Please review the following fix which it brings back NewStringPlatform alias for JNU_NewStringPlatform. Without it, 32

Re: RFR JDK-8232624: Java cannot start: NewStringPlatform missing

2019-10-20 Thread David Holmes
Hi Alexey, On 21/10/2019 2:20 am, Alexey Ivanov wrote: Hello, Please review the following fix which it brings back NewStringPlatform alias for JNU_NewStringPlatform. Without it, 32 bit Windows build of Java does not work. bug: https://bugs.openjdk.java.net/browse/JDK-8232624 webrev:

Re: Review Request JDK-8232617: Update the outdated code comments in java.lang.System class

2019-10-20 Thread David Holmes
On 19/10/2019 10:38 am, Mandy Chung wrote: thanks. I think "separated from" reads right to me.  I will leave it as is. I think "separately" or "separate" seems more correct here. And if we're picking on grammar then it should be "The VM ..." and "of this class" rather than "for this class".

Re: Loading classes during VM shutdown

2019-10-20 Thread David Holmes
Hi Ioi, On 19/10/2019 3:36 am, Ioi Lam wrote: For JDK-8232081 (Try to link all classes with ArchiveClassesAtExit), when creating a dynamic CDS archive, I need to link all classes in the "loaded" state. I am thinking of doing it at this point:

Re: RFR (S) 8232168: Fix non wide char canonicalization on Windows

2019-10-17 Thread David Holmes
, Ralf -Original Message- From: David Holmes Sent: Mittwoch, 16. Oktober 2019 05:59 To: Schmelter, Ralf ; hotspot-runtime-...@openjdk.java.net Subject: Re: RFR (S) 8232168: Fix non wide char canonicalization on Windows Hi Ralf, This isn't a hotspot issue but a core-libs one. The use

Re: RFR of JDK-8134599: Improve the code coverage for ThreadLocal

2019-10-14 Thread David Holmes
Hi Hamlin, On 15/10/2019 12:03 pm, Hamlin Li wrote: Could some help to review it? Basic test seems okay but a few minor suggestions: 1. Please add a public summary to the bug report to convey the same information as: 27 * @summary per latest JDK code coverage report, 2 methods

Re: RFR: 8231355: Remove unused utility methods in libjava

2019-10-07 Thread David Holmes
Hi Claes, This all looks good to me! Thanks, David On 8/10/2019 4:38 am, Claes Redestad wrote: Hi, please review this patch which removes unused methods and redundant aliases from jni_util and friends in libjava. There's one affected call site in the hotspot runtime code, otherwise this is

Re: Regression after jpackage+1-49 update

2019-10-07 Thread David Holmes
On 7/10/2019 4:43 pm, Alan Bateman wrote: On 07/10/2019 04:35, David Holmes wrote: Nothing to do with jpackage but a fix to Class.forName to ensure classes are linked as per the specification of forName. You can temporarily workaround with -XX:+ClassForNameDeferLinking, but your code needs

Re: Regression after jpackage+1-49 update

2019-10-06 Thread David Holmes
Hi, On 7/10/2019 1:21 am, Tornai András wrote: Hello, After I updated my project from jpackage+1-35 to the latest jpackage+1-49 I noticed that Java now throws NoClassDefFoundError while this was not the case before. Nothing to do with jpackage but a fix to Class.forName to ensure classes

<    1   2   3   4   5   6   7   8   9   10   >