On Mon, Jun 18, 2018 at 8:17 AM, David Holmes <david.hol...@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. >> >
