jdk10 : UseSharedSpaces flag and INCLUDE_CDS macro

[Baesken, Matthias]

Hi Ioi ,
Yes I looked a bit at space reduction  but the effect is very small .

I was noticing the "mixture"  of   UseSharedSpaces code parts guarded (and not guarded) with  INCLUDE_CDS
When looking at a build configuration with cds disabled at build time.

But I'm totally fine with your  answer about not using INCLUDE_CDS everywhere.

Thanks, Matthias



>Hi Matthias,

>

>We don't use INCLUDE_CDS everywhere so we can avoid cluttering the code.

>In many cases, functions called inside if (UseSharedSpaces) are declared

>to have empty bodies. E.g.,

>

>void VM_RedefineClasses::doit() {

>   Thread *thread = Thread::current();

>

>   if (UseSharedSpaces) {

>     // Sharing is enabled so we remap the shared readonly space to

>     // shared readwrite, private just in case we need to redefine

>     // a shared class. We do the remap during the doit() phase of

>     // the safepoint to be safer.

>     if (!MetaspaceShared::remap_shared_readonly_as_readwrite()) {

>       log_info(redefine, class, load)("failed to remap shared readonly

>space to readwrite, private");

>       _res = JVMTI_ERROR_INTERNAL;

>       return;

>     }

>   }

>

>class MetaspaceShared {

>   static bool remap_shared_readonly_as_readwrite() NOT_CDS_RETURN_(true);

>

>

>So hopefully the C++ compile will just elide all the code inside the if

>(!...) branch (also also the check for UseSharedSpaces). There may be a

>few places where we should have added #if INCLUDE_CDS. For example,

>InstanceKlass::restore_unshareable_info.

>

>We are regularly building the minimal VM which doesn't have INCLUDE_CDS.

>So it looks like the VM will be built correctly when INCLUDE_CDS is not

>specified, but it probably has a bit of dead code.

>

>Are you mainly trying to reduce the size of libjvm when CDS is not enabled?

>

>Thanks

>- Ioi

>

>

>On 7/5/17 6:36 AM, Baesken, Matthias wrote:

>> Hello, when looking into CDS related build options,  I noticed  that most  code-parts  that are executed only when UseSharedSpaces is set,

>>    are guarded by  the compile-time macro    INCLUDE_CDS   to support  switching off   compilation of this coding

>>    when  CDS is disabled at compile time  :

>>

>>

>> See  hotspot/make/lib/JvmFeatures.gmk  :

>>

>> ifneq ($(call check-jvm-feature, cds), true)

>>    JVM_CFLAGS_FEATURES += -DINCLUDE_CDS=0

>>

>>

>>

>> However some places miss the compile-time guarding ( #if INCLUDE_CDS  ....) for example in

>>

>>

>> share/vm/prims/jvmtiRedefineClasses.cpp

>> share/vm/memory/universe.cpp

>>

>> (also some other places)

>>

>>

>> Should I prepare a change and add the compile-time guard at the places where missing as well ?

>>

>> Best regards, Matthias