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

2020-02-27 Thread Ioi Lam
On 2/27/20 10:14 PM, Calvin Cheung wrote: Hi Ioi, On 2/27/20 8:22 PM, Ioi Lam wrote: Hi Calvin, This looks good to me. Thanks for taking another look. I would suggest adding the something like this to the test case: class Child extends Parent {     int get() {return 2;}     int dummy()

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

2020-02-27 Thread Calvin Cheung
Hi Ioi, On 2/27/20 8:22 PM, Ioi Lam wrote: Hi Calvin, This looks good to me. Thanks for taking another look. I would suggest adding the something like this to the test case: class Child extends Parent {     int get() {return 2;}     int dummy() {     // When Child is linked during

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

2020-02-27 Thread Calvin Cheung
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 source of such classes - generated classes like lambda

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

2020-02-27 Thread David Holmes
Hi Ioi, On 28/02/2020 2:09 pm, Ioi Lam wrote: Hi David, There classes are usually loaded during verification. They are not linked because they are referenced only by methods that were never executed during dynamic dumping. You can see an example here

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

2020-02-27 Thread Ioi Lam
Hi Calvin, This looks good to me. I would suggest adding the something like this to the test case: class Child extends Parent {     int get() {return 2;}     int dummy() {     // When Child is linked during dynamic dump (when the VM is shutting         // down), it will be verified, which

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

2020-02-27 Thread Ioi Lam
Hi David, There classes are usually loaded during verification. They are not linked because they are referenced only by methods that were never executed during dynamic dumping. You can see an example here

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

2020-02-27 Thread David Holmes
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 source of such classes - generated classes like lambda forms? Also why do we need to link them when loading

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

2020-02-27 Thread Calvin Cheung
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-dev as you are messing with their code :) On 27/02/2020 1:19 pm, Calvin Cheung wrote: JBS:

[15] RFR: 8239836: ZoneRules.of() doesn't check transitionList/standardOffsetTL arguments validity

2020-02-27 Thread naoto . sato
Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8239836 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8239836/webrev.00/ It turned out that 'transitionList' is not necessarily a superset of

Re: RFR(s): 8240189: [TESTBUG] Some cgroup tests are failing after JDK-8231111

2020-02-27 Thread Bob Vandette
The updates look fine to me. Have you scanned through all tests looking for “.length” and [0] to see if we have any more cases that need updating? Bob. > On Feb 27, 2020, at 2:31 PM, Severin Gehwolf wrote: > > Hi, > > Could somebody please review those test fixes for the Metrics platform >

RFR(s): 8240189: [TESTBUG] Some cgroup tests are failing after JDK-8231111

2020-02-27 Thread Severin Gehwolf
Hi, Could somebody please review those test fixes for the Metrics platform tests? Some code paths are apparently not being executed on "common" platforms which didn't make those issues show up earlier :-/ I've missed to update some hard-coded values to account for changes done via JDK-823.

Re: RFR: JDK-8237966,: Creating runtime pkg requires --mac-package-identifier

2020-02-27 Thread Alexey Semenyuk
Looks good. - Alexey On 2/27/2020 10:02 AM, Andy Herrick wrote: sure - updated webrev [3] makes that private and static. [3] http://cr.openjdk.java.net/~herrick/8237966/webrev.02/ /Andy On 2/26/2020 5:01 PM, Alexey Semenyuk wrote: Looks good. Minor note: I'd declare

Re: RFR: 8239965: XMLEncoder/Test4625418.java fails due to "Error: Cp943 - can't read properly"

2020-02-27 Thread naoto . sato
Looks good. Naoto On 2/27/20 6:00 AM, Ichiroh Takiguchi wrote: Hello Naoto. I appreciate your comments. And sorry for my easy mistake. Could you review the fix again ? Bug:    https://bugs.openjdk.java.net/browse/JDK-8239965 Change: https://cr.openjdk.java.net/~itakiguchi/8239965/webrev.01/

Re: RFR: JDK-8237966,: Creating runtime pkg requires --mac-package-identifier

2020-02-27 Thread Andy Herrick
sure - updated webrev [3] makes that private and static. [3] http://cr.openjdk.java.net/~herrick/8237966/webrev.02/ /Andy On 2/26/2020 5:01 PM, Alexey Semenyuk wrote: Looks good. Minor note: I'd declare isValidBundleIdentifier() as static private. - Alexey On 2/26/2020 12:48 PM, Andy

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

2020-02-27 Thread Bob Vandette
> On Feb 27, 2020, at 9:59 AM, Magnus Ihse Bursie > wrote: > > On 2020-02-27 15:52, Bob Vandette wrote: >> >>> On Feb 27, 2020, at 7:15 AM, Magnus Ihse Bursie >>> >>> wrote: >>> >>> On 2020-02-26 22:01, Bob Vandette wrote: >>> Here’s an updated webrev that implementes the

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

2020-02-27 Thread Magnus Ihse Bursie
On 2020-02-27 15:52, Bob Vandette wrote: On Feb 27, 2020, at 7:15 AM, Magnus Ihse Bursie wrote: On 2020-02-26 22:01, Bob Vandette wrote: Here’s an updated webrev that implementes the suggestion that allows JNIEXPORT in jni.h to be overridden and the build limits visibility for static

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

2020-02-27 Thread Bob Vandette
> On Feb 27, 2020, at 7:15 AM, Magnus Ihse Bursie > wrote: > > On 2020-02-26 22:01, Bob Vandette wrote: >> Here’s an updated webrev that implementes the suggestion that allows >> JNIEXPORT in jni.h to be overridden >> and the build limits visibility for static libraries. >> >> If this

Re: RFR: 8239965: XMLEncoder/Test4625418.java fails due to "Error: Cp943 - can't read properly"

2020-02-27 Thread Ichiroh Takiguchi
Hello Naoto. I appreciate your comments. And sorry for my easy mistake. Could you review the fix again ? Bug:https://bugs.openjdk.java.net/browse/JDK-8239965 Change: https://cr.openjdk.java.net/~itakiguchi/8239965/webrev.01/ Thanks, Ichiroh Takiguchi On 2020-02-27 05:56,

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

2020-02-27 Thread Magnus Ihse Bursie
On 2020-02-26 22:01, Bob Vandette wrote: Here’s an updated webrev that implementes the suggestion that allows JNIEXPORT in jni.h to be overridden and the build limits visibility for static libraries. If this webrev is accepted, I’ll update the CSR solution to match this implementation.

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

2020-02-27 Thread Magnus Ihse Bursie
On 2020-02-27 00:31, David Holmes wrote: Hi Bob, On 27/02/2020 7:01 am, Bob Vandette wrote: Here’s an updated webrev that implementes the suggestion that allows JNIEXPORT in jni.h to be overridden and the build limits visibility for static libraries. !   #if (defined(__GNUC__) && ((__GNUC__