RFR: 8291991: Adjust the "shared class paths mismatch" message if class path logging is enabled [v2]

[Calvin Cheung]

On Thu, 10 Nov 2022 22:14:30 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   simplify FileMapInfo::validate_shared_path_table
>
> src/hotspot/share/cds/filemap.cpp line 1128:
> 
>> 1126:       }
>> 1127:       if (!log_is_enabled(Info, cds) && !log_is_enabled(Info, class, path)) {
>> 1128:         log_warning(cds)("%s %s", mismatch_msg, hint_msg);
> 
> Usually, the message is printed inside `fail_continue()`, which goes to `log_info()`. However, in this particular case, we feel that the message is important enough that it should be printed in `log_warning()`. The current implementation is kind of awkward and makes this PR complicated. 
> 
> I think we can take this chance to clean things up. We can add an argument to `fail_continue()` to select the logging level. Something like this:
> 
> 
> void FileMapInfo::fail_continue(const char *msg, ...) {
>   va_list ap;
>   va_start(ap, msg);
>   fail_continue_impl(LogLevel::Info, msg, ap);
> }
> 
> void FileMapInfo::fail_continue(LogLevelType level, const char *msg, ...) {
>   va_list ap;
>   va_start(ap, msg);
>   fail_continue_impl(level, msg, ap);
> }
> 
> void FileMapInfo::fail_continue_impl(LogLevelType level, const char *msg, va_list ap) {
> ...
> 
> -     if (log_is_enabled(Info, cds)) {
> -       LogStream ls(Log(cds)::info());
> -       ls.print("UseSharedSpaces: "); 
> -       ls.vprint_cr(msg, ap);
> -     }
> +     LogMessage(cds) lm;
> +     lm.vwrite(level, msg, ap);
> ...
> }
> 
> 
> And thus this function can be simplified as:
> 
> 
> const char* mismatch_msg = "shared class paths mismatch";
> const char* hint_msg = log_is_enabled(Info, class, path) ?
>     "" : " (hint: enable -Xlog:class+path=info to diagnose the failure)";
> fail_continue(LogLevel::warning, "%s%s", mismatch_msg, hint_msg);
> 
> 
> The "UseSharedSpaces:" is also redundant since the log already contains `[cds]`. It's hard to pass that to `LogMessage` so we may as well just remove it.

Thanks for the review.

In `FileMapInfo::fail_continue_impl`, we'll need to make a copy of the `va_list`. Also, I noticed that the `lm.vwrite` needs to be called before `ls.vprint_cr`. Otherwise, vm will crash inside `lm.vwrite`. I added a comment in the new code.

Regarding the "UseSharedSpaces", I'd like to leave it as is for now to keep the same behavior as before.

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

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