RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default

[David Holmes]

Hi Volker,

v3 looks much cleaner - thanks.

But AFAICS the change to jvmtiEnv.cpp is also not needed. 
ClassLoaderExt::append_boot_classpath exists regardless of INCLUDE_CDS 
but operates differently (just calling 
ClassLoader::add_to_boot_append_entries).

Thanks,
David

On 19/06/2018 2:04 AM, Volker Simonis wrote:
> On Mon, Jun 18, 2018 at 8:17 AM, David Holmes <david.holmes at oracle.com> wrote:
>> Hi Volker,
>>
>> src/hotspot/share/runtime/globals.hpp
>>
>> This change should not be needed! We do minimal VM builds without CDS and we
>> don't have to touch the UseSharedSpaces defaults (unless recent change have
>> broken this - in which case that needs to be addressed in its own right!)
>>
> 
> Yes, you're right, CDS_ONLY/NOT_CDS isn't really required here,
> because UseSharedSpaces is reseted later on at the end of
> Arguments::parse(). I just thought it would be cleaner to disable it
> statically, if the VM doesn't support it. But anyway I don't really
> mind and I've reverted that change in globals.hpp.
> 
>> src/hotspot/share/classfile/javaClasses.cpp
>>
>> AFAICS you should be using INCLUDE_CDS in the ifdefs not
>> INCLUDE_CDS_JAVA_HEAP. But again I'm unclear (as was Thomas) why this should
>> be needed as we have not needed it before. As Thomas notes we have:
>>
>> ./hotspot/share/memory/metaspaceShared.hpp:  static bool
>> is_archive_object(oop p) NOT_CDS_JAVA_HEAP_RETURN_(false);
>> ./hotspot/share/classfile/stringTable.hpp:  static oop
>> create_archived_string(oop s, Thread* THREAD)
>> NOT_CDS_JAVA_HEAP_RETURN_(NULL);
>>
>> so these methods should be defined when CDS is not available.
>>
> 
> Thomas and you are right. Must have been a mis-configuration on AIX
> where I saw undefined symbols at link time. I've removed the ifdefs
> from javaClasses.cpp now.
> 
> Finally, I've also wrapped all the FileMapInfo fields in vmStructs.cpp
> into CDS_ONLY macros as suggested by Jiangli because the really only
> make sense for a CDS-enabled VM.
> 
> Here's the new webrev:
> 
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965.v3/
> 
> Please let me know if you think there's still something missing.
> 
> Regards,
> Volker
> 
> 
>> ??
>>
>> Thanks,
>> David
>> -----
>>
>>
>>
>>
>>
>> On 15/06/2018 12:26 AM, Volker Simonis wrote:
>>>
>>> Hi,
>>>
>>> can I please have a review for the following fix:
>>>
>>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/
>>> https://bugs.openjdk.java.net/browse/JDK-8204965
>>>
>>> CDS does currently not work on AIX because of the way how we
>>> reserve/commit memory on AIX. The problem is that we're using a
>>> combination of shmat/mmap depending on the page size and the size of
>>> the memory chunk to reserve. This makes it impossible to reliably
>>> reserve the memory for the CDS archive and later on map the various
>>> parts of the archive into these regions.
>>>
>>> In order to fix this we would have to completely rework the memory
>>> reserve/commit/uncommit logic on AIX which is currently out of our
>>> scope because of resource limitations.
>>>
>>> Unfortunately, I could not simply disable CDS in the configure step
>>> because some of the shared code apparently relies on parts of the CDS
>>> code which gets excluded from the build when CDS is disabled. So I
>>> also fixed the offending parts in hotspot and cleaned up the configure
>>> logic for CDS.
>>>
>>> Thank you and best regards,
>>> Volker
>>>
>>> PS: I did run the job through the submit forest
>>> (mach5-one-simonis-JDK-8204965-20180613-1946-26349) but the results
>>> weren't really useful because they mention build failures on linux-x64
>>> which I can't reproduce locally.
>>>
>>