8389938: Follow up after removing UseObjectMonitorTable flag by fbredber · Pull Request #32471 · openjdk/jdk

fbredber · 2026-08-20T14:13:27Z

This PR is a follow-up to JDK-8389325 ("Remove the UseObjectMonitorTable flag and related code"). It:

Removes obsolete mark-word and monitor-pointer support.
Removes displaced-mark related code in garbage collectors (G1, Parallel GC and Shenandoah).
Renames UseObjectMonitorTableTest to ObjectMonitorTableTest.
Addresses all the leftover review comments from JDK-8389325.

Passes tier1-5 tests successfully on supported platforms.

I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

JDK-8389938: Follow up after removing UseObjectMonitorTable flag (Enhancement - P4)

Reviewers

Patricio Chilano Mateo (@pchilano - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/32471/head:pull/32471
$ git checkout pull/32471

Update a local copy of the PR:
$ git checkout pull/32471
$ git pull https://git.openjdk.org/jdk.git pull/32471/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 32471

View PR using the GUI difftool:
$ git pr show -t 32471

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/32471.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2026-08-20T14:14:53Z

👋 Welcome back fbredberg! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-08-20T14:15:00Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2026-08-20T14:15:56Z

@fbredber The following labels will be automatically applied to this pull request:

hotspot
serviceability
shenandoah

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

openjdk · 2026-08-20T14:15:59Z

The total number of required reviews for this PR has been set to 2 based on the presence of this label: hotspot. This can be overridden with the /reviewers command.

mlbridge · 2026-08-21T12:25:14Z

Webrevs

00: Full (dba3cffb)

pchilano

Looks good to me, thanks for the cleanup.

pchilano · 2026-08-21T14:49:44Z

  // to the ObjectMonitor reference manipulation code:
  //
  #define OM_OFFSET_NO_MONITOR_VALUE_TAG(f) \
    ((in_bytes(ObjectMonitor::f ## _offset())) - checked_cast<int>(markWord::monitor_value))


This macro can also be removed.

pchilano · 2026-08-21T14:50:50Z

-  bool has_displaced_mark_helper() const {
-    return has_monitor_pointer();
-  }
-  markWord displaced_mark_helper() const;


I see the SA agent has the equivalent methods hasDisplacedMarkHelper/displacedMarkHelper.

coleenp

Some questions and suggested additions.

coleenp · 2026-08-21T17:09:01Z

    // have to check has_monitor() before is_locked()
    // Valhalla: inline types/arrays can't be monitored
-    st->print(" monitor(" INTPTR_FORMAT ")=", value());
+    st->print(" monitor(" INTPTR_FORMAT ")", value());


But the monitor isn't the value now. Shouldn't this just be removed? We don't want printing to search the table.

I think this whole method could use some cleanup. This just prints the value of the markWord (same as before) but it did so because it did not want to go chasing displaced marked words. Similarly the is_locked leg did not want to go chase stack locks.

Neither exists anymore. It is really only the GC that displaces/preservers the markWord in a STW collection because of forwarding pointers. So we should clean this up to be if marked just print the value, else actually print some more state.

Similarly we should clean up the valhalla invariants now that we do not displace markWords.

coleenp · 2026-08-21T19:36:45Z

@@ -235,9 +235,6 @@
  void                set_metadata(uintptr_t value);
  volatile uintptr_t* metadata_addr();


If I remember this correctly metadata could be two different things, but with the lock in the markWord gone, it can now only be one thing. But I guess that one thing is the hash code as uintptr_t ?
Is metadata_addr() used?

I agree we should probably fold metadata into hash functions and rename and retype the field to be intptr_t _hash a bit more churn but feels like we do not benefit from this abstraction anymore.

Separate but would also be nice to migrate the object monitors to Atomic.

coleenp · 2026-08-21T19:37:09Z

-  ShouldNotCallThis();
-  set_metadata(hdr.value());
-}
-


Is there also a set_metadata() that should be removed?

xmas92

There is still a stale comment in one of our test frameworks:

jdk/test/hotspot/jtreg/vmTestbase/nsk/share/gc/gp/misc/HashedGarbageProducer.java

Lines 34 to 48 in dba3cff

    
                The description is misleading.  I looked at some old email, and the 
        
                goal is to stress the code that deals with displaced mark words, so 
        
                the description should be more like "Stress tests for displaced mark 
        
                words."  In hotspot, each object has a mark word that stores several 
        
                things about the object including its hash code (if it has one) and 
        
                lock state.  Most objects never have a hash code and are never locked, 
        
                so the mark word is empty. 
        
                Most of our garbage collectors use the mark word temporarily during GC 
        
                to store a 'forwarding pointer.'  It's not important what that is, but 
        
                it means that objects that have a hash code or that are locked have to 
        
                have the mark word saved during GC and then restored at the end of GC. 
        
                We want to exercise this saving/restoring code.  So a test case should 
        
                have a large percentage (40-50%) of objects that either have a hash 
        
                code or are locked.

I think removing the first paragraph and rewriting the second one to just mention GC mark word preservation is enough to explain why this code still does System.identityHashCode(obj);

fbredber added 2 commits August 19, 2026 15:48

8389938: Follow up after removing UseObjectMonitorTable flag

858f45e

Renamed UseObjectMonitorTableTest.java to ObjectMonitorTableTest.java

bf50a80

openjdk Bot added serviceability serviceability-dev@openjdk.org hotspot hotspot-dev@openjdk.org shenandoah shenandoah-dev@openjdk.org labels Aug 20, 2026

Merge branch 'master' into 8389938_follow_up_removed_use_omt

dba3cff

fbredber marked this pull request as ready for review August 21, 2026 12:20

openjdk Bot added the rfr Pull request is ready for review label Aug 21, 2026

pchilano approved these changes Aug 21, 2026

View reviewed changes

coleenp reviewed Aug 21, 2026

View reviewed changes

xmas92 reviewed Aug 24, 2026

View reviewed changes

		@@ -235,9 +235,6 @@
		void set_metadata(uintptr_t value);
		volatile uintptr_t* metadata_addr();

	The description is misleading. I looked at some old email, and the
	goal is to stress the code that deals with displaced mark words, so
	the description should be more like "Stress tests for displaced mark
	words." In hotspot, each object has a mark word that stores several
	things about the object including its hash code (if it has one) and
	lock state. Most objects never have a hash code and are never locked,
	so the mark word is empty.

	Most of our garbage collectors use the mark word temporarily during GC
	to store a 'forwarding pointer.' It's not important what that is, but
	it means that objects that have a hash code or that are locked have to
	have the mark word saved during GC and then restored at the end of GC.
	We want to exercise this saving/restoring code. So a test case should
	have a large percentage (40-50%) of objects that either have a hash
	code or are locked.

Uh oh!

Conversation

fbredber commented Aug 20, 2026 • edited by openjdk Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper Bot commented Aug 20, 2026

Uh oh!

openjdk Bot commented Aug 20, 2026

Uh oh!

openjdk Bot commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk Bot commented Aug 20, 2026

Uh oh!

mlbridge Bot commented Aug 21, 2026

Webrevs

Uh oh!

pchilano left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coleenp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xmas92 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

fbredber commented Aug 20, 2026 •

edited by openjdk Bot

Loading

openjdk Bot commented Aug 20, 2026 •

edited

Loading