RFR: 8150084: Convert TraceMonitorMismatch to Unified Logging

Max Ockner max.ockner at oracle.com
Tue Mar 29 18:13:26 UTC 2016


This does not need @modules. According to Harold, if the test passes 
without @modules, then it doesn't need @modules.
Max

On 3/29/2016 1:01 PM, Coleen Phillimore wrote:
>
> This looks good - but one last question (sorry for incremental comments).
>
> http://cr.openjdk.java.net/~mockner/8150084.04/test/runtime/logging/MonitorMismatchTest.java.html 
>
>
> Does this need any @modules things?
>
> Thanks,
> Coleen
>
> On 3/29/16 12:56 PM, Max Ockner wrote:
>> Fixed: http://cr.openjdk.java.net/~mockner/8150084.04/
>>
>> Thanks,
>> Max
>> On 3/28/2016 5:47 PM, Coleen Phillimore wrote:
>>>
>>> MonitorMismatchHelper.java needs a copyright notice.
>>> Coleen
>>>
>>> On 3/28/16 4:15 PM, Max Ockner wrote:
>>>> Thanks for round 1 of reviews.
>>>>
>>>> webrev: http://cr.openjdk.java.net/~mockner/8150084.03/
>>>> - Renamed A.jasm to MonitorMismatchHelper.jasm and included it in 
>>>> webrev.
>>>> - Fixed improper spacing in test.
>>>>
>>>> Thanks,
>>>> Max
>>>> On 3/28/2016 3:52 PM, Coleen Phillimore wrote:
>>>>>
>>>>> It looks like A.jasm is missing from your webrev and can you name 
>>>>> it something more useful like MonitorMismatchHelper.java ?
>>>>> Otherwise, code looks good.
>>>>> thanks,
>>>>> Coleen
>>>>>
>>>>> On 3/28/16 3:10 PM, Max Ockner wrote:
>>>>>> Good catch. We don't intend for this to be DEBUG_ONLY. The 
>>>>>> "#ifndef PRODUCT" has been removed.
>>>>>> Webrev: http://cr.openjdk.java.net/~mockner/8150084.02/
>>>>>>
>>>>>> If we put the log_is_enabled conditional inside the function then 
>>>>>> we will always branch to the function. The conversion of 
>>>>>> TraceClassResolution required a similar decision, and we chose to 
>>>>>> leave the log_is_enabled condition outside of the 
>>>>>> trace_class_resolution() function.
>>>>>>
>>>>>> Thanks,
>>>>>> Max
>>>>>>
>>>>>> On 3/25/2016 2:18 PM, Rachel Protacio wrote:
>>>>>>> Hey, Max,
>>>>>>>
>>>>>>> The code looks fine as-is, but perhaps could be optimized? Since 
>>>>>>> all the uses of monitormismatch call report_monitor_mismatch(), 
>>>>>>> which has its code in non-product only, you could remove the 
>>>>>>> conditionals around it and have the function declared with 
>>>>>>> "PRODUCT_RETURN." Then, inside the function you could put the 
>>>>>>> "if log_is_enabled". So there will be less compiled code in 
>>>>>>> product mode. (This might also mean you could make the 
>>>>>>> monitormismatch tag "DEBUG_ONLY", but I'm not 100% sure about 
>>>>>>> how that works.)
>>>>>>>
>>>>>>> Rachel
>>>>>>>
>>>>>>> On 3/25/2016 1:28 PM, Max Ockner wrote:
>>>>>>>> Hello,
>>>>>>>> Please review another Unified Logging conversion.
>>>>>>>>
>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8150084
>>>>>>>> Webrev: http://cr.openjdk.java.net/~mockner/8150084.01/
>>>>>>>>
>>>>>>>> TraceMonitorMismatch has been converted to unified logging with 
>>>>>>>> monitormismatch tag and info level. There is very little 
>>>>>>>> output. To trigger the output, I ran a program which does a 
>>>>>>>> monitorenter without a monitorexit.
>>>>>>>>
>>>>>>>> 'java -XX:+TieredCompilation -Xcomp -Xlog:monitormismatch 
>>>>>>>> <program>'
>>>>>>>>
>>>>>>>> Tested with jtreg runtime tests and added 
>>>>>>>> MonitorMismatchTest.java to test for the output. The test body 
>>>>>>>> doesn't run on embedded since it uses TieredCompilation.
>>>>>>>>
>>>>>>>> If you search for TraceMonitorMismatch in the new source, you 
>>>>>>>> will find a bunch of instances in 
>>>>>>>> jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/GenerateOopMap.java. 
>>>>>>>> This file is a java implementation of GenerateOopMap.cpp and 
>>>>>>>> the two files aren't tied together in any way.  This java level 
>>>>>>>> TraceMonitorMismatch flag is set to true in the code and 
>>>>>>>> doesn't get its value from the jvm level TraceMonitorMismatch 
>>>>>>>> flag. I have left these alone as another unfortunate naming 
>>>>>>>> coincidence.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Max
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>



More information about the hotspot-runtime-dev mailing list