ConnectionBase::queueForWrite is not loom friendly

[franz1981]

Version

4.x

Context

This is a sister of netty/netty#16158 and affecting Quarkus while running with VirtualThread(s) because the mentioned synchronized block is not loom friendly and inflate the lock, causing increases in native memory usage, background deflation activity and leading to native mutex contention as well.

Replacing it with a reentrant lock could be a better option.

Steps to reproduce

No response

Do you have a reproducer?

No response

[franz1981]

To be fair, the inflation shouldn't happen just once in the lifetime of the monitor to be inflated, but still, this is not good.
Ideally we should just avoid to make it happen, since it shuldn't.

[franz1981]

This is what I can see in Quarkus:

Profiled with async-profiler -e lock on JDK 25, Quarkus 3.34.2 (Vert.x 4.5.26), 4 CPUs, 100 connections:

• interruptLock (EPollSelectorImpl.clearInterrupt): 1,062 ms wait on event loops in 10s
• Http1xServerConnection (ConnectionBase.queueForWrite lambda): 586 ms wait on event loops in 10s

The connection monitor is acquired by VertxOutputStream.write() (source) before calling response.write(), and held across the entire wakeup chain including syscalls:

VT carrier:                              Event loop:
─────────────────────────────────────    ─────────────────────────────────────
1. synchronized(connection)              (sleeping in epoll_wait)
2.   awaitWriteable()
3.   response.write(buf)
4.     → writeToChannel()
5.       → synchronized(this) [reentrant]
6.         → queueForWrite()
7.           → writeInProgress++
8.           → execute(lambda)
9.           → wakeup()
10.            → synchronized(interruptLock)
11.            → eventfd.set()  ──────→  epoll_wait returns
12.          → monitorexit(interruptLock) processEvents()
                                         clearInterrupt()
                                           → synchronized(interruptLock) ← MAY CONTEND
                                           → eventfd.reset()
                                         runAllTasks()
                                           → queueForWrite lambda
13.      → monitorexit(this) [reentrant]     → synchronized(connection) ← BLOCKS HERE
14. monitorexit(connection)  ──────────→     (unblocked, decrements writeInProgress)

eventfd.set() (step 11) wakes the event loop immediately, but the VT carrier still holds the connection monitor — it needs to complete monitorexit(interruptLock) (futex_wake syscall on the inflated monitor) before reaching step 14.
The event loop is fast enough to wake, process the interrupt, and reach the connection lock before the carrier releases it.

I think it is the same pattern observed by @barreiro some time ago which led to replacing the synchronized block (and getting it reverted).
A simpler fix retaining the lock could be to use a lazy submission inside the syncronized block and a separate wakeup outside of it, to narrow the window where the 2 threads (VT and EventLoop one) could compete.
i.e. using https://github.com/netty/netty/blob/4.1/common/src/main/java/io/netty/util/concurrent/SingleThreadEventExecutor.java#L821

But @cescoffier it means that Quarkus, which is synchronizing on the connection itself for writing, need to call itself the wakeup outside it.

[vietj]

perhaps from worker thread (loom) we should do differently and just do a simple schedule without trying to order message ? that way this flow would be used from non vertx thread only

[franz1981]

@vietj see quarkusio/quarkus#53646 which shows some number too 🙏
This will give an idea of the problem we could have using NIO event loops (platform threads) and Loom, with Vertx 5 (which doesn't have the connection lock).

[vietj]

@franz1981 which worker thread from Quarkus is creating this issue ?

[franz1981]

In this case I have used a virtual thread test and workers because is less limited by I/O performance

[vietj]

feel free to contribute a patch @franz1981

[franz1981]

@vietj it is the revert from @barriero actually: that patch was fine, it was Quarkus usage of the compressor which wasn't...
IF you check the Quarkus changes I made you can tell me...I'm not so familiar to Vertx to make sure I'm doing it right 🙏

[franz1981]

I think we are ready to reintroduce Luis changes!