[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8305566: ZGC: gc/stringdedup/TestStringDeduplicationFullGC.java#Z failed with SIGSEGV in
From: Thomas Schatzl <tschatzl () openjdk ! org>
Date: 2023-04-26 14:57:54
Message-ID: jgdht8TLtGXkVcj_SU_twnigJjRNs2uMbziXLN_QWR0=.a0289fd9-bbe8-4682-8818-eb424431cb6c () github ! com
[Download RAW message or body]
On Mon, 24 Apr 2023 09:25:01 GMT, Kim Barrett <kbarrett@openjdk.org> wrote:
> > Please review this change to the string deduplication thread to make it a kind
> > of JavaThread rather than a ConcurrentGCThread. There are several pieces to
> > this change:
> >
> > (1) New class StringDedupThread (derived from JavaThread), separate from
> > StringDedup::Processor (which is now just a CHeapObj instead of deriving from
> > ConcurrentGCThread). The thread no longer needs to or supports being stopped,
> > like other similar threads. It also needs to be started later, once Java
> > threads are supported. Also don't need an explicit visitor, since it will be
> > in the normal Java threads list. This separation made the changeover a little
> > cleaner to develop, and made the servicability support a little cleaner too.
> >
> > (2) The Processor now uses the ThreadBlockInVM idiom to be safepoint polite,
> > instead of using the SuspendibleThreadSet facility.
> >
> > (3) Because we're using ThreadBlockInVM, which has a different usage style
> > from STS, the tracking of time spent by the processor blocked for safepoints
> > doesn't really work. It's not very important anyway, since normal thread
> > descheduling can also affect the normal processing times being gathered and
> > reported. So we just drop the so-called "blocked" time and associated
> > infrastructure, simplifying Stat tracking a bit. Also renamed the
> > "concurrent" stat to be "active", since it's all in a JavaThread now.
> >
> > (4) To avoid #include problems, moved the definition of
> > JavaThread::is_active_Java_thread from the .hpp file to the .inline.hpp file,
> > where one of the functions it calls also is defined.
> >
> > (5) Added servicability support for the new thread.
> >
> > Testing:
> > mach5 tier1-3 with -XX:+UseStringDeduplication.
> > The test runtime/cds/DeterministicDump.java fails intermittently with that
> > option, which is not surprising - see JDK-8306712.
> >
> > I was never able to reproduce the failure; it's likely quite timing sensitive.
> > The fix of changing the type is based on StefanK's comment that ZResurrection
> > doesn't expect a non-Java thread to perform load-barriers.
>
> Kim Barrett has updated the pull request incrementally with one additional commit \
> since the last revision:
> fix include order
Please improve the bug name as suggested by @shipilev before pushing.
Looks good otherwise.
-------------
Marked as reviewed by tschatzl (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/13607#pullrequestreview-1402152540
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic