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

Volker Simonis volker.simonis at gmail.com
Fri Jun 15 07:43:14 UTC 2018


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.

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