Request for code review - JDK-8141123 Change int's to size_t in FreeIdSet

[Alexander Harlap]

Hi Kim,

Actually existing code is correct. We should look at 
FreeIdSet::claim_par_id() to see it:

uint FreeIdSet::claim_par_id() {
   MutexLockerEx x(_mon, Mutex::_no_safepoint_check_flag);
   while (_hd == end_of_list) {
     _waiters++;
     _mon->wait(Mutex::_no_safepoint_check_flag);
     _waiters--;
   }
   uint res = _hd;
   _hd = _ids[res];
   _ids[res] = claimed;  // For debugging.
   _claimed++;
   return res;
}

Let assume size=1.
At first call to claim_par_id() it will return 0,  but it cannot return 
1 ( desired behavior)

If we follow you suggestion, than at size=1

At first call to claim_par_id() it will return 0, and at sequential 
(without call to release_par_id()) it will return 1 - wrong behavior.

Thank you,
Alex


On 12/2/2015 2:22 PM, Kim Barrett wrote:
> On Dec 1, 2015, at 3:45 PM, Alexander Harlap <alexander.harlap at oracle.com> wrote:
>> Here is new version, to satisfy comments of Thomas, Kim and Tom.
>> I hope it is better now.
>>
>> http://cr.openjdk.java.net/~aharlap/8141123/webrev.04/
> ------------------------------------------------------------------------------
> src/share/vm/gc/shared/workgroup.cpp
>   504 FreeIdSet::FreeIdSet(uint size, Monitor* mon) :
> ...
>   508   _ids = NEW_C_HEAP_ARRAY(uint, size, mtGC);
>   509   for (uint i = 0; i < size - 1; i++) {
> ...
>   512   _ids[size-1] = end_of_list; // end of list.
>
> [Pre-existing issue]
> I think the allocation should be size+1, the iteration limit should be
> size, and the end_of_list sentinal installed at _ids[size].  As it is
> now, the set size is actually one less than size.  This would be
> particularly problematic if the set were given a size of one.  Since
> the only caller uses the number of processors as the size, the set
> would be empty if there were only one processor.  That's not really
> the target audience for G1, but still...
>
> Sorry I missed this earlier.
>