RFR: JDK-8302989: Add missing INCLUDE_CDS checks

David Holmes dholmes at openjdk.org
Mon Feb 27 06:24:14 UTC 2023


On Tue, 21 Feb 2023 14:42:38 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

> The cds only coding in hotspot is usually guarded with the INCLUDE_CDS macro so that it can be removed at compile time in case the correct configure flags are set.
> However at some places INCLUDE_CDS is missing and should be added.
> 
> One question - should (additionally to the UseSharedSpaces code section)  the DumpSharedSpaces code sections be guarded as well with INCLUDE_CDS macros ?

So you avoid doing e.g.

if (RewriteBytecodes CDS_ONLY(&& !UseSharedSpaces && !DumpSharedSpaces)) {

and

CDS_ONLY(assert(!UseSharedSpaces, "UsedSharedSpaces not supported with exploded module builds");) ***

but what about the code in `Metaspace::global_initialize()`? Looks the the `INCLUDE_CDS` section there needs expanding. And `Universe::genesis` is a bit inconsistent about excluding or not. And `InstanceRefKlass::update_nonstatic_oop_maps` looks like a candidate too. And realistically we shouldn't even be compiling systemDictionaryShared.cpp in a non-CDS build, but that would take a lot of additional conditionalization.

I'm just not seeing that converting a handful of sites is really beneficial. The flags themselves are not normal flags defined for CDS_ONLY precisely because we wanted them available without having to conditionalize every single usage. I'm not looking to "block" this but I can't really endorse it either.

*** there is probably potential for adding a `cds_assert` macro to handle a bunch of CDS-only assertions.

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

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


More information about the hotspot-dev mailing list