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

Thomas Schatzl thomas.schatzl at
Thu Nov 12 09:35:20 UTC 2015

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:
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 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.


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 :)


More information about the hotspot-gc-dev mailing list