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

[Ioi Lam]

Hi Max,

It looks really good now. Just a minor nit for classFileParser.cpp and 
systemDictionary.cpp, add the comment suggested by David:

1305     if (log_is_enabled(Info, classload)) {
1306       ik()->print_loading_log(LogLevel::Info, loader_data, NULL);
1307     }
          // No 'else' here as logging levels are not mutually exclusive
1308     if (log_is_enabled(Debug, classload)) {
1309       ik()->print_loading_log(LogLevel::Debug, loader_data, NULL);
1310     }

If there are no other changes, you don't need to post new webrev.

Thanks
- Ioi



On 1/28/16 12:15 PM, Coleen Phillimore wrote:
>
> Yes, this test looks really good.  Thanks Ioi for figuring out the 
> vagaries of jtreg.
>
> Coleen
>
> On 1/28/16 2:29 PM, Max Ockner 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
>>>
>>
>>
>>
>