RFR(xs): 8150619: Improve thread based logging introduced with 8149036

Thomas Stüfe thomas.stuefe at gmail.com
Mon Feb 29 08:26:58 UTC 2016


Hi David,

New Version:
http://cr.openjdk.java.net/~stuefe/webrevs/8150619-addon-to-thread-logging/webrev.01/webrev/index.html

See comments inline.

On Mon, Feb 29, 2016 at 6:55 AM, David Holmes <david.holmes at oracle.com>
wrote:

> Hi Thomas,
>
>>
>>
> General: ensure copyright years are updated - somehow some got missed in
> the push of 8149036.
>
>
I fixed all copyright years in the file headers. It would be nice if jcheck
would check this automatically.


> ---
>
> os_solaris.cpp
>
> The indent is still not right here:
>
> 893 static char* describe_thr_create_attributes(char* buf, size_t buflen,
>  894                                  size_t stacksize, long flags) {
>
> size_t should line up with the 'c' in (char*
>
> Ditto for os_windows.cpp
>
>
Okay, I fixed that. Sorry, I did not understand the indentation rules
before.


> ---
>
> test/runtime/logging/ThreadLoggingTest.java
>
>  2  * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights
> reserved.
>
> This is new test so only a single copyright year should be present.
>
>
Fixed.


> You have this:
>
>   54         output.shouldContain("stack guard pages");
>   55         output.shouldContain("stack guard pages");
>
> but I think the intent was:
>
>        output.shouldContain("stack guard pages activated");
>        output.shouldContain("stack guard pages removed");
>
>
Changed to one line of "output.shouldContain("stack guard pages");". This
matches both cases.


>  68         pb = ProcessTools.createJavaProcessBuilder("-Xlog:thread*",
> "-version");
>
> Okay I'm confused - I thought the whole point of using os+thread here was
> that you had to enable os _and_ thread level logging ??? Else why bother
> using the two? I would not have expected the above to work! :(
>

No, the logging line is just tagged with both keywords. As far as I
understand multiple tags in UL there is no way to specify conditions like
"&&" or "||" - you just tag them - and specifying rules happens on the
command line with "os+thread" or "os,thread". So all of this: "os*",
"thread*", "os+thread" should produce an output, the last one will stop
producing output as soon as someone adds a third tag to the logging calls.

Note that I added the "thread" keyword because Marcus requested this:
http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-February/018154.html

But I simplified the test to test once for "os+thread" instead of each tag
separately. Note that with this change, the test will have to be modified
if someone adds a third tag.



>
> FYI the test passed on our JPRT system.
>
>
Great, thanks.

Thank you,

Thomas


> Thanks,
> David
>
>
> Thanks & Kind Regards, Thomas
>>
>


More information about the hotspot-runtime-dev mailing list