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

[Alexander Harlap]

Here is new revision of change:

http://cr.openjdk.java.net/~aharlap/8141123/webrev.01/

Alex

On 11/12/2015 4:35 AM, Thomas Schatzl wrote:
> Hi Alex,
>
> On Wed, 2015-11-11 at 14:55 -0500, Alexander Harlap wrote:
>> Change int's to size_t in FreeIdSet
>> JDK-8141123
>>
>> Proposed change:
>>
>> http://cr.openjdk.java.net/~aharlap/8141123/webrev.00
>>
> Some comments:
>
>    - could you use a current version of webrev that adds links to the
> bottom of the page? That would improve the "review experience" a lot :)
> I think the current one from
> http://openjdk.java.net/guide/webrevHelp.html does that.
>
>    - please rename the _sz member to _size. Also, it should be an
> unsigned type.
>    - _index should be size_t too if I read the code correctly.
>
>    - the return value of claim_par_id() should also be uint.
>
>    - please add braces and newlines around one-line loops etc.
>
> E.g.
>
> 516     for (int j = 0; j < NSets; j++) _sets[j] = NULL;
>
> should be
>
> for (int j = 0; j < NSets; j++) {
>    _sets[j] = NULL;
> }
>
>   - I think the allocation kind of the _ids array should be mtGC, not
> mtInternal. Only GC uses it.
>
>   - remove the FreeIdSetPtr typdef. It is not consistently used, and
> actually there is only one use anyway.
>
>   - FreeIdSet::set_safepoint() seems to be never called. I think it
> allows you to remove a *lot* of unused stuff.
>
>   - same with claim_perm_id().
>
>   - please remove all code ifdef'ed by FID_STATS. I doubt anyone ever
> used it for a long time.
>
> That should be enough for now :)
>
> Thanks,
>    Thomas
>
>