RFR: 8144874: Reimplement TraceClassResolution with UL

Rachel Protacio rachel.protacio at oracle.com
Mon Dec 21 23:01:14 UTC 2015


Looks good to me! Thanks for addressing my comments.
R

On 12/21/2015 5:00 PM, Max Ockner wrote:
> Oops, meant to reply-all:
>
> These have been fixed. New webrev: 
> http://cr.openjdk.java.net/~mockner/classresolve
> Thanks,
> Max
>
>
> On 12/18/2015 12:56 AM, David Holmes wrote:
>> On 18/12/2015 6:00 AM, Max Ockner wrote:
>>> New webrev: http://cr.openjdk.java.net/~mockner/classresolve
>>
>> Some indentation errors (4 spaces vs 2) have crept into 
>> arguments.cpp. Webrev doesn't show them but the patch does.
>>
>> Thanks,
>> David
>>
>>> On 12/16/2015 6:27 PM, Ioi Lam wrote:
>>>> Hi Max,
>>>>
>>>> [1] I think you need to rebase your changes and send out a new webrev.
>>>> The latest version of argument.cpp already has these
>>>>
>>>> static AliasedFlag const aliased_jvm_logging_flags[] = {
>>>>   { "-XX:+TraceMonitorInflation", "-Xlog:monitorinflation=debug" },
>>>>   { "-XX:-TraceMonitorInflation", "-Xlog:monitorinflation=off" },
>>>>   { NULL, NULL }
>>>> };
>>>
>>> Fixed.
>>>
>>>>
>>>> which would conflict with similar lines that you added.
>>>>
>>>> [2] In the test case ClassResolutionTest.java:
>>>>
>>>> Is there any need for adding "-Xmx64m"? If not, I think it should be
>>>> removed.
>>>
>>> Removed.
>>>
>>>>
>>>> [3] Also, could you send out a sample log that covers all of the
>>>> different logging lines that you have touched?
>>>
>>> What would you like to see? The webrev contains all of the lines that
>>> were touched, and there is only one level of logging here
>>> (-Xlog:classresolve=info)
>>>
>>>>
>>>> I think the "RESOLVE" in the following output is redundant and should
>>>> be removed.
>>>>
>>>> [classresolve] RESOLVE
>>>> ClassResolutionTest$ClassResolutionTestMain$Thing1Handler
>>>> ClassResolutionTest$ClassResolutionTestMain$Thing1
>>>
>>> I agree. This is now gone.
>>>
>>>>
>>>> Thanks
>>>> - Ioi
>>>>
>>>> On 12/16/15 12:44 PM, Max Ockner wrote:
>>>>> Hello all,
>>>>> Please review my code for another Unified Logging conversion.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8144874
>>>>> Webrev: http://cr.openjdk.java.net/~mockner/classresolve/
>>>>>
>>>>> Summary: "-XX:+TraceClassResolution" reimplemented using Unified
>>>>> Logging with classresolve tag and Info level. Support is maintained
>>>>> for TraceClassResolution using the alias table.
>>>>>
>>>>> Tested with: Selection Resolution tests, jtreg tests. This change
>>>>> also adds a jtreg test for the implementation of classresolve, and
>>>>> for the maintained support of TraceClassResolution.
>>>>>
>>>>> Thanks,
>>>>> Max
>>>>
>>>
>



More information about the hotspot-runtime-dev mailing list