RFR: 8219613: Use NonJavaThread PtrQueues

[zgu at redhat.com]

On Tue, 2019-02-26 at 17:10 -0500, Kim Barrett 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.

Yes, we current use other mechanism to initialize worker's gclab, so it
works correctly right now. I were hoping I could use on_thread_xxx
mechanism to replace our mechanism, so we can simplify a few things.

I still have a couple of concerns regarding current on_thread_xxx
implementation.

1) on_thread_create() callback, actually sees partially constructed
thread if the thread is a subclass of Thread.

2) For JavaThread, on_thread_attach() is called from creator's thread,
but non-Java thread is from the thread itself.

Thanks,

-Zhengyu

> 
> WorkGang::threads_do is very little used, and could _almost_ go away.
> It's there to support gc_threads_do and some related things, which
> are
> (I think only) used for error printing and the like.  MemoryService
> also uses gc_threads_do, to count the number of threads (a count that
> doesn't seem to get updated as the GC lazily allocates additional
> threads, which seems like a separate bug).
> 
> Something like WorkGang::threads_do might be needed for destroying
> work gangs, but we don't ever do that at present.
>