On Thu, 27 Aug 2026 08:50:20 GMT, Axel Boldt-Christmas <[email protected]> 
wrote:

>> Historically we have had displaced markWord's because locking (stack locks, 
>> and inflated monitors).
>> 
>> After the `UseObjectMonitorTable` removal there are no more need to protect 
>> against displaced markWords.
>> 
>> I suggest we simplify this logic and clean up the interface w.r.t. 
>> `identity_hash`.
>> 
>> Changes the `fast_no_hash_check` and `has_no_hash` into `has_identity_hash` 
>> and `has_hash` checks. 
>> As before it is invalid to read the hash on marked objects, added asserts to 
>> ensure this.
>> Rewrote `oopDesc::slow_identity_hash` to not regenerate a new hash every 
>> time it transiently fails to install the hash in the markWord due to some 
>> other header change.
>> 
>> There are a few places which right now calls `identity_hash` from develop 
>> logging / introspection printing `InstanceStackChunkKlass::print_chunk` and 
>> `Continuation::print`. It is a bit unfortunate that we have these 
>> side-effects in debug VMs. But think we should handle these in a future RFE.
>> 
>> Testing (in progress):
>> * Tier 1-3 Oracle supported platforms
>> * GHA
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Axel Boldt-Christmas has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update markWord comments

I like it. A few more nits.

src/hotspot/share/cds/aotMappedHeapWriter.cpp line 732:

> 730:   address buffered_addr = 
> requested_addr_to_buffered_addr(cast_from_oop<address>(requested_obj));
> 731: 
> 732:   markWord mw = Arguments::enable_preview() ? 
> src_klass->prototype_header() : markWord::prototype();

Suggestion:

  markWord mw = Arguments::is_valhalla_enabled() ? 
src_klass->prototype_header() : markWord::prototype();

like what the old code used. Or is there a reason why `enable_preview` is more 
correct here?

src/hotspot/share/oops/markWord.hpp line 78:

> 76: //    Inline types cannot be locked.
> 77: //    They have a deterministic hash based on the immutable payload and 
> class,
> 78: //    which may be cached in the markWord.

The new line break makes this look like two paragraphs, but without the nice 
separation that we have between blocks in the rest of this comment section. I'd 
suggest either of:

Suggestion:

//    Inline types cannot be locked. They have a deterministic hash based on
//    the immutable payload and class, which may be cached in the markWord.


or

//    Inline types cannot be locked.
// 
//    Inline types have a deterministic hash based on the immutable payload
//    and class, which may be cached in the markWord.


Hmm. While reading this I realize that the usage of "Inline types" sounds 
wrong. It is the objects and not the types that we are referring to. I prefer 
to write "value objects" when we talk about concrete value instances, as we do 
here.

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

PR Review: https://git.openjdk.org/jdk/pull/32535#pullrequestreview-5039014669
PR Review Comment: https://git.openjdk.org/jdk/pull/32535#discussion_r3870262503
PR Review Comment: https://git.openjdk.org/jdk/pull/32535#discussion_r3870253594

Reply via email to