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