RFR: 8232365: Implementation for JEP 363: Remove the Concurrent Mark Sweep (CMS) Garbage Collector

[David Holmes]

On 22/10/2019 3:56 am, Leo Korinth wrote:
> On 18/10/2019 11:20, David Holmes wrote:
>> Hi Leo,
>>
>> Picking up on one item ...
>>
>>  > Not fully addressed in code, but an issue that has to be dealt 
>> with, how
>>  > do I obsolete -Xconcgc and -Xnoconcgc? I believe the option should be
>>  > obsoleted, though I do not know if we have any precedence 
>> obsoleting -X
>>  > options.
>>
>> These flags (which are passed through from the launcher) are already 
>> marked as deprecated. So to "obsolete" them you just need to update 
>> the warning that gets issued (and otherwise delete them from the 
>> code). So you need to change this code in arguments.cpp to produce the 
>> appropriate obsoletion warning similar to what we issue for obsoleted 
>> -XX flags:
>>
>>     // -Xconcgc
>>      } else if (match_option(option, "-Xconcgc")) {
>>        if (FLAG_SET_CMDLINE(UseConcMarkSweepGC, true) != 
>> JVMFlag::SUCCESS) {
>>          return JNI_EINVAL;
>>        }
>>        handle_extra_cms_flags("-Xconcgc uses UseConcMarkSweepGC");
>>      // -Xnoconcgc
>>      } else if (match_option(option, "-Xnoconcgc")) {
>>        if (FLAG_SET_CMDLINE(UseConcMarkSweepGC, false) != 
>> JVMFlag::SUCCESS) {
>>          return JNI_EINVAL;
>>        }
>>        handle_extra_cms_flags("-Xnoconcgc uses UseConcMarkSweepGC");
>>
>> Let me know if you need more details.
>>
>> Then file a follow up issue to remove these flags in the future. I 
>> don't know if you have a timeline for that yet.
>>
>> Cheers,
>> David
>> -----
> 
> 
> Hi David, I filed https://bugs.openjdk.java.net/browse/JDK-8232739
> 
> Do you suggest something like this?

Yes just like that.

Thanks,
David
-----

> 
> diff --git a/src/hotspot/share/runtime/arguments.cpp 
> b/src/hotspot/share/runtime/arguments.cpp
> index 620168b4e4b..c0a9acea962 100644
> --- a/src/hotspot/share/runtime/arguments.cpp
> +++ b/src/hotspot/share/runtime/arguments.cpp
> @@ -2615,10 +2615,10 @@ jint Arguments::parse_each_vm_init_arg(const 
> JavaVMInitArgs* args, bool* patch_m
>         }
>       // -Xconcgc
>       } else if (match_option(option, "-Xconcgc")) {
> -      handle_extra_cms_flags("-Xconcgc uses UseConcMarkSweepGC");
> +      warning("-Xconcgc uses UseConcMarkSweepGC; support was removed 
> for both options in 14.0");
>       // -Xnoconcgc
>       } else if (match_option(option, "-Xnoconcgc")) {
> -      handle_extra_cms_flags("-Xnoconcgc uses UseConcMarkSweepGC");
> +      warning("-Xnoconcgc uses UseConcMarkSweepGC; support was removed 
> for both options in 14.0");
>       // -Xbatch
>       } else if (match_option(option, "-Xbatch")) {
>         if (FLAG_SET_CMDLINE(BackgroundCompilation, false) != 
> JVMFlag::SUCCESS) {
> @@ -3859,15 +3859,6 @@ bool Arguments::handle_deprecated_print_gc_flags() {
>     return true;
>   }
> 
> -void Arguments::handle_extra_cms_flags(const char* msg) {
> -  SpecialFlag flag;
> -  const char *flag_name = "UseConcMarkSweepGC";
> -  if (lookup_special_flag(flag_name, flag)) {
> -    handle_aliases_and_deprecation(flag_name, /* print warning */ true);
> -    warning("%s", msg);
> -  }
> -}
> -
>   // Parse entry point called from JNI_CreateJavaVM
> 
>   jint Arguments::parse(const JavaVMInitArgs* initial_cmd_args) {
> diff --git a/src/hotspot/share/runtime/arguments.hpp 
> b/src/hotspot/share/runtime/arguments.hpp
> index d9aa77af315..2759836bb88 100644
> --- a/src/hotspot/share/runtime/arguments.hpp
> +++ b/src/hotspot/share/runtime/arguments.hpp
> @@ -425,8 +425,6 @@ class Arguments : AllStatic {
> 
>     static bool handle_deprecated_print_gc_flags();
> 
> -  static void handle_extra_cms_flags(const char* msg);
> -
>     static jint parse_vm_init_args(const JavaVMInitArgs 
> *java_tool_options_args,
>                                    const JavaVMInitArgs *java_options_args,
>                                    const JavaVMInitArgs *cmd_line_args);
> 
> 
> Thanks,
> Leo
> 
>>
>> On 18/10/2019 6:20 pm, Leo Korinth wrote:
>>> Hi,
>>>
>>> Here is a patch that removes the CMS GC.
>>>
>>> I have neither tested arm nor ppc; I hope my changes to those .ad 
>>> files are correct, if someone can test those architectures, that 
>>> would be great.
>>>
>>> Please take an extra look at 
>>> CollectedHeap::check_for_non_bad_heap_word_value, it was buggy before 
>>> (but never called), It is now called (and hopefully correct).
>>>
>>> I have tried to remove most parts of CMS. I have not made it a goal 
>>> to remove all traces of CMS. I guess there are much more to cleanup, 
>>> and suggestions of more to remove are welcomed. I think more 
>>> complicated cleanups should be dealt with in separate enhancements.
>>>
>>> Not fully addressed in code, but an issue that has to be dealt with, 
>>> how do I obsolete -Xconcgc and -Xnoconcgc? I believe the option 
>>> should be obsoleted, though I do not know if we have any precedence 
>>> obsoleting -X options.
>>>
>>> My patch prints:
>>>
>>> $ java -Xconcgc -jar Notepad.jar
>>> Java HotSpot(TM) 64-Bit Server VM warning: -Xconcgc uses 
>>> UseConcMarkSweepGC
>>>
>>> I guess that is not enough for being obsolete, compare with:
>>>
>>> $ java -XX:UseConcMarkSweepGC -jar Notepad.jar
>>> Java HotSpot(TM) 64-Bit Server VM warning: Ignoring option 
>>> UseConcMarkSweepGC; support was removed in 14.0
>>>
>>> Bug:
>>>    https://bugs.openjdk.java.net/browse/JDK-8232365
>>>
>>> Webrev:
>>>    http://cr.openjdk.java.net/~lkorinth/8232365/00
>>>
>>> Testing:
>>>    tier 1-5.
>>>
>>> Thanks,
>>> Leo