You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/03/14 12:00:58 UTC
[GitHub] [rocketmq] areyouok commented on a change in pull request #3965: [ISSUE #3968]RemotingCommand `decodeCommandCustomHeader` optimized.
areyouok commented on a change in pull request #3965:
URL: https://github.com/apache/rocketmq/pull/3965#discussion_r825862171
##########
File path: remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java
##########
@@ -232,13 +232,11 @@ public void writeCustomHeader(CommandCustomHeader customHeader) {
}
public CommandCustomHeader decodeCommandCustomHeader(
- Class<? extends CommandCustomHeader> classHeader) throws RemotingCommandException {
+ Class<? extends CommandCustomHeader> classHeader) throws RemotingCommandException {
Review comment:
please revert irrelevant modifications.
##########
File path: remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java
##########
@@ -529,8 +518,8 @@ public void addExtField(String key, String value) {
@Override
public String toString() {
return "RemotingCommand [code=" + code + ", language=" + language + ", version=" + version + ", opaque=" + opaque + ", flag(B)="
- + Integer.toBinaryString(flag) + ", remark=" + remark + ", extFields=" + extFields + ", serializeTypeCurrentRPC="
- + serializeTypeCurrentRPC + "]";
+ + Integer.toBinaryString(flag) + ", remark=" + remark + ", extFields=" + extFields + ", serializeTypeCurrentRPC="
Review comment:
please revert irrelevant modifications.
##########
File path: jmh/pom.xml
##########
@@ -0,0 +1,59 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review comment:
do we need to add this module?
##########
File path: remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java
##########
@@ -292,15 +289,20 @@ public CommandCustomHeader decodeCommandCustomHeader(
}
private Field[] getClazzFields(Class<? extends CommandCustomHeader> classHeader) {
- Field[] field = CLASS_HASH_MAP.get(classHeader);
+ Field[] fields = CLASS_HASH_MAP.get(classHeader);
- if (field == null) {
- field = classHeader.getDeclaredFields();
+ if (fields == null) {
+ fields = classHeader.getDeclaredFields();
+
+ fields = Arrays.stream(fields)
+ .peek(f -> f.setAccessible(true))
Review comment:
remove this line.
##########
File path: remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java
##########
@@ -232,13 +232,11 @@ public void writeCustomHeader(CommandCustomHeader customHeader) {
}
public CommandCustomHeader decodeCommandCustomHeader(
- Class<? extends CommandCustomHeader> classHeader) throws RemotingCommandException {
+ Class<? extends CommandCustomHeader> classHeader) throws RemotingCommandException {
CommandCustomHeader objectHeader;
try {
objectHeader = classHeader.newInstance();
- } catch (InstantiationException e) {
- return null;
- } catch (IllegalAccessException e) {
+ } catch (InstantiationException | IllegalAccessException e) {
Review comment:
since 4.9.3 we should keep compatible under java 1.6, so you should revert this change.
##########
File path: remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java
##########
@@ -232,13 +232,11 @@ public void writeCustomHeader(CommandCustomHeader customHeader) {
}
public CommandCustomHeader decodeCommandCustomHeader(
- Class<? extends CommandCustomHeader> classHeader) throws RemotingCommandException {
+ Class<? extends CommandCustomHeader> classHeader) throws RemotingCommandException {
CommandCustomHeader objectHeader;
try {
objectHeader = classHeader.newInstance();
- } catch (InstantiationException e) {
- return null;
- } catch (IllegalAccessException e) {
+ } catch (InstantiationException | IllegalAccessException e) {
Review comment:
since 4.9.3 we should keep compatible under java 1.6, so you should revert this change.
##########
File path: remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java
##########
@@ -292,15 +289,20 @@ public CommandCustomHeader decodeCommandCustomHeader(
}
private Field[] getClazzFields(Class<? extends CommandCustomHeader> classHeader) {
- Field[] field = CLASS_HASH_MAP.get(classHeader);
+ Field[] fields = CLASS_HASH_MAP.get(classHeader);
- if (field == null) {
- field = classHeader.getDeclaredFields();
+ if (fields == null) {
+ fields = classHeader.getDeclaredFields();
+
+ fields = Arrays.stream(fields)
Review comment:
1.6 incompatible
##########
File path: remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java
##########
@@ -529,8 +518,8 @@ public void addExtField(String key, String value) {
@Override
public String toString() {
return "RemotingCommand [code=" + code + ", language=" + language + ", version=" + version + ", opaque=" + opaque + ", flag(B)="
- + Integer.toBinaryString(flag) + ", remark=" + remark + ", extFields=" + extFields + ", serializeTypeCurrentRPC="
- + serializeTypeCurrentRPC + "]";
+ + Integer.toBinaryString(flag) + ", remark=" + remark + ", extFields=" + extFields + ", serializeTypeCurrentRPC="
Review comment:
please revert irrelevant modifications.
##########
File path: remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java
##########
@@ -292,15 +289,20 @@ public CommandCustomHeader decodeCommandCustomHeader(
}
private Field[] getClazzFields(Class<? extends CommandCustomHeader> classHeader) {
- Field[] field = CLASS_HASH_MAP.get(classHeader);
+ Field[] fields = CLASS_HASH_MAP.get(classHeader);
- if (field == null) {
- field = classHeader.getDeclaredFields();
+ if (fields == null) {
+ fields = classHeader.getDeclaredFields();
+
+ fields = Arrays.stream(fields)
Review comment:
1.6 incompatible
##########
File path: remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java
##########
@@ -232,13 +232,11 @@ public void writeCustomHeader(CommandCustomHeader customHeader) {
}
public CommandCustomHeader decodeCommandCustomHeader(
- Class<? extends CommandCustomHeader> classHeader) throws RemotingCommandException {
+ Class<? extends CommandCustomHeader> classHeader) throws RemotingCommandException {
Review comment:
please revert irrelevant modifications.
##########
File path: remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java
##########
@@ -258,19 +256,18 @@ public CommandCustomHeader decodeCommandCustomHeader(
continue;
}
- field.setAccessible(true);
Review comment:
I think this line should not remove.
##########
File path: jmh/pom.xml
##########
@@ -0,0 +1,59 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review comment:
do we need to add this module?
##########
File path: remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java
##########
@@ -111,7 +112,7 @@ public static RemotingCommand createResponseCommand(Class<? extends CommandCusto
}
public static RemotingCommand createResponseCommand(int code, String remark,
- Class<? extends CommandCustomHeader> classHeader) {
+ Class<? extends CommandCustomHeader> classHeader) {
Review comment:
please revert irrelevant modifications.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org