RFR: 8338383: Implementation of Synchronize Virtual Threads without Pinning [v3]

Andrew Haley aph at openjdk.org
Tue Oct 22 15:53:21 UTC 2024


On Tue, 22 Oct 2024 15:37:23 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Patricio Chilano Mateo has updated the pull request incrementally with six additional commits since the last revision:
>> 
>>  - Fix comments in objectMonitor.hpp
>>  - Move frame::saved_thread_address() to platform dependent files
>>  - Fix typo in jvmtiExport.cpp
>>  - remove usage of frame::metadata_words in possibly_adjust_frame()
>>  - Fix comments in c2 locking paths
>>  - Revert and simplify changes to c1_Runtime1 on aarch64 and riscv
>
> src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 60:
> 
>> 58: 
>> 59:   assert(LockingMode != LM_LIGHTWEIGHT, "lightweight locking should use fast_lock_lightweight");
>> 60:   assert_different_registers(oop, box, tmp, disp_hdr, rscratch2);
> 
> Historically, silently using `rscratch1` and `rscratch2` in these macros has sometimes turned out to be a mistake.
> Please consider making `rscratch2` an additional argument to `fast_lock`, so that it's explicit in the caller. It won't make any difference to the generated code, but it might help readbility.

Note also that `inc_held_monitor_count` clobbers `rscratch2`.  That might be worth a comment at the call site.
I guess `inc_held_monitor_count` is so hot that we can't push and pop scratch registers, in which case it'd clobber nothing.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1810985771


More information about the nio-dev mailing list