On Thu, 17 Oct 2024 14:28:30 GMT, Patricio Chilano Mateo
<pchilanom...@openjdk.org> wrote:
> * We copy the oops stored in the LockStack of the carrier to the
> stackChunk when freezing (and clear the LockStack). We copy the oops back to
> the LockStack of the next carrier when thawing for the first time (and clear
> them from the stackChunk). Note that we currently assume carriers don't hold
> monitors while mounting virtual threads.
This last sentence has interesting consequences for user-defined schedulers.
Would it make sense to throw an exception if a carrier thread is holding a
monitor while mounting a virtual thread? Doing that would also have the
advantage of making some kinds of deadlock impossible.
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.
src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5341:
> 5339:
> 5340: void MacroAssembler::inc_held_monitor_count() {
> 5341: Address dst = Address(rthread,
> JavaThread::held_monitor_count_offset());
Suggestion:
// Clobbers: rscratch1 and rscratch2
void MacroAssembler::inc_held_monitor_count() {
Address dst = Address(rthread, JavaThread::held_monitor_count_offset());
src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5357:
> 5355:
> 5356: void MacroAssembler::dec_held_monitor_count() {
> 5357: Address dst = Address(rthread,
> JavaThread::held_monitor_count_offset());
Suggestion:
// Clobbers: rscratch1 and rscratch2
void MacroAssembler::dec_held_monitor_count() {
Address dst = Address(rthread, JavaThread::held_monitor_count_offset());
-------------
PR Comment: https://git.openjdk.org/jdk/pull/21565#issuecomment-2429587519
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1810966647
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1810987929
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1810989022
