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

Jiangli Zhou jiangli.zhou at oracle.com
Fri Jun 15 16:47:29 UTC 2018


Hi Volker,

> On Jun 15, 2018, at 12:43 AM, Volker Simonis <volker.simonis at 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.zhou at 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.simonis at 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.
>> 
>> 




More information about the build-dev mailing list