RFR: 8150084: Convert TraceMonitorMismatch to Unified Logging

Max Ockner max.ockner at oracle.com
Tue Mar 29 16:56:22 UTC 2016


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