[lworld] RFR: 8370509: [lworld] EnableValhalla is not needed for sync

Coleen Phillimore coleenp at openjdk.org
Fri Oct 24 17:24:32 UTC 2025


On Fri, 24 Oct 2025 11:51:53 GMT, Paul Hübner <phubner at openjdk.org> wrote:

>> I took out EnableValhalla as a conditional test in the ObjectSynchronizer code.  If the inline type bit is set in the markWord or the klass is InlineKlass, we can't synchronize on the object.
>> I added a test but it was sort of a duplicate of all runtime/valhalla/inlinetypes/MonitorEnterTest.java and duplicates all tests that do locking so I didn't add a test.
>> Testing with tier1-4, still in progress.
>
> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 7939:
> 
>> 7937:   orr(mark, mark, markWord::unlocked_value);
>> 7938:   // Mask inline_type bit such that we go to the slow path if object is an inline type
>> 7939:   andr(mark, mark, ~((int) markWord::inline_type_bit_in_place));
> 
> Since `EnableValhalla=true` by default, we were already exercising this path by default. 
> 
> Can we get into a situation where we do not use the object monitor table and the markword is inflated while we perform this check? We would get in trouble with this bit check if we can.

The code fetches the markWord, and tests for monitor value where an ObjectMonitor* may be installed.  It doesn't refetch the markWord from the object so that if a concurrent thread changes it, it'll always go slow path already.  Both platforms test the bit on a copy of the markWord after it's been fetched from the object and then checked for monitor_value. 
I've sort of always wondered why it's not checked at DiagnoseSyncOnValueBasedClasses but that has a load_klass which is more instructions.  This code can check the bit in the markWord instead.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1699#discussion_r2461359206


More information about the valhalla-dev mailing list