[ROCKETMQ-259]Fix too many reflection calls when decode remoting comm…

…and header
lmxy1990 · Aug 11, 2017 · f613c3b · f613c3b
1 parent f091203
commit f613c3b
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 12 deletions.
diff --git a/remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java b/remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java
@@ -43,7 +43,7 @@ public class RemotingCommand {
     private static final Map<Class, String> CANONICAL_NAME_CACHE = new HashMap<Class, String>();
     // 1, Oneway
     // 1, RESPONSE_COMMAND
-    private static final Map<Field, Annotation> NOT_NULL_ANNOTATION_CACHE = new HashMap<Field, Annotation>();
+    private static final Map<Field, Boolean> NULLABLE_FIELD_CACHE = new HashMap<Field, Boolean>();
     private static final String STRING_CANONICAL_NAME = String.class.getCanonicalName();
     private static final String DOUBLE_CANONICAL_NAME_1 = Double.class.getCanonicalName();
     private static final String DOUBLE_CANONICAL_NAME_2 = double.class.getCanonicalName();
@@ -252,11 +252,9 @@ public CommandCustomHeader decodeCommandCustomHeader(
                         try {
                             String value = this.extFields.get(fieldName);
                             if (null == value) {
-                                Annotation annotation = getNotNullAnnotation(field);
-                                if (annotation != null) {
+                                if (!isFieldNullable(field)) {
                                     throw new RemotingCommandException("the custom field <" + fieldName + "> is null");
                                 }
-
                                 continue;
                             }
 
@@ -305,16 +303,14 @@ private Field[] getClazzFields(Class<? extends CommandCustomHeader> classHeader)
         return field;
     }
 
-    private Annotation getNotNullAnnotation(Field field) {
-        Annotation annotation = NOT_NULL_ANNOTATION_CACHE.get(field);
-
-        if (annotation == null) {
-            annotation = field.getAnnotation(CFNotNull.class);
-            synchronized (NOT_NULL_ANNOTATION_CACHE) {
-                NOT_NULL_ANNOTATION_CACHE.put(field, annotation);
+    private boolean isFieldNullable(Field field) {
+        if (!NULLABLE_FIELD_CACHE.containsKey(field)) {
+            Annotation annotation = field.getAnnotation(CFNotNull.class);
+            synchronized (NULLABLE_FIELD_CACHE) {
+                NULLABLE_FIELD_CACHE.put(field, annotation == null);
             }
         }
-        return annotation;
+        return NULLABLE_FIELD_CACHE.get(field);
     }
 
     private String getCanonicalName(Class clazz) {

diff --git a/remoting/src/test/java/org/apache/rocketmq/remoting/protocol/RemotingCommandTest.java b/remoting/src/test/java/org/apache/rocketmq/remoting/protocol/RemotingCommandTest.java
@@ -16,8 +16,11 @@
  */
 package org.apache.rocketmq.remoting.protocol;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
 import java.nio.ByteBuffer;
 import org.apache.rocketmq.remoting.CommandCustomHeader;
+import org.apache.rocketmq.remoting.annotation.CFNotNull;
 import org.apache.rocketmq.remoting.exception.RemotingCommandException;
 import org.junit.Test;
 
@@ -179,6 +182,32 @@ public void testEncodeAndDecode_FilledBodyWithExtFields() throws RemotingCommand
         assertThat(((ExtFieldsHeader) decodedHeader).isBooleanValue()).isEqualTo(true);
         assertThat(((ExtFieldsHeader) decodedHeader).getDoubleValue()).isBetween(0.617, 0.619);
     }
+
+    @Test
+    public void testNotNullField() throws Exception {
+        RemotingCommand remotingCommand = new RemotingCommand();
+        Method method = RemotingCommand.class.getDeclaredMethod("isFieldNullable", Field.class);
+        method.setAccessible(true);
+
+        Field nullString = FieldTestClass.class.getDeclaredField("nullString");
+        assertThat(method.invoke(remotingCommand, nullString)).isEqualTo(false);
+
+        Field nullableString = FieldTestClass.class.getDeclaredField("nullable");
+        assertThat(method.invoke(remotingCommand, nullableString)).isEqualTo(true);
+
+        Field value = FieldTestClass.class.getDeclaredField("value");
+        assertThat(method.invoke(remotingCommand, value)).isEqualTo(false);
+    }
+}
+
+class FieldTestClass {
+    @CFNotNull
+    String nullString = null;
+
+    String nullable = null;
+
+    @CFNotNull
+    String value = "NotNull";
 }
 
 class SampleCommandCustomHeader implements CommandCustomHeader {