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

Hi Lin, On 31/03/2020 12:39 pm, linzang(臧琳) wrote: Hi David, Henry, Alan, Thanks a lot for your review. I have considered use clock_gettime() first, but I seached the code and found there is a marco SUPPORTS_CLOCK_MONOTONIC guard the usage of it. Which makes me think it may not be av

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

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 "weak hidden" in the patch I reviewed and also some "hidden weak

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

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. diff --git a/src/hotspot/share/ci/ciField.cpp b/src/hotspot/share/ci/ciField.cpp --- a/src/hotspot/share/ci/ciField.cpp +++ b/src/hotspot/

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

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: src/hotspot/

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

Hi Henry and Alan, Here is the updated webrev: http://cr.openjdk.java.net/~lzang/8241638/webrev02/webrev/ Thanks a lot for your help! BRs, Lin On 2020/4/2, 3:20 AM, "Alan Bateman" wrote: On 01/04/2020 18:47, Henry Jen wrote: > Yes, we need an official Reviewer t

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

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 implications

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

On 4/1/20 12:13 PM, Igor Ignatyev wrote: Hi Evgeny, (widening the audience, given this affects not just hotspot compiler, but hotspot tests as well as core libs tests in general) overall that looks good to me. one suggestion, for the ease of failure analysis it might be worth to print out

RFR JDK-8223347 Integration of Vector API (Incubator): Java API, implementation, and tests

Hi, A prior email sent out a request for review of the Vector API in preparation for JEP 338: Vector API (Incubator) [1] to be proposed for target: https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-March/065345.html

Re: Review Request 8238195: Lookup::defineClass should link the class to match the specification

On 01/04/2020 19:43, Mandy Chung wrote: The spec of `Lookup::defineClass` specifies to throw linkage error including `VerifyError` and JDK-8238358 fixes the implementation to match the spec [1]. This patch updates the spec to make it clear as proposed in the CSR:https://bugs.openjdk.java.n

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

On 01/04/2020 18:47, Henry Jen wrote: Yes, we need an official Reviewer to approve. I can help to push the change after that. I looked at the webrev01 but I don't think Lin has published the updated version yet. Happy to Review. -Alan

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

Hi Evgeny, (widening the audience, given this affects not just hotspot compiler, but hotspot tests as well as core libs tests in general) overall that looks good to me. one suggestion, for the ease of failure analysis it might be worth to print out the names of created files, although this migh

Re: RFR 8015413 Reformat line in Class.privateGetPublicFields() to be more debugger-friendly

IMHO the debugger should do a better job for us rather than we change the code to work for the debugger. Mandy On 3/31/20 12:02 AM, Vipin Mv1 wrote: Hi All, Please find the below changes for the issue https://bugs.openjdk.java.net/browse/JDK-8015413 diff -r 53568400fec3 src/java.base/share/

Review Request 8238195: Lookup::defineClass should link the class to match the specification

The spec of `Lookup::defineClass` specifies to throw linkage error including `VerifyError` and JDK-8238358 fixes the implementation to match the spec [1]. This patch updates the spec to make it clear as proposed in the CSR:https://bugs.openjdk.java.net/browse/JDK-8240338.  Also add a new test

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

Yes, we need an official Reviewer to approve. I can help to push the change after that. Cheers, Henry > On Mar 31, 2020, at 9:55 PM, linzang(臧琳) wrote: > > Thanks Henry, > > I agree to leave Solaris as it is, it seems to me there is no strong reason > to remove it. > > So, Do I need more

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

On 4/1/20 7:26 AM, Alan Bateman wrote: Maybe I missed it going by, but why is the isHidden check in the field offset methods on sun.misc.Unsafe rather than the internal Unsafe? The reflection and VarHandle implementation uses jdk.internal.misc.Unsafe to do field access (both read and write

Re: RFR (S): 8241947: Minor comment fixes for system property handling

My mistake, I mistook it for the spec table. On 4/1/20 12:06 PM, Alan Bateman wrote: On 01/04/2020 17:03, Roger Riggs wrote: Hi, Does dropping the "The following properties are guaranteed to be defined:" would seem to be a spec change. It's a comment on a private field, there's no change t

Re: RFR (S): 8241947: Minor comment fixes for system property handling

+1 Mandy On 3/31/20 11:57 AM, Langer, Christoph wrote: Hi Mandy, this is a good suggestion. The listing of system properties at the props field declaration seems somewhat redundant, given that it already exists more exactly and with API normativity in the doc of System::getProperties().

Re: RFR (S): 8241947: Minor comment fixes for system property handling

On 01/04/2020 17:03, Roger Riggs wrote: Hi, Does dropping the "The following properties are guaranteed to be defined:" would seem to be a spec change. It's a comment on a private field, there's no change to the getProperties spec. -Alan

Re: RFR (S): 8241947: Minor comment fixes for system property handling

Hi, Does dropping the "The following properties are guaranteed to be defined:" would seem to be a spec change. Just linking to the properties page makes no such guarantee. Roger On 4/1/20 10:45 AM, Alan Bateman wrote: On 31/03/2020 19:57, Langer, Christoph wrote: Hi Mandy, this is a good su

RE: RFR (S): 8241947: Minor comment fixes for system property handling

Thanks for the review, Alan. I'll push it then. Best regards Christoph > -Original Message- > From: Alan Bateman > Sent: Mittwoch, 1. April 2020 16:46 > To: Langer, Christoph ; Mandy Chung > ; core-libs-dev@openjdk.java.net > Cc: build-dev > Subject: Re: RFR (S): 8241947: Minor comment

Re: RFR: 8238665: Add JFR event for direct memory statistics

Yes, I can sponsor it. Erik > On 1 Apr 2020, at 16:43, Alan Bateman wrote: > > On 31/03/2020 17:22, Denghui Dong wrote: >> Hi Erik, >> >> IMO, one event type per pool is a better choice, because: >> 1. easy filtering and configuration as you said, and I don't think there >> will be too many b

Re: RFR (S): 8241947: Minor comment fixes for system property handling

On 31/03/2020 19:57, Langer, Christoph wrote: Hi Mandy, this is a good suggestion. The listing of system properties at the props field declaration seems somewhat redundant, given that it already exists more exactly and with API normativity in the doc of System::getProperties(). So what do you

Re: RFR: 8238665: Add JFR event for direct memory statistics

On 31/03/2020 17:22, Denghui Dong wrote: Hi Erik, IMO, one event type per pool is a better choice, because: 1. easy filtering and configuration as you said, and I don't think there will  be too many buffer pool types in the future 2. some buffer pools may have extra fields, e.g. direct memory ha

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

On 31/03/2020 20:25, Mandy Chung wrote: Alex's feedback:  rename isHiddenClass to isHidden as it can be a hidden class or interface. `isLocalClass` and `sAnonymousClass` are correct because the Java language only has local classes and anon classes, not local interfaces or anon. interfaces.