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

[Marcus Larsson]

Great, thanks Harold!

Marcus


On 2018-02-27 17:27, harold seigel wrote:
> This looks good to me, also.
>
> Harold
>
>
> On 2/27/2018 10:58 AM, Lois Foltan wrote:
>> Thanks Marcus, this looks good and I appreciate the addition of the 
>> comments ahead of class SimilarityComparator.
>> Lois
>>
>> On 2/27/2018 8:57 AM, Marcus Larsson wrote:
>>> 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
>>>
>>
>