LivingLikeKrillin opened a new pull request, #2612:
URL: https://github.com/apache/plc4x/pull/2612
## Summary
Replaces the per-connection NIO `Selector` in `TcpTransportInstance` with
one virtual thread per
connection doing **blocking** `SocketChannel.read()` / `write()`. Stays
fully Netty-free. Follows the
SPI3 transport layer (commit `372501287d`); @chrisdutz greenlit a redesign
of this transport and
offered bench testing on real devices — this is that redesign, TCP-only as a
first step.
Scope is confined to `plc4j/transports/tcp` (`TcpTransportInstance`). **No
public API / SPI / driver
changes.**
## Motivation (verified against current code)
1. **A vthread blocked in `Selector.select()` does not release its carrier
on Java 21.** Two reasons:
(a) pre-JEP-491 the selection path synchronizes on the selector monitor
(a monitor pin, fixed in
JDK 24); and (b) more fundamentally, `select()`'s native poll is **not**
a carrier-unmounting /
poller-managed operation, so the carrier stays blocked **even after JEP
491**. The scheduler
compensates up to `maxPoolSize` (default 256) → we pay vthread overhead
but get platform-thread
behavior plus a hidden ~256 ceiling, and this does not improve on newer
JDKs.
2. **Write backpressure is a busy-wait:** on a full send buffer, `write()`
registers `OP_WRITE`,
`wakeup()`s, then `Thread.sleep(1)` in a loop while holding `writeLock`
(never consumes the event).
3. **Selector + per-connection bookkeeping** (`interestOps`,
`reEnableReadIfNeeded`, OP_READ toggling).
On Java 21 a vthread blocked in a **blocking-mode**
`SocketChannel.read()`/`write()` parks and
**releases its carrier** (JDK parks it on the shared NIO poller) — no pin.
So one-vthread-per-connection
blocking reads is both simpler and avoids the ceiling.
## What changed
- `select()` loop → `runReadLoop()` doing blocking `read()` into the
existing `RingBuffer`.
- Removed `Selector`, `SelectionKey`, `interestOps`, `reEnableReadIfNeeded`,
and the `OP_WRITE` +
`Thread.sleep(1)` write path. Blocking `write()` now provides natural
backpressure.
- Full ring buffer → **backpressure** (`parkNanos` park-and-retry, bounded
to free space), never a
disconnect (only the codec knows frame boundaries; COTP can legitimately
drain cross-thread).
- `close()` is lock-free CAS (`AtomicBoolean`): closing the channel is what
unblocks a parked
read/write; `AsynchronousCloseException` with `open==false` is treated as
a normal shutdown.
- Listener invocation guarded (`safeRun`) so a misbehaving listener can't
silently kill the read loop.
## Zero downstream impact (audited)
Public surface, `readLock`, `RingBuffer`, and the `AsyncTransportInstance`
callback contract are
**unchanged**. `readLock` is deliberately kept because
`CotpTransportInstance` calls the read-side
methods **cross-thread** during the S7/COTP handshake — it is a load-bearing
guard, not removable.
| Consumer | Affected? |
|---|---|
| `MessageCodecBase` + driver codecs (read-thread) | No — contract/semantics
preserved |
| `ConnectionBase.startReceiving` (`registerDataListener`) | No — read
vthread invokes the listener exactly as the selector loop did |
| `CotpTransportInstance` (cross-thread read-side, concurrent) | No —
`readLock` + read-side thread-safety preserved |
| `OpcuaConnection` (`instanceof` + `getRemoteAddress`) | No — concrete
class + surface preserved |
## Evidence
- **`TcpTransportInstanceTest` (31 tests) passes unmodified** on the new
implementation → behavior-equivalent.
- **Scaling — carrier (OS) threads for 200 idle connections (measured;
`@Disabled` probe, run manually):**
| model | JDK 21 | JDK 25 |
|---|---|---|
| selector (before) | 201 | 201 |
| blocking (after) | 2–3 | 2–3 |
The selector inflates to ~1 carrier per connection on **both** JDKs, so
JEP 491 (JDK 24, removes
`synchronized` pinning) does not help here — the cost is `select()` being
a non-unmounting blocking
call, not monitor pinning. The blocking model stays flat (bounded by CPU
count, not connection
count), so the win does not erode as Java advances. (All four cells
reproduced with the same probe:
selector `CARRIER_COUNT=201` and blocking `CARRIER_COUNT=2` for 200
connections, on JDK 21 **and**
JDK 25; no pinned-thread traces for the blocking model under
`-Djdk.tracePinnedThreads=full`.)
- **End-to-end regression:** `ModbusDockerIT` (pymodbus container) — all
cases green. The
`modbus-tcp://`, `modbus-rtu:tcp://`, and `modbus-ascii:tcp://` cases
(~38) exercise the new
`TcpTransportInstance` over a real socket; the UDP and TLS cases use the
separate UDP / TLS
transports and are unaffected by this change.
## Scope / non-goals
- TCP only. UDP (shares the selector pattern) is the natural follow-up;
serial/TLS already use
different reader-thread models.
- No SPI contract reshape. The `AsyncTransportInstance` callback is
intentionally kept as a thin shim
on the blocking core; migrating to a blocking-pull contract is
**intentionally out of scope**.
## Testing notes
- `TcpTransportInstanceScalingTest` is an `@Disabled` evidence probe (opens
200 sockets + sleeps; not a
CI regression test) — run manually, ideally with
`-Djdk.tracePinnedThreads=full`.
- `mvn -pl :plc4j-transports-tcp -am verify` is green (tests + apache-rat +
jacoco).
- Bench validation on real devices welcome, as offered.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
