[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR: 8143155: Remove TraceRuntimeCalls, TraceJNICalls, and TraceJVMCalls rather than convert to 
From:       Rachel Protacio <rachel.protacio () oracle ! com>
Date:       2015-11-23 16:51:22
Message-ID: 5653440A.9080007 () oracle ! com
[Download RAW message or body]

Thank you, David and Dmitry! I'll check it in.
Rachel

On 11/22/2015 11:04 PM, David Holmes wrote:
> Looks good to me too!
>
> Thanks,
> David
>
> On 21/11/2015 7:52 AM, Rachel Protacio wrote:
>> Oh, I see what you mean. Sorry, I misunderstood before. I just updated
>> the webrev.01. Test passes.
>> Thanks,
>> Rachel
>>
>> On 11/20/2015 3:13 PM, Dmitry Dmitriev wrote:
>>> Hi Rachel,
>>>
>>> I don't see VMOptionWarning.java test in webrev.01, but I think that
>>> this test should be modified. I mean that you need to find other
>>> develop and nonproduct flag and use them in VMOptionWarning.java test
>>> instead of deleted "-XX:+TraceJNICalls" and "-XX:+TraceJVMCalls".
>>>
>>> Thanks,
>>> Dmitry
>>>
>>> On 20.11.2015 23:03, Rachel Protacio wrote:
>>>> Thank you, Dmitry and David!
>>>>
>>>> Updated webrev: http://cr.openjdk.java.net/~rprotacio/8143155.01/
>>>> I reinstated the VMOptionWarning.java test cases, and in jvm.cpp
>>>> deleted the unused wrappers. David, thanks for pointing that out. I
>>>> took the liberty of deleting the %s/%d/etc. parts of the messages
>>>> since they previously were printing those out literally (and not
>>>> substituting the variables), which I think was definitely not
>>>> intended. In the unlikely case that you think anyone was relying on
>>>> those exact strings, let me know and I'll change them back?
>>>>
>>>> Thank you,
>>>> Rachel
>>>>
>>>> On 11/19/2015 5:23 PM, David Holmes wrote:
>>>>> Hi Rachel,
>>>>>
>>>>> Overall seems okay. One minor issue below.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>> On 20/11/2015 5:39 AM, Rachel Protacio wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please review this change, which removes TraceRuntimeCalls,
>>>>>> TraceJNICalls, and TraceJVMCalls options. The output from the
>>>>>> options is
>>>>>> excessive and useless, while there are entirely useful options
>>>>>> available, i.e. CountRuntimeCalls, CountJNICalls, and CountJVMCalls.
>>>>>>
>>>>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8143155/
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8143155
>>>>>
>>>>> src/share/vm/prims/jvm.cpp
>>>>>
>>>>> These:
>>>>>
>>>>>    #define JVMWrapper(arg1) JVMCountWrapper(arg1);
>>>>> !   #define JVMWrapper2(arg1, arg2) JVMCountWrapper(arg1);
>>>>> !   #define JVMWrapper3(arg1, arg2, arg3) JVMCountWrapper(arg1);
>>>>> !   #define JVMWrapper4(arg1, arg2, arg3, arg4) 
>>>>> JVMCountWrapper(arg1);
>>>>>
>>>>> should reduce to just the single form now that only 1 arg is needed.
>>>>> And the 3/4 variants are unused anyway. That said something seems a
>>>>> bit broken here as we have things like:
>>>>>
>>>>> JVMWrapper2("JVM_NativePath (%s)", path);
>>>>>
>>>>> which will expand to
>>>>>
>>>>> JVMCountWrapper("JVM_NativePath (%s)")
>>>>>
>>>>> which doesn't really make sense to me. I guess it is harmless, but
>>>>> certainly looks odd. There are only 10 uses to fix up though :)
>>>>>
>>>>>
>>>>>> Thank you!
>>>>>> Rachel
>>>>
>>>
>>

[prev in list] [next in list] [prev in thread] [next in thread]