RFR: 8190346: improve unified JVM logging help message and warnings
Lois Foltan
lois.foltan at oracle.com
Tue Feb 27 15:58:22 UTC 2018
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
>
More information about the hotspot-runtime-dev
mailing list