RFR (M): 8061616: HotspotDiagnosticMXBean.getVMOption() throws IllegalArgumentException for flags of type double

[Volker Simonis]

On Fri, Jul 24, 2020 at 5:16 PM Hohensee, Paul <hohensee at amazon.com> wrote:
>
> GetDoubleVMOption.java is indeed a new file introduced by this patch. If you look closely at the webrev entry for GetDoubleVMOption.java, you'll see
>
> (was test/com/sun/management/HotSpotDiagnosticMXBean/GetVMOption.java)
>

I saw that but I hadn't looked at the raw version. From the unified
diff it's hard to see that the outcome will be exactly that of the new
file.

Have you intentionally created GetDoubleVMOption.java as a copy of
GetVMOption.java or why else did it show up in this way in the webrev?
I find it hard to review such copy/patch diffs of otherwise unrelated
files.

> The patch first copied GetVMOption.java to create GetDoubleVMOption.java and then modified it to get the final version. What you see in the webrev diff are the differences between the final version of GetDoubleVMOption.java and GetVMOption.java. If you compare the 'Raw' version of GetDoubleVMOption.java with the one in
>
> http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/ef6ec39fd6bd
>

Agreed. If I import and apply the whole patch it also looks fine.

Please consider this downported as reviewed from my side.

Best regards,
Volker

> you'll see they're the same except for switching to InitialRAMPercentage.
>
> Thanks,
> Paul
>
> On 7/24/20, 12:12 AM, "Volker Simonis" <volker.simonis at gmail.com> wrote:
>
>     Hi Paul,
>
>     thanks for doing this downport. In general it looks good but I think
>     there's something wrong with the webrev.
>
>     For the jdk part it seems that GetVMOption.java test was merged into
>     GetDoubleVMOption.java which from my understanding should be a new
>     test introduced with this change.
>
>     The Hotspot part looks good.
>
>     Thank you and best regards,
>     Volker
>
>     On Fri, Jul 24, 2020 at 1:28 AM Hohensee, Paul <hohensee at amazon.com> wrote:
>     >
>     > Ping. :)
>     >
>     > Thanks,
>     > Paul
>     >
>     > On 7/10/20, 4:52 PM, "jdk8u-dev on behalf of Hohensee, Paul" <jdk8u-dev-retn at openjdk.java.net on behalf of hohensee at amazon.com> wrote:
>     >
>     >     Please review a proposed backport from jdk9 to jdk8u that fixes a serviceability issue.
>     >
>     >     Bug: https://bugs.openjdk.java.net/browse/JDK-8061616
>     >     Original jdk patch: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/ef6ec39fd6bd
>     >     Original hotspot patch: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/1bac07f399ac
>     >
>     >     jdk webrev: http://cr.openjdk.java.net/~phh/8061616/webrev.8u.jdk.01/
>     >     hotspot webrev: http://cr.openjdk.java.net/~phh/8061616/webrev.8u.hotspot.01/
>     >
>     >     Other than a few context and copyright date differences, the only change is testing InitialRAMPercentage rather than CompileThresholdScaling. The latter doesn’t exist in jdk8.
>     >
>     >     Passed:
>     >
>     >
>     >     jdk/ test/com/sun/management/HotSpotDiagnosticMXBean
>     >     hotspot/test/testlibrary_tests/whitebox/vm_flags/DoubleTest.java
>     >
>     >     Thanks,
>     >     Paul
>     >
>     >
>     >
>