RFR: 8305566: Change StringDedup thread to derive from JavaThread [v3]
Kim Barrett
kbarrett at openjdk.org
Fri Apr 28 03:26:53 UTC 2023
> 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 12 additional commits since the last revision:
- Merge branch 'master' into jt-strdedup
- fix include order
- fix stray tab
- move is_active_Java_thread
- copyrights
- servicabilty support
- use JavaThread
- separate thread class
- simplify init
- do not pass around STS joiner
- ... and 2 more: https://git.openjdk.org/jdk/compare/8957f67e...da07a420
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/13607/files
- new: https://git.openjdk.org/jdk/pull/13607/files/f17cc6be..da07a420
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=13607&range=02
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=13607&range=01-02
Stats: 46789 lines in 794 files changed: 30868 ins; 11396 del; 4525 mod
Patch: https://git.openjdk.org/jdk/pull/13607.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/13607/head:pull/13607
PR: https://git.openjdk.org/jdk/pull/13607
More information about the serviceability-dev
mailing list