RFR: 8079408: Reimplement TraceClassLoading, TraceClassUnloading, and TraceClassLoaderData with Unified Logging.

[Jiangli Zhou]

Hi Max,

The changes look very good. I have just few minor comments and questions below.

- klass.cpp
I’m new to the logging code, so I might be wrong. It seems the ‘if (log_is_enabled(Trace, classunload))’ checks at line 390 & 405 are not needed since log_trace() also checks if the trace flag is enabled.

- filemap.cpp
‘TraceClassLoading && Verbose’ is removed at line 211. Should a ‘log_info(classload)’ call be added? Or maybe the skipping of classloading trace for shared path is intentional?

The same question for other changes in filemap.cpp.

- instanceKlass.cpp
Please break line 2908 into two lines so it’s not too long. There are a few other long lines in print_loading_log() should be broken into two.

- arguments.cpp
LogLevel:Info is added for TraceClassLoading and TraceClassUnloading in aliased_logging_flags[]. Should there be one for LogLevel::Debug for the classloading tracing? Maybe there is no old styled option for debug level class loading trace, so it's not defined in aliased_logging_flags(?).

Thanks,
Jiangli


> On Jan 28, 2016, at 11:29 AM, Max Ockner <max.ockner at oracle.com> wrote:
> 
> Forwarded private discussion with Ioi about changing the test to use ClassUnloadCommon.
> 
> Hopefully nearing the end:
> 
> Webrev: http://cr.openjdk.java.net/~mockner/classload.08/
> 
> -------- Original Message --------
> Subject: 	Re: RFR: 8079408: Reimplement TraceClassLoading, TraceClassUnloading, and TraceClassLoaderData with Unified Logging.
> Date: 	Thu, 28 Jan 2016 10:57:15 -0800
> From: 	Ioi Lam <ioi.lam at oracle.com>
> To: 	Max Ockner <max.ockner at oracle.com>
> 
> 
> 
> Hi Max,
> 
> Thanks for doing this. Also, I sent the e-mail to you personally, so you may want to post the new version in the open list for record purpose.
> 
> Thanks
> - Ioi
> 
> On 1/28/16 8:11 AM, Max Ockner wrote:
>> Ioi,
>> I have replaced my test with this test that you have given me (thank you!)
>> I also have removed the commented out section which used to test for "making nmethod ...", and I have added the comment that you suggested at the top of ClassUnloadCommon.java
>> Thanks,
>> Max
>> On 1/27/2016 7:13 PM, Ioi Lam wrote:
>>> 
>>> 
>>> On 1/27/16 12:51 PM, Max Ockner wrote:
>>>> Though Ioi suggested I change my new test, I have not done that. I was recommended to copy from runtime/ClassUnload/UnloadTest.java instead of rolling my own test for class unloading. I mentioned that it was tricky to make the new test work, but it was tricky because I was trying to copy from UnloadTest.java. This test refers to a class "test.Empty" from a "classes" library, but the new test has a processBuilder which I think does not play nicely with the class path for  "test.Empty". In the end it was much easier to hardcode the entire test into one place than to follow UnloadTest.java and refer to extra libraries.
>>>> 
>>> 
>>> Hi Max,
>>> 
>>> I've written a version of ClassLoadUnloadTest.java using ClassUnloadCommon. The main trick is here:
>>> 
>>>        Collections.addAll(argsList, "-Dtest.classes=" + System.getProperty("test.classes","."));
>>> 
>>> That's because of this line in ClassUnloadCommon:
>>> 
>>>            return new URLClassLoader(new URL[] {
>>> Paths.get(System.getProperty("test.classes",".") + File.separatorChar + "classes").toUri().toURL(),
>>>            }, null);
>>> 
>>> When Jtreg launches a sub-process, it strips off all the Java system properties, so you need to add the "-D" in the command-line.
>>> 
>>> I also disabled the test related to "making nmethod ... unloadable". As Rachel found out in JDK-8146137, the compiler is unpredictable. Even if you invoke a method 10001 times, there's no guaranteed that it will always be compiled. Since this is not an 'easy-to-test' feature, we should just skip it.
>>> 
>>> I would also suggest adding this to the header of ClassUnloadCommon.java
>>> 
>>> /*
>>> * To use ClassUnloadCommon from a sub-process, see hotspot/test/runtime/logging/ClassLoadUnloadTest.java
>>> * for an example.
>>> */
>>> 
>>> What do you think?
>>> - Ioi
>> 
> 
> 
>