Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
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 simpler due to JDK-8234185, which was just checked in yesterday. Waiting for this fix to go in first, allowed us not to have to delete the jdk_util.h file, which in turn means that we don't have to touch all those files that used that include. bug: https://bugs.openjdk.java.net/browse/JDK-8223261 webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev2 tests: passes Mach5 tier1,2,3 (another tier1,2,3,4,5,6 in progress...) cheers
Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
Looks good. Mandy On 12/7/19 5:34 AM, 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 simpler due to JDK-8234185, which was just checked in yesterday. Waiting for this fix to go in first, allowed us not to have to delete the jdk_util.h file, which in turn means that we don't have to touch all those files that used that include. bug: https://bugs.openjdk.java.net/browse/JDK-8223261 webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev2 tests: passes Mach5 tier1,2,3 (another tier1,2,3,4,5,6 in progress...) cheers
RE: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
Hi Gerard, this looks good. Cheers Christoph > -Original Message- > From: gerard ziemski > Sent: Samstag, 7. Dezember 2019 14:34 > To: hotspot-dev developers ; core-libs- > d...@openjdk.java.net; Claes Redestad ; > Mandy Chung ; David Holmes > ; Sergey Bylokhov > ; Langer, Christoph > > Subject: Re: RFR (M) 8223261 "JDK-8189208 followup - remove > JDK_GetVersionInfo0 and the supporting code" > > 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 simpler due to JDK-8234185, > which was just checked in yesterday. Waiting for this fix to go in > first, allowed us not to have to delete the jdk_util.h file, which in > turn means that we don't have to touch all those files that used that > include. > > > bug: https://bugs.openjdk.java.net/browse/JDK-8223261 > webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev2 > tests: passes Mach5 tier1,2,3 (another tier1,2,3,4,5,6 in progress...) > > > cheers
Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
Looks good to me! /Claes On 2019-12-07 14:34, 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 simpler due to JDK-8234185, which was just checked in yesterday. Waiting for this fix to go in first, allowed us not to have to delete the jdk_util.h file, which in turn means that we don't have to touch all those files that used that include. bug: https://bugs.openjdk.java.net/browse/JDK-8223261 webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev2 tests: passes Mach5 tier1,2,3 (another tier1,2,3,4,5,6 in progress...) cheers
Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
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 simpler due to JDK-8234185, which was just checked in yesterday. Waiting for this fix to go in first, allowed us not to have to delete the jdk_util.h file, which in turn means that we don't have to touch all those files that used that include. bug: https://bugs.openjdk.java.net/browse/JDK-8223261 webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev2 tests: passes Mach5 tier1,2,3 (another tier1,2,3,4,5,6 in progress...) cheers
Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
On 15/11/2019 1:13 am, gerard ziemski wrote: Thank you for the review. I'm definitively going to wait for Christoph to check in his fix first. I tried in fact to leave jdk_util.c/.h files in empty without removing them from the repo, and even though Mac/Linux were OK with that, Solaris/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 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 prototype. I had not yet filed a bug but here is what I have: http://cr.openjdk.java.net/~clanger/webrevs/cleanup-canonicalize/ So maybe you could refrain from removing jdk_util.h or maybe you can hold off submitting your change until my cleanup is reviewed? I'd also suggest not deleting jdk_util.h. It seems very odd to me to have jdk_util_md.h with no shared jdk_util.h. If you keep jdk_util.h then you don't need to touch a number of the files. Otherwise this looks like a good cleanup. Thanks, David - I'll create a bug and post an official review thread tomorrow... Thanks Christoph -Original Message- From: hotspot-dev On Behalf Of Mandy Chung Sent: Mittwoch, 13. November 2019 20:30 To: gerard ziemski Cc: awt-...@openjdk.java.net; hotspot-dev developers ; core-libs-dev@openjdk.java.net Subject: Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code" On 11/13/19 10:50 AM, gerard ziemski wrote: Hi all, Please review this cleanup, where we remove JDK_GetVersionInfo0 and related code, since we can get build versions directly from within the VM itself: I'm including core-libs and awt in this review because the proposed fix touches their corresponding files. bug: https://bugs.openjdk.java.net/browse/JDK-8223261 webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev1 tests: passes Mach5 tier1,2,3,4,5,6 This is a good clean up. JDK_GetVersionInfo0 was needed long time ago in particular in HS express support that is no longer applicable. One leftover comment should also be removed. src/hotspot/share/runtime/vm_version.hpp // Gets the jvm_version_info.jvm_version defined in jvm.h otherwise looks good. Mandy
Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
Thank you for the review. I'm definitively going to wait for Christoph to check in his fix first. I tried in fact to leave jdk_util.c/.h files in empty without removing them from the repo, and even though Mac/Linux were OK with that, Solaris/Windows were not. 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 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 prototype. I had not yet filed a bug but here is what I have: http://cr.openjdk.java.net/~clanger/webrevs/cleanup-canonicalize/ So maybe you could refrain from removing jdk_util.h or maybe you can hold off submitting your change until my cleanup is reviewed? I'd also suggest not deleting jdk_util.h. It seems very odd to me to have jdk_util_md.h with no shared jdk_util.h. If you keep jdk_util.h then you don't need to touch a number of the files. Otherwise this looks like a good cleanup. Thanks, David - I'll create a bug and post an official review thread tomorrow... Thanks Christoph -Original Message- From: hotspot-dev On Behalf Of Mandy Chung Sent: Mittwoch, 13. November 2019 20:30 To: gerard ziemski Cc: awt-...@openjdk.java.net; hotspot-dev developers ; core-libs-dev@openjdk.java.net Subject: Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code" On 11/13/19 10:50 AM, gerard ziemski wrote: Hi all, Please review this cleanup, where we remove JDK_GetVersionInfo0 and related code, since we can get build versions directly from within the VM itself: I'm including core-libs and awt in this review because the proposed fix touches their corresponding files. bug: https://bugs.openjdk.java.net/browse/JDK-8223261 webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev1 tests: passes Mach5 tier1,2,3,4,5,6 This is a good clean up. JDK_GetVersionInfo0 was needed long time ago in particular in HS express support that is no longer applicable. One leftover comment should also be removed. src/hotspot/share/runtime/vm_version.hpp // Gets the jvm_version_info.jvm_version defined in jvm.h otherwise looks good. Mandy
Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
Thank you for the review. I'm happy to wait for your fix to go in first - this will make my fix smaller. cheers On 11/13/19 2:05 PM, 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 prototype. I had not yet filed a bug but here is what I have: http://cr.openjdk.java.net/~clanger/webrevs/cleanup-canonicalize/ So maybe you could refrain from removing jdk_util.h or maybe you can hold off submitting your change until my cleanup is reviewed? I'll create a bug and post an official review thread tomorrow... Thanks Christoph -Original Message- From: hotspot-dev On Behalf Of Mandy Chung Sent: Mittwoch, 13. November 2019 20:30 To: gerard ziemski Cc: awt-...@openjdk.java.net; hotspot-dev developers ; core-libs-dev@openjdk.java.net Subject: Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code" On 11/13/19 10:50 AM, gerard ziemski wrote: Hi all, Please review this cleanup, where we remove JDK_GetVersionInfo0 and related code, since we can get build versions directly from within the VM itself: I'm including core-libs and awt in this review because the proposed fix touches their corresponding files. bug: https://bugs.openjdk.java.net/browse/JDK-8223261 webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev1 tests: passes Mach5 tier1,2,3,4,5,6 This is a good clean up. JDK_GetVersionInfo0 was needed long time ago in particular in HS express support that is no longer applicable. One leftover comment should also be removed. src/hotspot/share/runtime/vm_version.hpp // Gets the jvm_version_info.jvm_version defined in jvm.h otherwise looks good. Mandy
Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
Thank you for the review, will remove the comment and updated the webrev, but only after Christop Langer gets his fix in - I'm going to wait for him to check in first. cheers On 11/13/19 1:29 PM, Mandy Chung wrote: On 11/13/19 10:50 AM, gerard ziemski wrote: Hi all, Please review this cleanup, where we remove JDK_GetVersionInfo0 and related code, since we can get build versions directly from within the VM itself: I'm including core-libs and awt in this review because the proposed fix touches their corresponding files. bug: https://bugs.openjdk.java.net/browse/JDK-8223261 webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev1 tests: passes Mach5 tier1,2,3,4,5,6 This is a good clean up. JDK_GetVersionInfo0 was needed long time ago in particular in HS express support that is no longer applicable. One leftover comment should also be removed. src/hotspot/share/runtime/vm_version.hpp // Gets the jvm_version_info.jvm_version defined in jvm.h otherwise looks good. Mandy
Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
Looks fine. On 11/13/19 10:50 am, gerard ziemski wrote: Hi all, Please review this cleanup, where we remove JDK_GetVersionInfo0 and related code, since we can get build versions directly from within the VM itself: I'm including core-libs and awt in this review because the proposed fix touches their corresponding files. bug: https://bugs.openjdk.java.net/browse/JDK-8223261 webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev1 tests: passes Mach5 tier1,2,3,4,5,6 cheers -- Best regards, Sergey.
Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
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 prototype. I had not yet filed a bug but here is what I have: http://cr.openjdk.java.net/~clanger/webrevs/cleanup-canonicalize/ So maybe you could refrain from removing jdk_util.h or maybe you can hold off submitting your change until my cleanup is reviewed? I'd also suggest not deleting jdk_util.h. It seems very odd to me to have jdk_util_md.h with no shared jdk_util.h. If you keep jdk_util.h then you don't need to touch a number of the files. Otherwise this looks like a good cleanup. Thanks, David - I'll create a bug and post an official review thread tomorrow... Thanks Christoph -Original Message- From: hotspot-dev On Behalf Of Mandy Chung Sent: Mittwoch, 13. November 2019 20:30 To: gerard ziemski Cc: awt-...@openjdk.java.net; hotspot-dev developers ; core-libs-dev@openjdk.java.net Subject: Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code" On 11/13/19 10:50 AM, gerard ziemski wrote: Hi all, Please review this cleanup, where we remove JDK_GetVersionInfo0 and related code, since we can get build versions directly from within the VM itself: I'm including core-libs and awt in this review because the proposed fix touches their corresponding files. bug: https://bugs.openjdk.java.net/browse/JDK-8223261 webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev1 tests: passes Mach5 tier1,2,3,4,5,6 This is a good clean up. JDK_GetVersionInfo0 was needed long time ago in particular in HS express support that is no longer applicable. One leftover comment should also be removed. src/hotspot/share/runtime/vm_version.hpp // Gets the jvm_version_info.jvm_version defined in jvm.h otherwise looks good. Mandy
RE: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
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 prototype. I had not yet filed a bug but here is what I have: http://cr.openjdk.java.net/~clanger/webrevs/cleanup-canonicalize/ So maybe you could refrain from removing jdk_util.h or maybe you can hold off submitting your change until my cleanup is reviewed? I'll create a bug and post an official review thread tomorrow... Thanks Christoph > -Original Message- > From: hotspot-dev On Behalf Of > Mandy Chung > Sent: Mittwoch, 13. November 2019 20:30 > To: gerard ziemski > Cc: awt-...@openjdk.java.net; hotspot-dev developers d...@openjdk.java.net>; core-libs-dev@openjdk.java.net > Subject: Re: RFR (M) 8223261 "JDK-8189208 followup - remove > JDK_GetVersionInfo0 and the supporting code" > > > > On 11/13/19 10:50 AM, gerard ziemski wrote: > > Hi all, > > > > Please review this cleanup, where we remove JDK_GetVersionInfo0 and > > related code, since we can get build versions directly from within the > > VM itself: > > > > I'm including core-libs and awt in this review because the proposed > > fix touches their corresponding files. > > > > > > bug: https://bugs.openjdk.java.net/browse/JDK-8223261 > > webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev1 > > tests: passes Mach5 tier1,2,3,4,5,6 > > > > This is a good clean up. JDK_GetVersionInfo0 was needed long time ago > in particular in HS express support that is no longer applicable. > > One leftover comment should also be removed. > > src/hotspot/share/runtime/vm_version.hpp > // Gets the jvm_version_info.jvm_version defined in jvm.h > > otherwise looks good. > > Mandy
Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
On 11/13/19 10:50 AM, gerard ziemski wrote: Hi all, Please review this cleanup, where we remove JDK_GetVersionInfo0 and related code, since we can get build versions directly from within the VM itself: I'm including core-libs and awt in this review because the proposed fix touches their corresponding files. bug: https://bugs.openjdk.java.net/browse/JDK-8223261 webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev1 tests: passes Mach5 tier1,2,3,4,5,6 This is a good clean up. JDK_GetVersionInfo0 was needed long time ago in particular in HS express support that is no longer applicable. One leftover comment should also be removed. src/hotspot/share/runtime/vm_version.hpp // Gets the jvm_version_info.jvm_version defined in jvm.h otherwise looks good. Mandy
Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"
On 2019-11-13 19:50, gerard ziemski wrote: webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev1 Nice cleanup! Looks good to me. /Claes
