RFR: 8168722: Unified Logging configuration output needs simplifying

[Lois Foltan]

On 2/20/2018 3:51 AM, Marcus Larsson wrote:

> Hi,
>
> Please review the following patch to make UL configuration 
> descriptions more readable/usable. Instead of explicitly listing all 
> tag sets with their corresponding levels (a=info,b=info, ..., x=debug, 
> etc), a significantly shorter aggregate is now generated and printed 
> instead (all=info,x=debug).
>
> Summary:
> Initially, the configured level for the majority of the log tag sets 
> is found and used for the "all=<level>" part of the config string. 
> Then, using a greedy iterative algorithm, selections are generated and 
> added to the configuration string to cover the descriptions for tag 
> sets with levels deviating from the current description. In each step, 
> the selection which covers the most deviating tag sets is chosen. This 
> is repeated until there are no deviating tag sets remaining. For the 
> curious, the problem can be thought of as a variation of the set cover 
> problem [0].
>
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8168722
>
> Webrev:
> http://cr.openjdk.java.net/~mlarsson/8168722/webrev.00
>
> Tested with hs-tier 1-2, and locally with 
> jtreg/{serviceability,runtime,gc}/logging.
>
> Thanks,
> Marcus
>
> [0]: https://en.wikipedia.org/wiki/Set_cover_problem

Hi Marcus,
Overall this looks good.  Just a couple of minor comments:

- hotspot/share/logging/logOutput.cpp
line #43 - please consider making delimiter a char
line #102  - can subset_size and depth be const parameters?
line #139 - consider not post incrementing subset_size (make a const 
parameter), since subset_size is passed by value parameter, the 
incrementation won't make a difference after a return from the current 
call to generate_all_subsets_of.  I would rather see line #140 
explicitly use "subset_size + 1".  That then it makes it obvious that on 
the 2nd call to generate_all_subsets_of, subset_size has been incremented.
line #155 - is the variable "before" ever used?  I think it can be removed.
line #201 & 202, why the bump by 2?
line #214 - can on_level be const?  It doesn't seem to be assigned into 
within update_config_string().
line #239 - why is selections_cap set to 64?
line#259 - best_selection is defined to be a constant array but at line 
#286 you basically reassign the whole array, that looks fishy.

Thanks,
Lois