LivingLikeKrillin opened a new pull request, #2650:
URL: https://github.com/apache/plc4x/pull/2650
### What & why
The shared byte buffers in `plc4j/spi/buffers` sit on every driver's hottest
path — every
primitive field of every message is read/written through them (~70+ files
across `plc4j` — drivers,
spi, transports, test-utils — construct these byte buffers). Profiling a 32×
`uint16` batch (a
typical Modbus/S7-style read) with JMH showed two avoidable costs per field:
1. the option-context stack was a `java.util.Stack` (a synchronized
`Vector`), so every
`getContext()` did a monitor enter/exit that guarded nothing; and
2. every whole-byte, byte-aligned integer field still went through the
generic bit-by-bit path,
allocating an intermediate `byte[]` and a fresh `ByteOrder` object per
field.
This change removes both without altering any wire behaviour.
### Changes
- **`AbstractBuffer`** — use `ArrayDeque` for the option-context stack
instead of `java.util.Stack`.
A buffer is a single-threaded, per-message scratch object
(`positionInBits` and the backing array
are already unsynchronized), so `Stack`'s synchronization protected
nothing here.
- **`ByteOrderBigEndian`** — expose a shared stateless `INSTANCE` and return
it from `getByteOrder()`
instead of allocating one per field (`process()` is the identity, so the
instance is reusable).
- **`Read/WriteBufferByteBased`** (with shared guards + a `signExtend`
helper in
`AbstractBufferByteBased`) — add a zero-allocation fast path for
whole-byte, byte-aligned,
big-endian, plain-binary integer fields (unsigned and signed
short/int/long) that reads/writes
straight from the backing array. It is gated on the concrete
`EncodingUnsignedBinary` /
`EncodingTwosComplement` (not the broader `EncodingDefault`), so BCD /
float / etc. fall through
to the existing slow path unchanged.
### Benchmarks
JMH, JDK 21, 32× `uint16` batch, `-prof gc`:
| metric | improvement |
|---|---|
| read throughput | **~6.4× faster** |
| write throughput | **~5.6× faster** |
| allocation | **−38%** (~1944 → ~1200 B/msg) |
### Thread-safety
No regression. Buffers are per-message throwaway objects — no pooling, no
static/`ThreadLocal`
sharing — and `positionInBits` / the backing array were already
unsynchronized, so each buffer is
confined to a single thread for the duration of one message's
parse/serialize. The `Stack`'s
synchronization was incidental, not an anti-interleaving guarantee:
concurrent read/write requests
each get their own buffer. `ArrayDeque` is a drop-in — the code uses the
context field only as a
LIFO stack via `push`/`pop`/`peek`/`isEmpty`/`size`, all of which behave
identically, and never
iterates it or reads it by index.
### Binary compatibility
Changing `AbstractBuffer`'s `context` field type from `Stack` to `Deque` is
source-compatible but
binary-incompatible for any subclass that reads the `protected context`
field directly. Within the
repo exactly one class does so — `WriteBufferXmlBased` (`context.isEmpty()`
/ `context.size()`) —
and it compiles and recompiles cleanly against `Deque`; the other buffer
subclasses
(`ReadBufferXmlBased`, the ascii-box and byte buffers) never touch the
field. All three buffer
modules (`byte`, `xml`, `ascii-box`) recompile in a normal/CI build; only a
stale incremental build
(Maven not recompiling the `xml` module after `api` changed), or an
out-of-repo precompiled subclass
that reads `context`, would hit a `NoSuchFieldError`. Flagging it here for
downstream awareness.
### Testing
- Buffer unit tests green (Read 134, Write 115), including new regression
tests: the unsigned
byte-aligned fast path returns/emits the same value as the slow path (read
& write),
BCD-must-not-take-the-binary-fast-path (read & write), and signed
sign-extension / two's-complement
(read & write) — so both fast-path branches and the slow-path fallback are
positively covered.
- Modbus driver module test suite green (213 tests).
--
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]
