On Fri, 24 Oct 2025 11:51:53 GMT, Paul Hübner <[email protected]> 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

Reply via email to