RFR: 8219613: Use NonJavaThread PtrQueues

[zgu at redhat.com]

On Tue, 2019-02-26 at 17:35 -0500, 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:
> > > 
> > > Hi Kim,
> > > 
> > > 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)
> > > 
> > > 2) The alternative is to use on_thread_create(), but it is called
> > > from
> > > Thread's constructor, therefore, I can not tell what kind of
> > > thread it
> > > is.
> > > 
> > > Pleas let me know if I miss something.
> > 
> > I think you might be correct that there is a problem, but I think
> > the
> > problem is in Shenandoah code, and pre-exists this patch.
> > 
> > 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.
> 
> 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.)
Yes, I am aware of that we initialize some threads' gclab twice,
especially now, JavaThread has gclab initialized in on_thread_attach().

I am working on this.

Thanks,

-Zhengyu

>