RFR: 8356173: Remove ThreadCritical

[David Holmes]

On Tue, 6 May 2025 20:32:05 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> Updated the description in the bug.  This removes the last use of ThreadCritical and replaces it with a global PlatformMutex lock.
> Tested with tier1-4, and tier1 on all Oracle-supported OSs.

This seems reasonable but I have a few minor queries.

It was surprising how many files included `threadCritical.hpp` for no apparent reason.

Thanks

src/hotspot/share/memory/arena.cpp line 47:

> 45: void Arena::initialize_chunk_pool() {
> 46:   _global_chunk_pool_mutex = new PlatformMutex();
> 47: }

Possibly a candidate for @jdksjolen 's `Deferred<T>`?

src/hotspot/share/memory/arena.cpp line 219:

> 217:     pool->return_to_pool(c);
> 218:   } else {
> 219:     // Free chunks under NMT lock so that NMT adjustment is stable.

NMT lock???

src/hotspot/share/nmt/nmtUsage.cpp line 55:

> 53: 
> 54: void NMTUsage::update_malloc_usage() {
> 55:   // Lock needed keep values in sync, total area size

Suggestion:

  // Lock needed to keep values in sync, total area size

src/hotspot/share/runtime/threads.cpp line 463:

> 461: 
> 462:   // Initialize memory pools
> 463:   Arena::initialize_chunk_pool();

What code first uses a `ChunkPool` or the `ChunkPoolLocker`? (It isn't obvious at what point the NMT code may execute during early initialization.)

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

PR Review: https://git.openjdk.org/jdk/pull/25072#pullrequestreview-2827024851
PR Review Comment: https://git.openjdk.org/jdk/pull/25072#discussion_r2080935325
PR Review Comment: https://git.openjdk.org/jdk/pull/25072#discussion_r2080935989
PR Review Comment: https://git.openjdk.org/jdk/pull/25072#discussion_r2080937284
PR Review Comment: https://git.openjdk.org/jdk/pull/25072#discussion_r2080944116