RFR: 8144874: Reimplement TraceClassResolution with UL

David Holmes david.holmes at oracle.com
Wed Dec 23 04:10:19 UTC 2015


Thanks Max that all looks okay now. There are some preexisting 
indentation errors.

Cheers,
David

On 23/12/2015 4:25 AM, Max Ockner wrote:
> http://cr.openjdk.java.net/~mockner/classresolve/
> That has been removed. I inspected the webrev again, and it looks like
> all of the faulty spacing is gone.
> Max
> On 12/22/2015 7:30 AM, David Holmes wrote:
>> On 22/12/2015 8:00 AM, Max Ockner wrote:
>>> Oops, meant to reply-all:
>>>
>>> These have been fixed. New webrev:
>>> http://cr.openjdk.java.net/~mockner/classresolve
>>
>> I still see this:
>>
>>     // Catch -XX options which are aliased to Unified logging commands.
>>     if (match_option(option, "-XX:", &tail)) {
>>         if (lookup_logging_aliases(option->optionString,
>> aliased_logging_option)) {
>>             option->optionString = aliased_logging_option;
>>         }
>>     }
>>
>> indent 4 instead of 2.
>>
>> David
>>
>>> 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