RFR: 8080947: Add uint as a valid VM flag type

Dmitry Dmitriev dmitry.dmitriev at oracle.com
Thu Jun 4 12:00:45 UTC 2015


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