[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Integrated: 8305566: Change StringDedup thread to derive from JavaThread
From:       Kim Barrett <kbarrett () openjdk ! org>
Date:       2023-04-28 3:35:53
Message-ID: rDCy4Grp8RiPrkl5x5-FCcI4AY2RiThUCpZtbN49yhw=.b9b39bf5-f8b6-432c-872d-d9558c345cd7 () github ! com
[Download RAW message or body]

On Mon, 24 Apr 2023 08:24:53 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.

This pull request has now been integrated.

Changeset: d3abfec8
Author:    Kim Barrett <kbarrett@openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/d3abfec8b7ce901150952356f9f1109d09a8cb2a
Stats:     440 lines in 18 files changed: 193 ins; 146 del; 101 mod

8305566: Change StringDedup thread to derive from JavaThread

Reviewed-by: stefank, cjplummer, tschatzl

-------------

PR: https://git.openjdk.org/jdk/pull/13607
[prev in list] [next in list] [prev in thread] [next in thread]