RFR: 8190346: improve unified JVM logging help message and warnings

[Marcus Larsson]

Updated webrev:
http://cr.openjdk.java.net/~mlarsson/8190346/webrev.01

Incremental:
http://cr.openjdk.java.net/~mlarsson/8190346/webrev.00-01

Thanks,
Marcus

On 2018-02-27 11:48, Marcus Larsson wrote:
> Hi Lois,
>
> Thanks for reviewing. Replies in-line.
>
>
> On 2018-02-26 17:10, Lois Foltan wrote:
>> On 2/22/2018 10:34 AM, Marcus Larsson wrote:
>>
>>> Hi,
>>>
>>> Please review the following patch to improve the -Xlog help and 
>>> warning messages.
>>>
>>> The changes include:
>>>
>>>  * Attempt to find and give suggestions for log selections that don't
>>>    match any tag sets. For example, when passing "-Xlog:load" issue a
>>>    warning similar to the following: "No tag set matches selection
>>>    'load'. Did you mean any of the following? load* class+load
>>>    module+load".
>>>  * Change log level for almost all startup logging on 'logging' tag
>>>    from trace/debug to info level. These are log messages that are only
>>>    printed once during startup, so it makes little sense in obscuring
>>>    them under debug/trace. The all tag set listing is only moved from
>>>    trace to debug, though.
>>>  * Make LogConfiguration::print_command_line_help() take an
>>>    outputStream* instead of FILE*, to better align it with various
>>>    describe/print_on functions.
>>>  * Add descriptions of log file output related options to -Xlog:help
>>>    output.
>>>  * Print 'none' if there are no decorations configured for an output
>>>    (instead of just printing nothing).
>>>
>>> Issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8190346
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~mlarsson/8190346/webrev.00
>>>
>>> Tested with hs-tier 1-2, and locally with 
>>> test/hotspot/jtreg/{runtime,serviceability,gc}/logging.
>>>
>>> Thanks,
>>> Marcus
>>>
>> Hi Marcus,
>> This looks good.  Couple of review comments:
>>
>> hotspot/share/logging/logConfiguration.cpp:
>> - line #370-382 - curious to know why the need to walk the errbuf 
>> string looking for the next newline character?  Was there a bug 
>> present when just calling log_error or log_warning?
>
> There wasn't a bug before, just an assumption that the stream would 
> contain only a single line. With this change, the stream might contain 
> multiple lines, and I want to log each of those lines as separate 
> messages (so that each line is decorated).
>
>>   There doesn't seem to be code to protect one from walking off the 
>> end of errbuf?
>
> It should be safe. The stringStream will NUL-terminate whatever is in 
> the buffer, and strchr won't look past the terminating NUL. I'll add 
> an assert to clarify, and guard against overflows.
>
>>
>> hotspot/share/logging/logOutput.cpp
>> - line #90 - minor comment that the delimiter could just be of type 
>> 'char' instead of an array?  You would have to adjust the format 
>> string to use %c
>
> Indeed, fixed!
>
>>
>> hotspot/share/loggin/logSelection.cpp
>> - LogSelection::similariy and class SimilarityComparator - can you 
>> add a header comment for class SimilarityComparator describing the 
>> heuristic used to compute the set of potentially similar -Xlog 
>> options.  That's what it is doing correct?  There just seems to be a 
>> lot of hard coded numbers is this section of new code that would be 
>> could to understand.
>
> I'll add some comments.
>
> Thanks,
> Marcus