RFS(S): 8222934: mark new VM option AllowRedefinitionToAddOrDeleteMethods as deprecated

Daniel D. Daugherty daniel.daugherty at oracle.com
Thu Apr 25 19:40:44 UTC 2019


On 4/24/19 6:18 PM, serguei.spitsyn at oracle.com wrote:
> Please, review fix for:
>   https://bugs.openjdk.java.net/browse/JDK-8222934
>
> Webrev:
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8222934-jvmti-depr-option.1/ 
>

src/hotspot/share/runtime/globals.hpp
     No comments.

test/hotspot/jtreg/runtime/CommandLine/VMDeprecatedOptions.java
     L42:         // deprecated class redefinition flags:
     L43:         {"AllowRedefinitionToAddDeleteMethods", "true"},
     L44:
     L45:         // deprecated non-alias flags:
         I think your new flag entry should have been added to the
         "deprecated non-alias flags" section. You don't need to
         call out that this is a "class redefinition" flag.

         The reason for the "// deprecated alias flags (see also 
aliased_jvm_flags):"
         section (below what you changed) is because there is more
         work to do for those flags.

test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/TestAddDeleteMethods.java
     L94:     public static String ADeleteStaticFoo =
         This case is deleting both "staticFoo" and "finalFoo".
         Is that what you really want? If so, then the test case
         is misnamed.

     L119     public static String BAddStaticBar =
         This case is added "staticBar" and "finalBar". Is that what
         you really want? If so, then the test case is misnamed.

     Still a really cool test here!

Thumbs up. Your call on whether to tweak the test.

Dan


>
>
> Summary:
>   David, in review for 
> https://bugs.openjdk.java.net/browse/JDK-8222934 suggested:
>    1. I would have suggested to add "(Deprecated)" to the description 
> of the new flag in globals.hpp
>    2. The new flag should have been added to the deprecated VM options 
> tests.
>    3. The new test should run in both a positive and negative mode so 
> that it also checks that the new flag works.
>
>   The webrev above implements this suggestion.
>
>
> Testing:
>   In progress: Submit mach5 run for the updated tests.
>
>
> Thanks,
> Serguei
>
>
>
>



More information about the serviceability-dev mailing list