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

[Calvin Cheung]

On Mon, 14 Nov 2022 17:19:45 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> 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.

I've removed the "UseSharedSpaces:" log message and adjusted a few tests.

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

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