Request for review: 8016302: Change type of the number of GC workers to unsigned int (2)

Jesper Wilhelmsson jesper.wilhelmsson at oracle.com
Thu Apr 3 13:36:37 UTC 2014


Looks good!
/Jesper

Vladimir Kempik skrev 3/4/14 15:32:
> Hello
>
> Updated webrev, for loop changed to use UINT_MAX
>
> http://cr.openjdk.java.net/~vkempik/8016302/webrev.05/
>
> Thanks.
> Vladimir.
>
> On 03.04.2014 16:35, Jesper Wilhelmsson wrote:
>> Hi,
>>
>> It seems like UINT32_FORMAT is frequently used together with uint so leave it
>> as is.
>> /Jesper
>>
>> Vladimir Kempik skrev 3/4/14 13:41:
>>> Hello
>>>
>>> I've used UINT32_FORMAT because:
>>>
>>> 1) previous format type for worker was INT32_FORMAT, when worker was int
>>>
>>> 2) there is no such thing as UINT_FORMAT,  there is only UINTX_FORMAT
>>>
>>> Do you think UINTX_FORMAT is better than UINT32_FORMAT for this case ?
>>>
>>> Thanks, Vladimir
>>> On 03.04.2014 14:30, Jesper Wilhelmsson wrote:
>>>> Hi,
>>>>
>>>> In concurrentG1Refine.cpp:63, could you use uint for i in that loop as well
>>>> and change the termination condition to i != UINT_MAX as you have done in a
>>>> few other places?
>>>>
>>>> In g1GCPhaseTimes.cpp, g1HotCardCache.cpp and g1RemSet.cpp, why do you use
>>>> UINT32_FORMAT, shouldn't it be just UINT_FORMAT?
>>>>
>>>> Otherwise, looks good!
>>>> /Jesper
>>>>
>>>>
>>>> Vladimir Kempik skrev 2/4/14 18:17:
>>>>> Hi all,
>>>>>
>>>>> Could I have a couple of reviews for this change?
>>>>>
>>>>> http://cr.openjdk.java.net/~vkempik/8016302/webrev.04/
>>>>>
>>>>> In 7121618 variables representing GC workers (worker id, worker id offset)
>>>>> have
>>>>> been changed from int to unsigned int.
>>>>>
>>>>> Since then, code reintroduced the use of int's for this type of variable;
>>>>> fixing
>>>>> this by aligning the code to use uints for ints.
>>>>>
>>>>> Since last september the fix was updated for jdk9 and size_t was replaced with
>>>>> uint in
>>>>> dirtyCardQueue.hpp,
>>>>>
>>>>>    54 bool apply_closure(CardTableEntryClosure* cl,
>>>>>    55                      bool consume = true,
>>>>>    56                      size_t worker_i = 0);
>>>>>
>>>>>    99   // The number of parallel ids that can be claimed to allow
>>>>> collector or
>>>>>   100   // mutator threads to do card-processing work.
>>>>>   101   static size_t num_par_ids();
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>
>



More information about the hotspot-gc-dev mailing list