LivingLikeKrillin commented on code in PR #2650:
URL: https://github.com/apache/plc4x/pull/2650#discussion_r3620641786
##########
plc4j/spi/buffers/byte/src/main/java/org/apache/plc4x/java/spi/buffers/bytebased/AbstractBufferByteBased.java:
##########
@@ -65,7 +67,40 @@ protected ByteOrder getByteOrder(WithOption... options) {
return byteOrder.get();
}
}
- return new ByteOrderBigEndian();
+ return ByteOrderBigEndian.INSTANCE;
+ }
+
+ // ---- Byte-aligned integer fast-path helpers (shared by Read/Write byte
buffers) ----
+ // Eligible only when there are no per-field option overrides, the
position is byte-aligned, a
+ // whole number of bytes is requested, byte order is big-endian, and the
resolved integer encoding
+ // is plain binary (unsigned) / two's-complement (signed). Gated on the
concrete encoding class,
+ // NOT the broader EncodingDefault, so BCD/float/etc. correctly fall
through to the slow path.
+
+ protected boolean isFastUnsignedBinaryBE(int numBits, WithOption[]
options) {
+ // isAligned() tests the ABSOLUTE bit index (startBit +
positionInBits) — the same predicate the
+ // readBits/writeBits whole-byte fast paths use — so a
non-byte-aligned sub-buffer correctly
+ // falls through to the generic path (readAlignedBytesBE indexes by
(startBit+positionInBits)/8).
+ if (options.length != 0 || !isAligned() || (numBits & 7) != 0) {
+ return false;
+ }
+ Optional<Encoding> enc = getUnsignedIntegerEncoding();
+ return enc.isPresent() && enc.get() instanceof EncodingUnsignedBinary
+ && getByteOrder() instanceof ByteOrderBigEndian;
+ }
Review Comment:
Fixed in 7c3b965c72. Each of the twelve read/write methods now resolves the
encoding and byte order exactly once at the top and reuses the resolved values
for both the fast-path eligibility check and the slow path — the BE-specific
guards collapsed into a single structural `isByteAlignedWholeBytes` check
combined with checks on the already-resolved values, so a non-eligible aligned
field no longer pays a second context/manager lookup. (EncodingRaw paths now
resolve a byte order they don't use — a negligible constant — in exchange for
exactly-once resolution everywhere else.)
Follow-up hardening in 6a8d06af75: the gates compare exact classes
(`getClass() == ...`) rather than `instanceof`, so a codec subclass registered
through the managers keeps its overridden slow-path behaviour, and `writeBit`
now honours `startBit` like `readBit` (pre-existing asymmetry), with a
regression test.
##########
plc4j/spi/buffers/byte/src/main/java/org/apache/plc4x/java/spi/buffers/bytebased/AbstractBufferByteBased.java:
##########
@@ -65,7 +67,40 @@ protected ByteOrder getByteOrder(WithOption... options) {
return byteOrder.get();
}
}
- return new ByteOrderBigEndian();
+ return ByteOrderBigEndian.INSTANCE;
+ }
+
+ // ---- Byte-aligned integer fast-path helpers (shared by Read/Write byte
buffers) ----
+ // Eligible only when there are no per-field option overrides, the
position is byte-aligned, a
+ // whole number of bytes is requested, byte order is big-endian, and the
resolved integer encoding
+ // is plain binary (unsigned) / two's-complement (signed). Gated on the
concrete encoding class,
+ // NOT the broader EncodingDefault, so BCD/float/etc. correctly fall
through to the slow path.
+
+ protected boolean isFastUnsignedBinaryBE(int numBits, WithOption[]
options) {
+ // isAligned() tests the ABSOLUTE bit index (startBit +
positionInBits) — the same predicate the
+ // readBits/writeBits whole-byte fast paths use — so a
non-byte-aligned sub-buffer correctly
+ // falls through to the generic path (readAlignedBytesBE indexes by
(startBit+positionInBits)/8).
+ if (options.length != 0 || !isAligned() || (numBits & 7) != 0) {
+ return false;
+ }
+ Optional<Encoding> enc = getUnsignedIntegerEncoding();
+ return enc.isPresent() && enc.get() instanceof EncodingUnsignedBinary
+ && getByteOrder() instanceof ByteOrderBigEndian;
+ }
+
+ protected boolean isFastSignedTwosComplementBE(int numBits, WithOption[]
options) {
+ if (options.length != 0 || !isAligned() || (numBits & 7) != 0) {
+ return false;
+ }
+ Optional<Encoding> enc = getSignedIntegerEncoding();
+ return enc.isPresent() && enc.get() instanceof EncodingTwosComplement
+ && getByteOrder() instanceof ByteOrderBigEndian;
+ }
Review Comment:
Same fix as above (7c3b965c72): `isFastSignedTwosComplementBE` is gone — the
signed methods resolve encoding/byte order once and gate on the structural
`isByteAlignedWholeBytes` plus exact-class checks on the resolved values,
sharing them with the slow path.
--
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]
