RFR(xs): 8134744: WorkerThread::id() should match GangWorker's worker_id

[Per Liden]

Hi Derek,

On 2015-09-01 00:12, Derek White wrote:
> Hi Per,
>
> I'm confused on the same issue, or I've gotten lost between thinking
> about [Abstract] (WorkGang | GangWorker) :-)
>
> AbstractWorkGang::initialize_workers() assigns ids from
> 0..total_workers(). This id is also GangWorker's index into the
> AbstractWorkGang's_worker array. So changing GangWorker's id breaks
> implied consistency that AbstractWorkGang::worker(some_gang_worker._id)
> == some_gang_worker. Right? This seem more confusing than the WorkData's
> _worker_id != GangerWorkers._id.

When GangWorkers are created they are assigned id 0...total_workers - 1, 
for the sole purpose of being able to create a unique string to 
represent its name ("GCWorker#1", "GCWorker#2", etc). This id has no 
true relation to how the WorkGang wants to organize the threads. I.e. 
it's not really used as an index into WorkGang's private array of 
GangWorkers.

>
> Note that I didn't see how WorkerThread::id(), mentioned below, fits in
> this picture.

GangWorker inherits from WorkerThread.

>
> Since the WorkData's _worker_id is arbitrarily handed out, but the
> GangWorker's id is an index into an array, it seems safer to keep
> GangWorker's ID static, and setting WorkData's id when it gets assigned
> to a worker.

That wouldn't work with how this is used today. When you execute 
gang->run_task(task), the task::work(worker_id) function should be 
called as many times as there are workers, with each worker_id being unique.

I think the fundamental problem here is that the concepts of "work id" 
and "worker id" is kind of mixed here and it didn't really matter before 
the recent changes to the GangWorker because they used to always be the 
same. The recent changes does provide a really nice optimization, but 
exposes this inconsistency.

The alternative here would of course be to just leave this as is, but 
I'm worried we might see other bugs like this.

Do other people have opinions on this?

cheers,
/Per

>
>   - Derek
>
> On 8/31/15 2:35 PM, Jon Masamitsu wrote:
>> Per,
>>
>> A GangWorker is given an "uint id" when it is
>> created.  If it is going to be reset the id whenever it
>> does work, should it have its own "id"?
>>
>> Jon
>>
>> On 08/31/2015 07:16 AM, Per Liden wrote:
>>> Hi,
>>>
>>> This is a follow up patch to JDK-8134749 (SoftReferences declared
>>> dead too early), which makes sure WorkerThread::id() and match
>>> GangWorker's worker_id always match. This allows them to be used
>>> interchangeably, which is both convenient and removes a potential
>>> source for bugs (it's easy to assumed they are the same). This change
>>> should not affect any existing code.
>>>
>>> Webrev: http://cr.openjdk.java.net/~pliden/8134744/webrev.0/
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8134744
>>>
>>> Testing: jprt
>>>
>>> /Per
>>>
>>
>