RFR: 8324492: Remove Atomic support for OopHandle

[Axel Boldt-Christmas]

On Tue, 23 Jan 2024 10:52:06 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

> Please review this change to the lazy initialization of the MemoryManager
> object and the associated MemoryPool objects.
> 
> They previously used an atomic access to the respective OopHandle member
> holding the associated Java object as the is-initialized sentinal, testing
> whether the handle was empty or had an associated OopStorage entry. When
> empty, initialization was performed using a lock to prevent races.
> 
> Now they use a separate atomic is-initialized flag as the sentinal.
> 
> As a result, the support for atomic access to an OopHandle's underlying handle
> (via a translator) is no longer needed and is removed.
> 
> While there, I moved the allocation of the associated OopStorage entries out
> from under the Management_lock.
> 
> Testing: mach5 tier1
> 
> A couple of notes for reviewers.
> 
> Once initialized with a Java object recorded in the associated OopHandle, the
> OopHandle and the value recorded therein is never changed.
> 
> The old is-initialized check makes use of OopHandle::resolve returning null if
> either the handle is empty (has no OopStorage entry yet) or the OopStorage
> entry contains null. The latter never happens in this case.

Changes looks good to me.

Just had a couple of questions/comments.

src/hotspot/share/services/memoryManager.cpp line 147:

> 145:     } else {
> 146:       // Record the object we created via call_special.
> 147:       _memory_mgr_obj = mgr_handle;

Could assert that `_memory_mgr_obj.is_empty()`.

src/hotspot/share/services/memoryManager.hpp line 66:

> 64: 
> 65: public:
> 66:   virtual ~MemoryManager() = default; // FIXME

Was this added because we are deleting these objects through the base class pointer? Or just in case?

Regardless what is the `FIXME` comment for?

src/hotspot/share/services/memoryPool.cpp line 142:

> 140:     } else {
> 141:       // Record the object we created via call_special.
> 142:       _memory_pool_obj = pool_handle;

Could assert that `_memory_pool_obj.is_empty()`.

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

Marked as reviewed by aboldtch (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17533#pullrequestreview-1841143156
PR Review Comment: https://git.openjdk.org/jdk/pull/17533#discussion_r1464773008
PR Review Comment: https://git.openjdk.org/jdk/pull/17533#discussion_r1464767168
PR Review Comment: https://git.openjdk.org/jdk/pull/17533#discussion_r1464767954