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

[David Holmes]

Hi Thomas,

On 25/02/2016 11:19 PM, Thomas Stüfe wrote:
> Hi all,
>
> please review and sponsor this tiny addition to 8149036. These are some
> tiny cleanups which could not be added to the original change anymore
> because it was already pushed.
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8150619
> webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8150619-addon-to-thread-logging/webrev.00/webrev/index.html
>
> Original mail thread for 8149036:
> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-February/018099.html
>
> The change unifies the logging messages for all platforms and all log
> sites as much as possible.
>
> Please note: "tid" always refers to whatever os::current_thread_id()
> returns. This may be a pthread_t (AIX, BSD), a kernel thread id or
> similar (Mac, Linux, Solaris), or a windows thread id. Beside printing
> the "tid", I print out additional thread ids where it may be
> interesting, especially at thread start and attach. On AIX, I print the
> kernel thread id, on Linux/Mac, the pthread id.

Ok.

> There are some instances where I have only one and not the other:
> - after a call to pthread_create(), I only have the pthread id of the
> newborn thread, not the kernel thread id, so this is all I can print.
> - in shared code, all I have or all I can print without making the code
> to complicated is the "tid"

Ok. It's always easier when logging the current thread.

> Finally, the change adds a jtreg test to logging, to test the new
> logging switches.

General: ensure copyright years are updated - somehow some got missed in 
the push of 8149036.

---

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

---

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.

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");

  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! :(

FYI the test passed on our JPRT system.

Thanks,
David

> Thanks & Kind Regards, Thomas