RFR: 8292329: Enable CDS shared heap for zero builds [v2]

Ioi Lam iklam at openjdk.org
Tue Aug 23 16:55:36 UTC 2022


On Tue, 23 Aug 2022 16:27:40 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   @shipilev review comments
>
> make/Images.gmk line 132:
> 
>> 130: 
>> 131:   # Only G1 supports dumping the shared heap, so explicitly use G1
>> 132:   # it if the JVM supports it. (Note: the default GC with zero is SerialGC)
> 
> Suggestion:
> 
>   # Only G1 supports dumping the shared heap, so explicitly use G1 if the JVM supports it.

Fixed.

> make/Images.gmk line 133:
> 
>> 131:   # Only G1 supports dumping the shared heap, so explicitly use G1
>> 132:   # it if the JVM supports it. (Note: the default GC with zero is SerialGC)
>> 133:   $1_$2_CDS_DUMP_FLAGS := $(CDS_DUMP_FLAGS) $(if $(filter g1gc, $(JVM_FEATURES_$1)),-XX:+UseG1GC)
> 
> I think it should be e.g. (untested):
> 
> Suggestion:
> 
>   $1_$2_CDS_DUMP_FLAGS := $(CDS_DUMP_FLAGS)
>   ifeq ($(call check-jvm-feature, g1gc), true)
>      $1_$2_CDS_DUMP_FLAGS += -XX:+UseG1GC
>   endif

That doesn't work because `check-jvm-feature` requires `JVM_VARIANT` to be set, but `CreateCDSArchive` is not called in a context where  `JVM_VARIANT` is set. ( `JVM_VARIANT` is set only in a few specific places in Main.gmk, etc).

One option is to change the foreach loop a few lines below to:


  $(foreach JVM_VARIANT, $(JVM_VARIANTS), \
    $(eval $(call CreateCDSArchive,)) \
  )


But I've not seen `JVM_VARIANT` being used this way, so I am a little hesitant about doing it.

-------------

PR: https://git.openjdk.org/jdk/pull/9984



More information about the build-dev mailing list