Hi Volker, > On Jun 15, 2018, at 12:43 AM, Volker Simonis <volker.simo...@gmail.com> wrote: > > Hi Jiangli, > > thanks for looking at the change. > > 'CDS_only' is only required for static fields because the > VMStructEntry for them contains a reference to the actual static field > which isn't present if we disable CDS, because the corresponding > compilations units (i.e. filemap.cpp) won't be part of libjvm.so. For > non-static fields, the VMStructEntry structure only contains the > offset of the corresponding field with regards to an object of that > type which is harmless.
Thanks for the explanation. For consistency, would it be worth to add CDS_ONLY for the non-static fields in FileMapInfo also? Thanks, Jiangli > > Regards, > Volker > > > On Thu, Jun 14, 2018 at 6:42 PM, Jiangli Zhou <jiangli.z...@oracle.com> wrote: >> Hi Volker, >> >> The changes look good to me overall. I’ll refer to the JVMTI experts for >> jvmtiEnv.cpp change. I have a question for the change in vmStructs.cpp. Any >> reason why only _current_info needs CDS_ONLY? >> >> /********************************************/ >> \ >> /* FileMapInfo fields (CDS archive related) */ >> \ >> /********************************************/ >> \ >> >> \ >> nonstatic_field(FileMapInfo, _header, >> FileMapInfo::FileMapHeader*) \ >> - static_field(FileMapInfo, _current_info, >> FileMapInfo*) \ >> + CDS_ONLY(static_field(FileMapInfo, _current_info, >> FileMapInfo*)) \ >> nonstatic_field(FileMapInfo::FileMapHeader, _space[0], >> FileMapInfo::FileMapHeader::space_info)\ >> nonstatic_field(FileMapInfo::FileMapHeader::space_info, _addr._base, >> char*) \ >> nonstatic_field(FileMapInfo::FileMapHeader::space_info, _used, >> size_t) \ >> >> \ >> >> Thanks, >> Jiangli >> >> On Jun 14, 2018, at 7:26 AM, Volker Simonis <volker.simo...@gmail.com> >> 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. >> >>
