wang-jiahua commented on code in PR #10524:
URL: https://github.com/apache/rocketmq/pull/10524#discussion_r3427330894
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RocketMQSerializable.java:
##########
@@ -28,6 +28,74 @@
public class RocketMQSerializable {
private static final Charset CHARSET_UTF8 = StandardCharsets.UTF_8;
+ private static final String[] SINGLE_BYTE_STRINGS = new String[128];
+ static {
+ for (int i = 0; i < 128; i++) {
+ SINGLE_BYTE_STRINGS[i] = String.valueOf((char) i);
+ }
+ }
+
+ public static void writeDecimalLong(ByteBuf buf, long value) {
+ int lenIndex = buf.writerIndex();
+ buf.writeInt(0);
+ int start = buf.writerIndex();
+ if (value == 0) {
+ buf.writeByte('0');
+ } else {
+ boolean neg = value < 0;
+ if (neg) {
+ buf.writeByte('-');
+ if (value == Long.MIN_VALUE) {
+ writePositiveDigits(buf, 922337203685477580L);
+ buf.writeByte('8');
+ buf.setInt(lenIndex, buf.writerIndex() - start);
+ return;
+ }
+ value = -value;
+ }
+ writePositiveDigits(buf, value);
+ }
+ buf.setInt(lenIndex, buf.writerIndex() - start);
+ }
+
+ public static void writeDecimalInt(ByteBuf buf, int value) {
+ int lenIndex = buf.writerIndex();
+ buf.writeInt(0);
+ int start = buf.writerIndex();
+ if (value == 0) {
+ buf.writeByte('0');
+ } else {
+ boolean neg = value < 0;
+ if (neg) {
+ buf.writeByte('-');
+ if (value == Integer.MIN_VALUE) {
+ writePositiveDigits(buf, 214748364L);
+ buf.writeByte('8');
+ buf.setInt(lenIndex, buf.writerIndex() - start);
+ return;
+ }
+ value = -value;
+ }
+ writePositiveDigits(buf, value);
+ }
+ buf.setInt(lenIndex, buf.writerIndex() - start);
+ }
+
+ private static void writePositiveDigits(ByteBuf buf, long value) {
+ int digitStart = buf.writerIndex();
+ int numDigits = 0;
+ long temp = value;
+ while (temp > 0) {
+ numDigits++;
+ temp /= 10;
+ }
+ buf.writerIndex(digitStart + numDigits);
+ temp = value;
+ for (int i = numDigits - 1; i >= 0; i--) {
+ buf.setByte(digitStart + i, (byte) ('0' + (int) (temp % 10)));
+ temp /= 10;
+ }
+ }
Review Comment:
Good catch. Added `buf.ensureWritable(numDigits)` before advancing the
writer index in `writePositiveDigits`. Fixed in latest push.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RocketMQSerializable.java:
##########
@@ -28,6 +28,74 @@
public class RocketMQSerializable {
private static final Charset CHARSET_UTF8 = StandardCharsets.UTF_8;
+ private static final String[] SINGLE_BYTE_STRINGS = new String[128];
+ static {
+ for (int i = 0; i < 128; i++) {
+ SINGLE_BYTE_STRINGS[i] = String.valueOf((char) i);
+ }
+ }
+
+ public static void writeDecimalLong(ByteBuf buf, long value) {
+ int lenIndex = buf.writerIndex();
+ buf.writeInt(0);
+ int start = buf.writerIndex();
+ if (value == 0) {
+ buf.writeByte('0');
+ } else {
+ boolean neg = value < 0;
+ if (neg) {
+ buf.writeByte('-');
+ if (value == Long.MIN_VALUE) {
+ writePositiveDigits(buf, 922337203685477580L);
+ buf.writeByte('8');
+ buf.setInt(lenIndex, buf.writerIndex() - start);
+ return;
+ }
+ value = -value;
+ }
+ writePositiveDigits(buf, value);
+ }
+ buf.setInt(lenIndex, buf.writerIndex() - start);
+ }
+
+ public static void writeDecimalInt(ByteBuf buf, int value) {
+ int lenIndex = buf.writerIndex();
+ buf.writeInt(0);
+ int start = buf.writerIndex();
+ if (value == 0) {
+ buf.writeByte('0');
+ } else {
+ boolean neg = value < 0;
+ if (neg) {
+ buf.writeByte('-');
+ if (value == Integer.MIN_VALUE) {
+ writePositiveDigits(buf, 214748364L);
+ buf.writeByte('8');
+ buf.setInt(lenIndex, buf.writerIndex() - start);
+ return;
+ }
+ value = -value;
+ }
+ writePositiveDigits(buf, value);
+ }
+ buf.setInt(lenIndex, buf.writerIndex() - start);
+ }
Review Comment:
The duplication is intentional. Each method handles a different primitive
type with type-specific MIN_VALUE edge cases (`Long.MIN_VALUE` vs
`Integer.MIN_VALUE`). Consolidating would require either autoboxing or a shared
`long` implementation with an `int` wrapper that still needs its own MIN_VALUE
check. The current approach keeps each method self-contained and avoids
unnecessary type conversions.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RocketMQSerializable.java:
##########
@@ -231,7 +306,7 @@ public static RemotingCommand rocketMQProtocolDecode(final
ByteBuf headerBuffer,
public static HashMap<String, String> mapDeserialize(ByteBuf byteBuffer,
int len) throws RemotingCommandException {
- HashMap<String, String> map = new HashMap<>(128);
+ HashMap<String, String> map = new HashMap<>(24);
Review Comment:
Added a comment explaining the rationale: typical RocketMQ headers have 5-15
entries, and 24 is the next power-of-2 above 15 × 0.75 (default load factor),
which avoids resize. Fixed in latest push.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RocketMQSerializable.java:
##########
@@ -52,6 +120,13 @@ private static String readStr(ByteBuf buf, boolean
useShortLength, int limit) th
if (len > limit) {
throw new RemotingCommandException("string length exceed limit:" +
limit);
}
+ if (len == 1) {
+ byte b = buf.readByte();
+ if (b >= 0) {
+ return SINGLE_BYTE_STRINGS[b];
+ }
+ return new String(new byte[]{b}, StandardCharsets.UTF_8);
+ }
CharSequence cs = buf.readCharSequence(len, StandardCharsets.UTF_8);
return cs == null ? null : cs.toString();
Review Comment:
Good catch. Changed to use the existing `CHARSET_UTF8` constant for
consistency. Fixed in latest push.
--
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]
