LivingLikeKrillin commented on code in PR #2650:
URL: https://github.com/apache/plc4x/pull/2650#discussion_r3616142075
##########
plc4j/spi/buffers/byte/src/main/java/org/apache/plc4x/java/spi/buffers/bytebased/AbstractBufferByteBased.java:
##########
@@ -65,7 +67,37 @@ 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) {
+ if (options.length != 0 || (positionInBits & 7) != 0 || (numBits & 7)
!= 0) {
+ return false;
+ }
+ Optional<Encoding> enc = getUnsignedIntegerEncoding();
+ return enc.isPresent() && enc.get() instanceof EncodingUnsignedBinary
+ && getByteOrder() == ByteOrderBigEndian.INSTANCE;
+ }
Review Comment:
Fixed in 0c1face8cc. The root cause was broader than the fast path:
`isAligned()` — which the `readBits`/`writeBits` whole-byte `arraycopy` fast
paths also use — tested only `positionInBits`, ignoring `startBit`. So a
non-byte-aligned sub-buffer (`startBit % 8 != 0` while its own `positionInBits
== 0`) already read from the wrong byte on the generic path too, not just the
new integer fast path. `isAligned()` now tests the absolute bit index
`((startBit + positionInBits) % 8) == 0`, and both integer fast-path guards
gate on `!isAligned()`, so the fast path and the generic path agree and handle
non-byte-aligned sub-buffers correctly. Added a regression test
(`byteAlignedFastPathRespectsNonByteAlignedSubBufferStartBit`).
On the byte order: the guards now use `getByteOrder() instanceof
ByteOrderBigEndian` instead of reference-equality with the singleton, so an
explicitly big-endian-configured buffer (the ServiceLoader-created instance) is
also eligible for the fast path.
##########
plc4j/spi/buffers/byte/src/main/java/org/apache/plc4x/java/spi/buffers/bytebased/AbstractBufferByteBased.java:
##########
@@ -65,7 +67,37 @@ 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) {
+ if (options.length != 0 || (positionInBits & 7) != 0 || (numBits & 7)
!= 0) {
+ return false;
+ }
+ Optional<Encoding> enc = getUnsignedIntegerEncoding();
+ return enc.isPresent() && enc.get() instanceof EncodingUnsignedBinary
+ && getByteOrder() == ByteOrderBigEndian.INSTANCE;
+ }
+
+ protected boolean isFastSignedTwosComplementBE(int numBits, WithOption[]
options) {
+ if (options.length != 0 || (positionInBits & 7) != 0 || (numBits & 7)
!= 0) {
+ return false;
+ }
+ Optional<Encoding> enc = getSignedIntegerEncoding();
+ return enc.isPresent() && enc.get() instanceof EncodingTwosComplement
+ && getByteOrder() == ByteOrderBigEndian.INSTANCE;
+ }
Review Comment:
Same fix as the unsigned guard above (0c1face8cc): both
`isFastUnsignedBinaryBE` and `isFastSignedTwosComplementBE` now gate on
`!isAligned()` — where `isAligned()` tests the absolute bit index `((startBit +
positionInBits) % 8) == 0` — and use `getByteOrder() instanceof
ByteOrderBigEndian` rather than reference-equality with the singleton.
--
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]
