RFR: 8292269: Log more CDS failure messages in the warning channel [v6]

David Holmes dholmes at openjdk.org
Mon Feb 20 04:28:27 UTC 2023


On Sat, 18 Feb 2023 01:29:03 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> When -Xshare:auto is used, FileMapInfo::fail_continue() prints the diagnostic message to the Info channel instead of Warning. This was necessary in the past because CDS would failed to load for many reasons (e.g., failure to mmap due to ASLR) that are not the fault of the user. If we printed the message in the Warning channel, the user would be overwhelmed and would ultimately ignore the warnings, rendering them useless.
>> 
>> However, currently CDS is much more reliable. For some error conditions that require user attention, we should consider changing the failure logs to use the Warning channel.
>> 
>> Logging will now shift toward Unified Logging, so `fail_continue()` will be replaced with `log_info(cds)` and `log_warning(cds)`. Relevant tests are updated to accommodate this change. Verified with tier1-4 tests
>
> Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fixed a merge issue

I'm unapproving as this latest set of changes is more than just minor tweaks.

Not at all clear why these latest changes were necessary.  The code seems messier now.

Why do the tests now explicitly need -Xshare:auto?

src/hotspot/share/cds/metaspaceShared.cpp line 914:

> 912:   if (static_mapinfo != nullptr) {
> 913:     log_info(cds)("Core region alignment: " SIZE_FORMAT, static_mapinfo->core_region_alignment());
> 914:     dynamic_mapinfo = open_dynamic_archive(has_failed);

`has_failed` seems unused in relation to the `open_dynamic_archive` call.

src/hotspot/share/cds/metaspaceShared.cpp line 983:

> 981: }
> 982: 
> 983: FileMapInfo* MetaspaceShared::open_dynamic_archive(bool &has_failed) {

Shouldn't the null return suffice for failure checking, possibly in combination with a flag check?

src/hotspot/share/runtime/arguments.cpp line 2924:

> 2922:   if (UseSharedSpaces && RequireSharedSpaces) {
> 2923:     LogConfiguration::configure_stdout(LogLevel::Info, true, LOG_TAGS(cds));
> 2924:   }

why?

-------------

Changes requested by dholmes (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12419


More information about the hotspot-runtime-dev mailing list