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

Kim Barrett kim.barrett at oracle.com
Wed Nov 25 22:36:28 UTC 2015


On Nov 25, 2015, at 4:09 PM, Alexander Harlap <alexander.harlap at oracle.com> wrote:
> 
> Here is new revision :
> 
> http://cr.openjdk.java.net/~aharlap/8141123/webrev.03

------------------------------------------------------------------------------ 
src/share/vm/gc/shared/workgroup.cpp
 540     // Notify all would be safer, but this is OK, right?
 541     _mon->notify_all();

The comment seems a little odd, considering the code.  Perhaps at some
point in the development it was calling notify instead of notify_all?
Though it has always been notify_all since it was introduced as part
of the initial G1 integration.  I suggest removing the comment.

------------------------------------------------------------------------------ 
src/share/vm/gc/shared/workgroup.hpp
 400   // Returns an unclaimed parallel id (waiting for one to be released if
 401   // necessary).  Returns "UINT_MAX" if a GC wakes up a wait for an id.
 402   uint claim_par_id();

The return of UINT_MAX (really, end_of_list, but that's non-public) no
longer occurs, due to the elimination of the _safepoint check in the
loop.  [But see my other question, about whether the real problem is
that there should be calls to set_safepoint, but they are missing.]

------------------------------------------------------------------------------ 
src/share/vm/gc/shared/workgroup.cpp
 526   int res = _hd;

res should be uint now.  [Tom also pointed this out.]

------------------------------------------------------------------------------



More information about the hotspot-gc-dev mailing list