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

[Kim Barrett]

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,
>>> 
>>> On Wed, 2015-11-25 at 17:34 -0500, Kim Barrett wrote:
>>>> On Nov 12, 2015, at 4:35 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>>>> 
>>>>> - FreeIdSet::set_safepoint() seems to be never called. I think it
>>>>> allows you to remove a *lot* of unused stuff.
>>>> 
>>>> Perhaps the bug here is that there should be calls to set_safepoint but they are missing?
>>>> 
>>> 
>>> apparently the functionality is not needed, that is, it has not even
>>> been in use in the initial integration of G1 (and it has been added in
>>> that change).
>>> 
>>> So I suggest to just remove it and everything that depended on it
>>> instead of trying to find out what this functionality may have been used
>>> for.
>> 
>> Or this could be a "been there forever" latency bug. 
>> […]
> 
>  ... 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,
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
having the code that enables the race be far away from where the race
occurs makes me quite nervous.