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

Ioi Lam iklam at openjdk.org
Wed Feb 15 03:47:43 UTC 2023


On Tue, 14 Feb 2023 17:10:16 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.
>> 
>> Since only the "warning" and "info" channels are used, they can each have their own methods where fail_continue() defaults to the warning channel. Some messages must remain in the "info" channel as their inclusion would make the output too verbose. Verified with tier1-4 tests
>
> Matias Saavedra Silva has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Replaced fail_continue with log_info and log_warning
>  - Merge branch 'master' into cdsWarning_8292269
>  - CRC error now warning
>  - Merge branch 'master' into cdsWarning_8292269
>  - 8292269: Log more CDS failure messages in the warning channel

Looks good to me, apart from the comment.

Are any of the tests affected? Some tests may be sensitive to whether the log is from warning or info.

src/hotspot/share/cds/filemap.cpp line 2556:

> 2554: // information (version, boot classpath, etc.).  If initialization
> 2555: // fails, shared spaces are disabled and the file is closed. [See
> 2556: // log_warning(cds).]

The `[See ...]` part of the comment should be removed.

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

Marked as reviewed by iklam (Reviewer).

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


More information about the hotspot-runtime-dev mailing list