RFR: 8190346: improve unified JVM logging help message and warnings
Marcus Larsson
marcus.larsson at oracle.com
Tue Feb 27 10:48:08 UTC 2018
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