RFR: JDK-8302989: Add missing INCLUDE_CDS checks

[Ioi Lam]

On Tue, 28 Feb 2023 13:02:22 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

> > It may be more profitable (and less work) if we change these variables to `const` when CDS is disabled:
> > ```
> > extern bool DumpSharedSpaces;
> > extern bool DynamicDumpSharedSpaces;
> > extern bool RequireSharedSpaces;
> > extern "C" {
> > // Make sure UseSharedSpaces is accessible to the serviceability agent.
> > extern JNIEXPORT jboolean UseSharedSpaces;
> > }
> > ```
> >   
> > But some changes may be needed in SA.
> 
> Hi Ioi, do you think this should be done for all 4 bools ? Btw. when adding a const to UseSharedSpaces I run into something like this (when compiling with gcc, seems there is some issue when const is used together with JNIEXPORT , any ideas why ? ` globalDefinitions.cpp:50:26: error: 'visibility' attribute ignored [-Werror=attributes]`

I think it should be done for all four bools.

UseSharedSpaces is accessed by SA in src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/VM.java. I would suggest adding this:


  public boolean isSharingEnabled() {
    if (sharingEnabled == null) {
<<<<<
      Boolean sharingConfigured = getSharingConfigured(); // with similar "lookup" logic as below ...
      if (!sharingConfigured.booleanValue()) {
        sharingEnabled = sharingConfigured; // INCLUDE_CDS is disabled
        return false;
      }
>>>>>
       Address address = VM.getVM().getDebugger().lookup(null, "UseSharedSpaces");
        if (address == null && getOS().equals("win32")) {
            // On Win32 symbols are prefixed with the dll name. So look for
            // UseSharedSpaces as a symbol in jvm.dll.
            address = VM.getVM().getDebugger().lookup(null, "jvm!UseSharedSpaces");
        }
        sharingEnabled = address.getJBooleanAt(0);
    }
    return sharingEnabled.booleanValue();
  }


Then we can change globalDefinitions.cpp to something like:


#if INCLUDE_CDS
// Old CDS options
bool DumpSharedSpaces;
bool DynamicDumpSharedSpaces;
bool RequireSharedSpaces;
extern "C" {
JNIEXPORT jboolean UseSharedSpaces = true; // for SA
JNIEXPORT jboolean SharingConfigured= true; // for SA
}
#else
extern "C" {
JNIEXPORT jboolean SharingConfigured= false; // for SA
}
#endif


You may need to test with a build that has SA enabled but CDS disabled. I am not sure if this is a supported combination.

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

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