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

[Marcus Larsson]

Hi Robbin,

Thanks for reviewing!


On 2018-02-27 15:39, Robbin Ehn wrote:
> Hi Marcus,
>
> If possible please add a test for the similarity of tags.

Added a sanity test that I hope isn't too flaky.

New Webrev:
http://cr.openjdk.java.net/~mlarsson/8190346/webrev.02

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

Thanks,
Marcus

>
> Looks good!
>
> /Robbin
>
> On 02/27/2018 02:57 PM, 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
>>