ivanzlenko commented on code in PR #4670: URL: https://github.com/apache/ignite-3/pull/4670#discussion_r1828803056
########## modules/core/src/main/java/org/apache/ignite/internal/util/ByteUtils.java: ########## @@ -163,6 +163,26 @@ public static int bytesToIntKeepingOrder(byte[] bytes) { return bytesToInt(bytes) ^ 0x00808080; } + /** + * Constructs {@code short} from byte array in Big Endian order. + * + * @param bytes Array of bytes. + * @param off Offset in {@code bytes} array. + * @return Short value. + */ + public static short bytesToShort(byte[] bytes, int off) { + assert bytes != null; + + int bytesCnt = Math.min(Short.BYTES, bytes.length - off); Review Comment: ```suggestion int bytesCount = Math.min(Short.BYTES, bytes.length - off); ``` No need to shorten this word here. ########## modules/core/src/main/java/org/apache/ignite/internal/hlc/HybridTimestamp.java: ########## @@ -131,7 +132,18 @@ public static long hybridTimestampToLong(@Nullable HybridTimestamp timestamp) { * @param bytes Byte array representing a timestamp. */ public static HybridTimestamp fromBytes(byte[] bytes) { - return hybridTimestamp(ByteUtils.bytesToLong(bytes)); + // Reversing bytes as ByteUtils works in BE, but we store in LE (as IgniteDataOutput uses LE and we want to be consistent between Review Comment: Shouldn't it be the part of the javaDoc rather than inline comment? ########## modules/core/src/main/java/org/apache/ignite/internal/hlc/HybridTimestamp.java: ########## @@ -256,8 +268,21 @@ public HybridTimestamp roundUpToPhysicalTick() { /** * Returns a byte array representing this timestamp. */ + @SuppressWarnings("NumericCastThatLosesPrecision") public byte[] toBytes() { - return ByteUtils.longToBytes(longValue()); + long physical = getPhysical(); + int logical = getLogical(); + + byte[] bytes = new byte[Integer.BYTES + Short.BYTES + VarIntUtils.varIntLength(logical)]; + + // Reversing bytes as ByteUtils works in BE, but we store in LE (as IgniteDataOutput uses LE and we want to be consistent between Review Comment: Same about javaDoc ########## modules/core/src/main/java/org/apache/ignite/internal/util/ByteUtils.java: ########## @@ -195,11 +215,32 @@ public static byte[] intToBytesKeepingOrder(int i) { */ public static byte[] putIntToBytes(int i, byte[] bytes, int off) { assert bytes != null; - assert bytes.length >= off + Integer.BYTES; + assert off + Integer.BYTES <= bytes.length; for (int k = Integer.BYTES - 1; k >= 0; k--) { bytes[off + k] = (byte) i; - i >>>= 8; + i >>>= Byte.SIZE; + } + + return bytes; + } + + /** + * Converts a primitive {@code short} value to a byte array in Big Endian order and stores it in the specified byte array. + * + * @param s Unsigned int value. + * @param bytes Bytes array to write result to. + * @param off Offset in the target array to write result to. + * @return The array. + */ + public static byte[] putShortToBytes(short s, byte[] bytes, int off) { Review Comment: Worth extracting method with similar code. In that case we can create LongToBytes, whateEverToBytes with just a one line. ########## modules/core/src/main/java/org/apache/ignite/internal/util/ByteUtils.java: ########## @@ -163,6 +163,26 @@ public static int bytesToIntKeepingOrder(byte[] bytes) { return bytesToInt(bytes) ^ 0x00808080; } + /** + * Constructs {@code short} from byte array in Big Endian order. + * + * @param bytes Array of bytes. + * @param off Offset in {@code bytes} array. + * @return Short value. + */ + public static short bytesToShort(byte[] bytes, int off) { + assert bytes != null; + + int bytesCnt = Math.min(Short.BYTES, bytes.length - off); + + int res = 0; + + for (int i = 0; i < bytesCnt; i++) { Review Comment: ```suggestion for (int i = 0; i < bytesCount; i++) { ``` -- 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: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org