RFR: 8080947: Add uint as a valid VM flag type
David Lindholm
david.lindholm at oracle.com
Thu Jun 4 12:09:28 UTC 2015
Dmitry,
Thanks, you are correct. Fixed now. Do you need another webrev?
Thanks,
David
On 2015-06-04 14:00, Dmitry Dmitriev wrote:
> Hello David,
>
> In src/share/vm/runtime/globals.cpp module:
>
> 738 bool CommandLineFlags::uintAtPut(const char* name, size_t len,
> uint* value, Flag::Flags origin) {
> 739 Flag* result = Flag::find_flag(name, len);
> 740 if (result == NULL) return false;
> 741 if (!result->is_uint()) return false;
> 742 uint old_value = result->get_uint();
> 743 trace_flag_changed<EventUnsignedIntFlagChanged, u4>(name,
> old_value, *value, origin);
> 744 result->set_int(*value);
> 745 *value = old_value;
> 746 result->set_origin(origin);
> 747 return true;
> 748 }
> 749
> 750 void CommandLineFlagsEx::uintAtPut(CommandLineFlagWithType flag,
> uint value, Flag::Flags origin) {
> 751 Flag* faddr = address_of_flag(flag);
> 752 guarantee(faddr != NULL && faddr->is_uint(), "wrong flag type");
> 753 trace_flag_changed<EventUnsignedIntFlagChanged,
> u4>(faddr->_name, faddr->get_uint(), value, origin);
> 754 faddr->set_int(value);
> 755 faddr->set_origin(origin);
> 756 }
>
> On lines 744 and 754 "set_int" method is called for unsigned flags.
> Probably "set_uint" method should be called?
>
> Regards,
> Dmitry
>
> On 04.06.2015 14:40, David Lindholm wrote:
>> Hi,
>>
>> Thanks. I have put up a new webrev with a few small changes:
>>
>> http://cr.openjdk.java.net/~david/JDK-8080947/webrev.hotspot.01/
>>
>>
>> Thanks,
>> David
>>
>> On 2015-06-04 03:09, Coleen Phillimore wrote:
>>>
>>> David, If you have all the reviews you need, you can integrate.
>>> Gerard is working through review comments and has to retest, and is
>>> fine with merging with your change.
>>>
>>> Thanks everyone for all the thorough reviews of the command line
>>> validation change.
>>>
>>> Coleen
>>>
>>> On 6/3/15 4:51 AM, David Lindholm wrote:
>>>> Hi David,
>>>>
>>>> Thanks for looking at this. I have a few places where I convert
>>>> uint and int to Java types, besides management.cpp also
>>>> whitebox.cpp/java and VM.java . After discussing with several
>>>> people we though it was most correct to go with JLONG as java type
>>>> for both int and uint, since it is not certain that an uint will
>>>> fit in a JINT and I wanted the same java type for both int and uint.
>>>>
>>>> I don't think the C spec specifies the size of int (please correct
>>>> me if I'm wrong), so having JLONG as type for int and uint is safer
>>>> than JINT.
>>>>
>>>> But I can change to JINT if you think that is better.
>>>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 2015-06-03 10:01, David Holmes wrote:
>>>>> Hi David,
>>>>>
>>>>> On 28/05/2015 9:28 PM, David Lindholm wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please review this patch that adds uint and int as valid VM flag
>>>>>> types.
>>>>>> This patch adds the possibility to specify VM flags with types
>>>>>> int and
>>>>>> uint, it does not change the type of any flags.
>>>>>>
>>>>>>
>>>>>> Webrev:
>>>>>> http://cr.openjdk.java.net/~david/JDK-8080947/webrev.hotspot.00/
>>>>>> Webrev: http://cr.openjdk.java.net/~david/JDK-8080947/webrev.jdk.00/
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8080947
>>>>>
>>>>> src/share/vm/services/management.cpp
>>>>>
>>>>> + } else if (flag->is_int()) {
>>>>> + global->value.j = (jlong)flag->get_int();
>>>>> + global->type = JMM_VMGLOBAL_TYPE_JLONG;
>>>>> + } else if (flag->is_uint()) {
>>>>> + global->value.j = (jlong)flag->get_uint();
>>>>> + global->type = JMM_VMGLOBAL_TYPE_JLONG;
>>>>>
>>>>> These should be JINT types not JLONG.
>>>>>
>>>>> Cheers,
>>>>> David H.
>>>>> -------
>>>>>
>>>>>>
>>>>>> Testing: Passed JPRT
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>
>>>
>>
>
More information about the hotspot-dev
mailing list