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

[Thomas Schatzl]

Hi,

On Wed, 2015-12-02 at 16:08 -0500, Kim Barrett wrote:
> On Dec 2, 2015, at 3:35 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> > 
> > Hi,
> > 
> > On Tue, 2015-12-01 at 18:45 -0500, Kim Barrett wrote:
> >> On Nov 26, 2015, at 6:13 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> >>> 
> >>> Hi,
> >>> 
> [...] 
> >  ... and reading through all this investigation, actually I think the
> > whole FreeIdSet is unnecessary. The mutator thread could just take any
> > random valid thread id and the FromCardCache would still work (although
> > at reduced efficiency, but that is the case in that situation already
> > anyway).
> > 
> > Ideally there would be some small extra cache, or we just do not use the
> > cache in this situation which seems a reasonable option too (or let all
> > mutators use a single equal extra id), to not thrash the FromCardCache
> > of the refinement threads.
> > 
> > I am leaving it to Alex and you to decide whether you want this cleanup
> > as an intermediate step, or go for a full cleanup.
> 
> I'm fine with treating the work Alex has done so far as a good
> intermediate cleanup.  I'll file followup bugs about the additional
> issues uncovered here.
> 
> Attempting to eliminate FreeIdSet certainly has appeal.  However, I
> don't have any intuition about the importance of the FromCardCache,

The fromcardcache caches for every combination of thread id and region
which card has most recently been added to the remembered set.

It purely acts as a filter of repeated attempts to add the same card by
the same thread to the remembered set. It seems to be quite effective at
doing so in many cases.

> which makes me reluctant to prevent the mutator threads from using it.
> But I'd be even more uncomfortable allowing mutator threads to
> *simultaneously* share a worker_id / cache key.  I think the races
> that would ensue are, in the current code, probably harmless.  But

>From what I looked at, they are harmless. Sharing a single thread id may
or may not be less problematic than for every insert check whether the
given thread id is the special thread id.

The existing rset code already seems have the requirement to be able to
handle parallel card insertion attempts with the same thread id. I think
in the situation of a claimed thread id from the free-id-set by mutator
threads matches one of a refinement thread. Or native code always using
thread id zero to add remembered set entries...

> having the code that enables the race be far away from where the race
> occurs makes me quite nervous.

I agree to some degree. The problem is that the rset implementation
needs to handle this situation already anyway. It's just a matter of
exploiting this requirement again vs. decreased complexity in the
refinement handling.

I know what I would choose if I were asked, and somebody verified again
what I described above - only had a brief look, and using some details
from memory.

Thanks,
  Thomas