RFR: 8152635: Create a UL class to represent a Log + Level combination

Bengt Rutisson bengt.rutisson at oracle.com
Tue Mar 29 14:45:16 UTC 2016


Hi StefanK,

On 2016-03-29 15:46, Stefan Karlsson wrote:
> Hi all,
>
> Please review this patch to introduce a Unified Logging class to 
> describe a combination of a tagset and a log level.
>
> http://cr.openjdk.java.net/~stefank/8152635/webrev.01
> https://bugs.openjdk.java.net/browse/JDK-8152635
>
> The patch is applied on top of the patch in:
> http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-March/022302.html
>
> With this class we could write code as:
> LogTarget(Debug, gc, heap) log;
> if (log.is_enabled()) {
>   log.print(...);
>   log.print(...);
>   ... = log.stream();
> }
>
> instead of the current model:
> Log(gc, heap) log;
> if (log.is_debug()) {
>   log.debug(...)
>   log.debug(...)
>   ... = log.debug_stream():
> }
>
> The LogTarget class/macro ensures that we only mention the log level 
> once, so that we don't accidentally log to the wrong level. The 
> previous version forces the coder to repeat 'debug' four times.
>
> One example where the wrong log level has been used:
> https://bugs.openjdk.java.net/browse/JDK-8152962
>
> I've converted one function in compactibleFreeListSpace.cpp to show 
> how to use the new class.

These changes look good to me.

>
> Test: new internal vm test, jprt

One minor thing about the tests.

I would think that this code:

  290   if (fgets(output, sizeof(output), fp) != NULL) {
  291     assert(strstr(output, Test_logtarget_string_literal) != NULL, 
"log line missing");
  292   }

Could be an assert rather than an if. We do expect the file read to work 
on all platforms, right?

Thanks,
Bengt

>
> Thanks,
> StefanK



More information about the hotspot-dev mailing list