RFR: 8219613: Use NonJavaThread PtrQueues

[Aleksey Shipilev]

On 2/26/19 11:35 PM, Kim Barrett wrote:
>> On Feb 26, 2019, at 5:10 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>> On Feb 26, 2019, at 10:58 AM, zgu at redhat.com wrote:
>>> Probably not directly relates to this RFR, but I see a couple of issues
>>> with this BarrierSet::on_thread_xxx mechanism.
>>>
>>> 1) When creating worker thread, it is added to WorkGang's thread first,
>>> and on_thread_attach() is not called until worker thread starts to run.
>>> If I use on_thread_attach() to initialize something (e.g. gclab), I may
>>> hit this timing gap, because WorkGang::threads_do() may hit
>>> uninitialized data (e.g. gclab)

I think this is resolved specially in Shenandoah::post_initialize, when GC workers get their initial
GCLABs. Zhengyu, can you confirm this?

>> ShenandoahHeap::make_parsable and
>> ShenandoahHeap::retire_and_reset_gclabs seem to be looking at the
>> gclab data of non-Java threads, and going to some effort to do so.
>> But the per-thread gclab info only gets initialized for Java threads.
>> Neither of those are changed by this patch.
>>
>> In pre-RFR discussion with Aleksey about the Shenandoah part of this
>> patch, he said non-Java threads don't use the gclab mechanism, so only
>> initializing the per-thread gclab data for Java threads was (and
>> remains) correct.  But make_parsable and retire_and_reset_gclabs are
>> looking at that data anyway.

I think this is still correct. Worker threads are indeed non-Java threads, but they are handled
specially in ShenandoahWorkGang::install_worker, where their gclab is getting initialized. This
"exception" is why make_parsable, retire_and_reset_gclab and friends are applying closure to GC
worker threads. And why ShenandoahInitGCLABClosure applies to all threads, and then filter by
(thread->is_Java_thread() || thread->is_Worker_thread()) -- this is needed to resolve some
initialization races.

> Also, ShenandoahHeap::post_initialize applies the ShenandoahInitGCLABClosure
> to "all" threads (via Threads::threads_do), and some worker threads
> (via WorkGang::threads_do on the various work gangs), which will
> double call the closure (unless one is "lucky" and hits the race.)

Oh, double-visiting is not nice indeed! Need to fix that, thanks. This issue does not block this patch.

-Aleksey

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20190226/d78df6cf/signature.asc>