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