BitoAgent commented on code in PR #13786:
URL: https://github.com/apache/dubbo/pull/13786#discussion_r1568049556


##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcCompositeCodec.java:
##########
@@ -16,123 +16,103 @@
  */
 package org.apache.dubbo.rpc.protocol.tri.h12.grpc;
 
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.config.ConfigurationUtils;
+import org.apache.dubbo.common.io.StreamUtils;
+import org.apache.dubbo.common.utils.ArrayUtils;
 import org.apache.dubbo.remoting.http12.exception.DecodeException;
 import org.apache.dubbo.remoting.http12.exception.EncodeException;
 import org.apache.dubbo.remoting.http12.message.HttpMessageCodec;
 import org.apache.dubbo.remoting.http12.message.MediaType;
+import org.apache.dubbo.rpc.model.FrameworkModel;
+import org.apache.dubbo.rpc.model.MethodDescriptor;
+import org.apache.dubbo.rpc.model.PackableMethod;
+import org.apache.dubbo.rpc.model.PackableMethodFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.Charset;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
-import com.google.protobuf.Message;
-
-import static 
org.apache.dubbo.common.constants.CommonConstants.PROTOBUF_MESSAGE_CLASS_NAME;
+import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_KEY;
+import static 
org.apache.dubbo.common.constants.CommonConstants.DUBBO_PACKABLE_METHOD_FACTORY;
 
 public class GrpcCompositeCodec implements HttpMessageCodec {
 
-    private final ProtobufHttpMessageCodec protobufHttpMessageCodec;
+    private static final String PACKABLE_METHOD_CACHE = 
"PACKABLE_METHOD_CACHE";
 
-    private final WrapperHttpMessageCodec wrapperHttpMessageCodec;
+    private final URL url;
 
-    public GrpcCompositeCodec(
-            ProtobufHttpMessageCodec protobufHttpMessageCodec, 
WrapperHttpMessageCodec wrapperHttpMessageCodec) {
-        this.protobufHttpMessageCodec = protobufHttpMessageCodec;
-        this.wrapperHttpMessageCodec = wrapperHttpMessageCodec;
-    }
+    private final FrameworkModel frameworkModel;
+
+    private final String mediaType;
 
-    public void setEncodeTypes(Class<?>[] encodeTypes) {
-        this.wrapperHttpMessageCodec.setEncodeTypes(encodeTypes);
+    private PackableMethod packableMethod;
+
+    public GrpcCompositeCodec(URL url, FrameworkModel frameworkModel, String 
mediaType) {
+        this.url = url;
+        this.frameworkModel = frameworkModel;
+        this.mediaType = mediaType;
     }
 
-    public void setDecodeTypes(Class<?>[] decodeTypes) {
-        this.wrapperHttpMessageCodec.setDecodeTypes(decodeTypes);
+    public void loadPackableMethod(MethodDescriptor methodDescriptor) {
+        if (methodDescriptor instanceof PackableMethod) {
+            packableMethod = (PackableMethod) methodDescriptor;
+            return;
+        }
+        Map<MethodDescriptor, PackableMethod> cacheMap = 
(Map<MethodDescriptor, PackableMethod>) url.getServiceModel()
+                .getServiceMetadata()
+                .getAttributeMap()
+                .computeIfAbsent(PACKABLE_METHOD_CACHE, k -> new 
ConcurrentHashMap<>());
+        packableMethod = cacheMap.computeIfAbsent(methodDescriptor, md -> 
frameworkModel
+                .getExtensionLoader(PackableMethodFactory.class)
+                
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
+                        .getString(DUBBO_PACKABLE_METHOD_FACTORY, DEFAULT_KEY))
+                .create(methodDescriptor, url, mediaType));

Review Comment:
    **Issue**: The method 'loadPackableMethod' introduces a casting operation 
without an explicit check, which could lead to a ClassCastException if the 
'url.getServiceModel().getServiceMetadata().getAttributeMap()' does not return 
a Map<MethodDescriptor, PackableMethod> as expected. <br> **Fix**: Add a check 
to ensure the object returned from 
'url.getServiceModel().getServiceMetadata().getAttributeMap()' is indeed an 
instance of Map<MethodDescriptor, PackableMethod> before casting. If not, 
handle the case appropriately to prevent a ClassCastException. <br> **Code 
Suggestion**: 
    ```
    + if (url.getServiceModel().getServiceMetadata().getAttributeMap() 
instanceof Map) {
    +     Map<MethodDescriptor, PackableMethod> cacheMap = 
(Map<MethodDescriptor, PackableMethod>) url.getServiceModel()
    +             .getServiceMetadata()
    +             .getAttributeMap()
    +             .computeIfAbsent(PACKABLE_METHOD_CACHE, k -> new 
ConcurrentHashMap<>());
    +     packableMethod = cacheMap.computeIfAbsent(methodDescriptor, md -> 
frameworkModel
    +             .getExtensionLoader(PackableMethodFactory.class)
    +             
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
    +                     .getString(DUBBO_PACKABLE_METHOD_FACTORY, 
DEFAULT_KEY))
    +             .create(methodDescriptor, url, mediaType));
    + } else {
    +     // Handle the case where the attribute map is not as expected
    + }
    ```
   
   



##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcCompositeCodecFactory.java:
##########
@@ -22,19 +22,14 @@
 import org.apache.dubbo.remoting.http12.message.HttpMessageDecoderFactory;
 import org.apache.dubbo.remoting.http12.message.HttpMessageEncoderFactory;
 import org.apache.dubbo.remoting.http12.message.MediaType;
-import org.apache.dubbo.remoting.utils.UrlUtils;
 import org.apache.dubbo.rpc.model.FrameworkModel;
 
 @Activate
 public class GrpcCompositeCodecFactory implements HttpMessageEncoderFactory, 
HttpMessageDecoderFactory {
 
     @Override
     public HttpMessageCodec createCodec(URL url, FrameworkModel 
frameworkModel, String mediaType) {
-        String serializeName = UrlUtils.serializationOrDefault(url);
-        WrapperHttpMessageCodec wrapperHttpMessageCodec = new 
WrapperHttpMessageCodec(url, frameworkModel);
-        wrapperHttpMessageCodec.setSerializeType(serializeName);
-        ProtobufHttpMessageCodec protobufHttpMessageCodec = new 
ProtobufHttpMessageCodec();
-        return new GrpcCompositeCodec(protobufHttpMessageCodec, 
wrapperHttpMessageCodec);
+        return new GrpcCompositeCodec(url, frameworkModel, mediaType);

Review Comment:
    **Optimization Issue**: The creation of GrpcCompositeCodec now directly 
uses URL, FrameworkModel, and mediaType, potentially bypassing detailed 
configuration or customization that was previously possible with individual 
codec setup. <br> **Fix**: Ensure that the GrpcCompositeCodec constructor 
properly handles all necessary configurations and customizations that might 
have been done in the individual codecs. Consider adding methods to 
GrpcCompositeCodec to allow for detailed configuration. <br> **Code 
Suggestion**: 
    ```
    -        return new GrpcCompositeCodec(url, frameworkModel, mediaType);
    +        GrpcCompositeCodec codec = new GrpcCompositeCodec(url, 
frameworkModel, mediaType);
    +        // Add necessary configuration methods on codec if needed
    +        return codec;
    ```
   
   



##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcCompositeCodec.java:
##########
@@ -16,123 +16,103 @@
  */
 package org.apache.dubbo.rpc.protocol.tri.h12.grpc;
 
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.config.ConfigurationUtils;
+import org.apache.dubbo.common.io.StreamUtils;
+import org.apache.dubbo.common.utils.ArrayUtils;
 import org.apache.dubbo.remoting.http12.exception.DecodeException;
 import org.apache.dubbo.remoting.http12.exception.EncodeException;
 import org.apache.dubbo.remoting.http12.message.HttpMessageCodec;
 import org.apache.dubbo.remoting.http12.message.MediaType;
+import org.apache.dubbo.rpc.model.FrameworkModel;
+import org.apache.dubbo.rpc.model.MethodDescriptor;
+import org.apache.dubbo.rpc.model.PackableMethod;
+import org.apache.dubbo.rpc.model.PackableMethodFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.Charset;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
-import com.google.protobuf.Message;
-
-import static 
org.apache.dubbo.common.constants.CommonConstants.PROTOBUF_MESSAGE_CLASS_NAME;
+import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_KEY;
+import static 
org.apache.dubbo.common.constants.CommonConstants.DUBBO_PACKABLE_METHOD_FACTORY;
 
 public class GrpcCompositeCodec implements HttpMessageCodec {
 
-    private final ProtobufHttpMessageCodec protobufHttpMessageCodec;
+    private static final String PACKABLE_METHOD_CACHE = 
"PACKABLE_METHOD_CACHE";
 
-    private final WrapperHttpMessageCodec wrapperHttpMessageCodec;
+    private final URL url;
 
-    public GrpcCompositeCodec(
-            ProtobufHttpMessageCodec protobufHttpMessageCodec, 
WrapperHttpMessageCodec wrapperHttpMessageCodec) {
-        this.protobufHttpMessageCodec = protobufHttpMessageCodec;
-        this.wrapperHttpMessageCodec = wrapperHttpMessageCodec;
-    }
+    private final FrameworkModel frameworkModel;
+
+    private final String mediaType;
 
-    public void setEncodeTypes(Class<?>[] encodeTypes) {
-        this.wrapperHttpMessageCodec.setEncodeTypes(encodeTypes);
+    private PackableMethod packableMethod;
+
+    public GrpcCompositeCodec(URL url, FrameworkModel frameworkModel, String 
mediaType) {
+        this.url = url;
+        this.frameworkModel = frameworkModel;
+        this.mediaType = mediaType;
     }
 
-    public void setDecodeTypes(Class<?>[] decodeTypes) {
-        this.wrapperHttpMessageCodec.setDecodeTypes(decodeTypes);
+    public void loadPackableMethod(MethodDescriptor methodDescriptor) {
+        if (methodDescriptor instanceof PackableMethod) {
+            packableMethod = (PackableMethod) methodDescriptor;
+            return;
+        }
+        Map<MethodDescriptor, PackableMethod> cacheMap = 
(Map<MethodDescriptor, PackableMethod>) url.getServiceModel()
+                .getServiceMetadata()
+                .getAttributeMap()
+                .computeIfAbsent(PACKABLE_METHOD_CACHE, k -> new 
ConcurrentHashMap<>());
+        packableMethod = cacheMap.computeIfAbsent(methodDescriptor, md -> 
frameworkModel
+                .getExtensionLoader(PackableMethodFactory.class)
+                
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
+                        .getString(DUBBO_PACKABLE_METHOD_FACTORY, DEFAULT_KEY))
+                .create(methodDescriptor, url, mediaType));
     }
 
     @Override
     public void encode(OutputStream outputStream, Object data, Charset 
charset) throws EncodeException {
-        // protobuf
-        // TODO int compressed = 
Identity.MESSAGE_ENCODING.equals(requestMetadata.compressor.getMessageEncoding())
 ? 0 :
-        // 1;
         try {
-            int compressed = 0;
-            outputStream.write(compressed);
-            if (isProtobuf(data)) {
-                ProtobufWriter.write(protobufHttpMessageCodec, outputStream, 
data);
-                return;
-            }
-            // wrapper
-            wrapperHttpMessageCodec.encode(outputStream, data);
-        } catch (IOException e) {
+            outputStream.write(0);
+            byte[] bytes = packableMethod.packResponse(data);
+            writeLength(outputStream, bytes.length);
+            outputStream.write(bytes);

Review Comment:
    **Optimization Issue**: The encoding process involves converting the 
response data into bytes and then writing it to an OutputStream. This process 
can be optimized by directly writing to the OutputStream instead of creating an 
intermediate byte array, which can reduce memory usage and improve performance. 
<br> **Fix**: Modify the packResponse method to write directly to the 
OutputStream. This avoids the creation of an intermediate byte array and can 
significantly reduce memory footprint and potentially increase throughput. <br> 
**Code Suggestion**: 
    ```
    +            packableMethod.packResponseDirectly(outputStream, data);
    ```
   
   



##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcCompositeCodec.java:
##########
@@ -16,123 +16,103 @@
  */
 package org.apache.dubbo.rpc.protocol.tri.h12.grpc;
 
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.config.ConfigurationUtils;
+import org.apache.dubbo.common.io.StreamUtils;
+import org.apache.dubbo.common.utils.ArrayUtils;
 import org.apache.dubbo.remoting.http12.exception.DecodeException;
 import org.apache.dubbo.remoting.http12.exception.EncodeException;
 import org.apache.dubbo.remoting.http12.message.HttpMessageCodec;
 import org.apache.dubbo.remoting.http12.message.MediaType;
+import org.apache.dubbo.rpc.model.FrameworkModel;
+import org.apache.dubbo.rpc.model.MethodDescriptor;
+import org.apache.dubbo.rpc.model.PackableMethod;
+import org.apache.dubbo.rpc.model.PackableMethodFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.Charset;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
-import com.google.protobuf.Message;
-
-import static 
org.apache.dubbo.common.constants.CommonConstants.PROTOBUF_MESSAGE_CLASS_NAME;
+import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_KEY;
+import static 
org.apache.dubbo.common.constants.CommonConstants.DUBBO_PACKABLE_METHOD_FACTORY;
 
 public class GrpcCompositeCodec implements HttpMessageCodec {
 
-    private final ProtobufHttpMessageCodec protobufHttpMessageCodec;
+    private static final String PACKABLE_METHOD_CACHE = 
"PACKABLE_METHOD_CACHE";
 
-    private final WrapperHttpMessageCodec wrapperHttpMessageCodec;
+    private final URL url;
 
-    public GrpcCompositeCodec(
-            ProtobufHttpMessageCodec protobufHttpMessageCodec, 
WrapperHttpMessageCodec wrapperHttpMessageCodec) {
-        this.protobufHttpMessageCodec = protobufHttpMessageCodec;
-        this.wrapperHttpMessageCodec = wrapperHttpMessageCodec;
-    }
+    private final FrameworkModel frameworkModel;
+
+    private final String mediaType;
 
-    public void setEncodeTypes(Class<?>[] encodeTypes) {
-        this.wrapperHttpMessageCodec.setEncodeTypes(encodeTypes);
+    private PackableMethod packableMethod;
+
+    public GrpcCompositeCodec(URL url, FrameworkModel frameworkModel, String 
mediaType) {
+        this.url = url;
+        this.frameworkModel = frameworkModel;
+        this.mediaType = mediaType;
     }
 
-    public void setDecodeTypes(Class<?>[] decodeTypes) {
-        this.wrapperHttpMessageCodec.setDecodeTypes(decodeTypes);
+    public void loadPackableMethod(MethodDescriptor methodDescriptor) {
+        if (methodDescriptor instanceof PackableMethod) {
+            packableMethod = (PackableMethod) methodDescriptor;
+            return;
+        }
+        Map<MethodDescriptor, PackableMethod> cacheMap = 
(Map<MethodDescriptor, PackableMethod>) url.getServiceModel()
+                .getServiceMetadata()
+                .getAttributeMap()
+                .computeIfAbsent(PACKABLE_METHOD_CACHE, k -> new 
ConcurrentHashMap<>());
+        packableMethod = cacheMap.computeIfAbsent(methodDescriptor, md -> 
frameworkModel
+                .getExtensionLoader(PackableMethodFactory.class)
+                
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
+                        .getString(DUBBO_PACKABLE_METHOD_FACTORY, DEFAULT_KEY))
+                .create(methodDescriptor, url, mediaType));
     }
 
     @Override
     public void encode(OutputStream outputStream, Object data, Charset 
charset) throws EncodeException {
-        // protobuf
-        // TODO int compressed = 
Identity.MESSAGE_ENCODING.equals(requestMetadata.compressor.getMessageEncoding())
 ? 0 :
-        // 1;
         try {
-            int compressed = 0;
-            outputStream.write(compressed);
-            if (isProtobuf(data)) {
-                ProtobufWriter.write(protobufHttpMessageCodec, outputStream, 
data);
-                return;
-            }
-            // wrapper
-            wrapperHttpMessageCodec.encode(outputStream, data);
-        } catch (IOException e) {
+            outputStream.write(0);
+            byte[] bytes = packableMethod.packResponse(data);
+            writeLength(outputStream, bytes.length);
+            outputStream.write(bytes);
+        } catch (Exception e) {
             throw new EncodeException(e);
         }
     }
 
     @Override
     public Object decode(InputStream inputStream, Class<?> targetType, Charset 
charset) throws DecodeException {
-        if (isProtoClass(targetType)) {
-            return protobufHttpMessageCodec.decode(inputStream, targetType, 
charset);
+        try {
+            byte[] data = StreamUtils.readBytes(inputStream);

Review Comment:
    **Optimization Issue**: Reading the entire InputStream into a byte array 
for processing may lead to increased memory usage, especially for large data 
sizes. This can affect the performance and scalability of the application. <br> 
**Fix**: Consider processing the InputStream directly without converting it to 
a byte array. If the parsing library supports it, use methods that read from an 
InputStream directly. This approach reduces memory usage and can improve 
performance. <br> **Code Suggestion**: 
    ```
    +            return packableMethod.parseRequestDirectly(inputStream);
    ```
   
   



##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcCompositeCodec.java:
##########
@@ -16,123 +16,103 @@
  */
 package org.apache.dubbo.rpc.protocol.tri.h12.grpc;
 
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.config.ConfigurationUtils;
+import org.apache.dubbo.common.io.StreamUtils;
+import org.apache.dubbo.common.utils.ArrayUtils;
 import org.apache.dubbo.remoting.http12.exception.DecodeException;
 import org.apache.dubbo.remoting.http12.exception.EncodeException;
 import org.apache.dubbo.remoting.http12.message.HttpMessageCodec;
 import org.apache.dubbo.remoting.http12.message.MediaType;
+import org.apache.dubbo.rpc.model.FrameworkModel;
+import org.apache.dubbo.rpc.model.MethodDescriptor;
+import org.apache.dubbo.rpc.model.PackableMethod;
+import org.apache.dubbo.rpc.model.PackableMethodFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.Charset;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
-import com.google.protobuf.Message;
-
-import static 
org.apache.dubbo.common.constants.CommonConstants.PROTOBUF_MESSAGE_CLASS_NAME;
+import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_KEY;
+import static 
org.apache.dubbo.common.constants.CommonConstants.DUBBO_PACKABLE_METHOD_FACTORY;
 
 public class GrpcCompositeCodec implements HttpMessageCodec {
 
-    private final ProtobufHttpMessageCodec protobufHttpMessageCodec;
+    private static final String PACKABLE_METHOD_CACHE = 
"PACKABLE_METHOD_CACHE";
 
-    private final WrapperHttpMessageCodec wrapperHttpMessageCodec;
+    private final URL url;
 
-    public GrpcCompositeCodec(
-            ProtobufHttpMessageCodec protobufHttpMessageCodec, 
WrapperHttpMessageCodec wrapperHttpMessageCodec) {
-        this.protobufHttpMessageCodec = protobufHttpMessageCodec;
-        this.wrapperHttpMessageCodec = wrapperHttpMessageCodec;
-    }
+    private final FrameworkModel frameworkModel;
+
+    private final String mediaType;
 
-    public void setEncodeTypes(Class<?>[] encodeTypes) {
-        this.wrapperHttpMessageCodec.setEncodeTypes(encodeTypes);
+    private PackableMethod packableMethod;
+
+    public GrpcCompositeCodec(URL url, FrameworkModel frameworkModel, String 
mediaType) {
+        this.url = url;
+        this.frameworkModel = frameworkModel;
+        this.mediaType = mediaType;
     }
 
-    public void setDecodeTypes(Class<?>[] decodeTypes) {
-        this.wrapperHttpMessageCodec.setDecodeTypes(decodeTypes);
+    public void loadPackableMethod(MethodDescriptor methodDescriptor) {
+        if (methodDescriptor instanceof PackableMethod) {
+            packableMethod = (PackableMethod) methodDescriptor;
+            return;
+        }
+        Map<MethodDescriptor, PackableMethod> cacheMap = 
(Map<MethodDescriptor, PackableMethod>) url.getServiceModel()
+                .getServiceMetadata()
+                .getAttributeMap()
+                .computeIfAbsent(PACKABLE_METHOD_CACHE, k -> new 
ConcurrentHashMap<>());
+        packableMethod = cacheMap.computeIfAbsent(methodDescriptor, md -> 
frameworkModel
+                .getExtensionLoader(PackableMethodFactory.class)
+                
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
+                        .getString(DUBBO_PACKABLE_METHOD_FACTORY, DEFAULT_KEY))
+                .create(methodDescriptor, url, mediaType));

Review Comment:
    **Optimization Issue**: The use of ConcurrentHashMap.computeIfAbsent within 
loadPackableMethod could lead to unnecessary synchronization overhead and 
potential contention under high-load scenarios, affecting performance. <br> 
**Fix**: Consider using a simple check-then-act idiom with 
ConcurrentHashMap.get() followed by ConcurrentHashMap.putIfAbsent() to reduce 
synchronization overhead. This approach minimizes locking duration, especially 
if the PackableMethod is already computed and present in the cache. <br> **Code 
Suggestion**: 
    ```
    + PackableMethod result = cacheMap.get(methodDescriptor);
    + if (result == null) {
    +     PackableMethod newValue = frameworkModel
    +             .getExtensionLoader(PackableMethodFactory.class)
    +             
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
    +                     .getString(DUBBO_PACKABLE_METHOD_FACTORY, 
DEFAULT_KEY))
    +             .create(methodDescriptor, url, mediaType);
    +     result = cacheMap.putIfAbsent(methodDescriptor, newValue);
    +     if (result == null) {
    +         result = newValue;
    +     }
    + }
    + packableMethod = result;
    ```
   
   



##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcCompositeCodec.java:
##########
@@ -16,123 +16,103 @@
  */
 package org.apache.dubbo.rpc.protocol.tri.h12.grpc;
 
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.config.ConfigurationUtils;
+import org.apache.dubbo.common.io.StreamUtils;
+import org.apache.dubbo.common.utils.ArrayUtils;
 import org.apache.dubbo.remoting.http12.exception.DecodeException;
 import org.apache.dubbo.remoting.http12.exception.EncodeException;
 import org.apache.dubbo.remoting.http12.message.HttpMessageCodec;
 import org.apache.dubbo.remoting.http12.message.MediaType;
+import org.apache.dubbo.rpc.model.FrameworkModel;
+import org.apache.dubbo.rpc.model.MethodDescriptor;
+import org.apache.dubbo.rpc.model.PackableMethod;
+import org.apache.dubbo.rpc.model.PackableMethodFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.Charset;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
-import com.google.protobuf.Message;
-
-import static 
org.apache.dubbo.common.constants.CommonConstants.PROTOBUF_MESSAGE_CLASS_NAME;
+import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_KEY;
+import static 
org.apache.dubbo.common.constants.CommonConstants.DUBBO_PACKABLE_METHOD_FACTORY;
 
 public class GrpcCompositeCodec implements HttpMessageCodec {
 
-    private final ProtobufHttpMessageCodec protobufHttpMessageCodec;
+    private static final String PACKABLE_METHOD_CACHE = 
"PACKABLE_METHOD_CACHE";
 
-    private final WrapperHttpMessageCodec wrapperHttpMessageCodec;
+    private final URL url;
 
-    public GrpcCompositeCodec(
-            ProtobufHttpMessageCodec protobufHttpMessageCodec, 
WrapperHttpMessageCodec wrapperHttpMessageCodec) {
-        this.protobufHttpMessageCodec = protobufHttpMessageCodec;
-        this.wrapperHttpMessageCodec = wrapperHttpMessageCodec;
-    }
+    private final FrameworkModel frameworkModel;
+
+    private final String mediaType;
 
-    public void setEncodeTypes(Class<?>[] encodeTypes) {
-        this.wrapperHttpMessageCodec.setEncodeTypes(encodeTypes);
+    private PackableMethod packableMethod;
+
+    public GrpcCompositeCodec(URL url, FrameworkModel frameworkModel, String 
mediaType) {
+        this.url = url;
+        this.frameworkModel = frameworkModel;
+        this.mediaType = mediaType;
     }
 
-    public void setDecodeTypes(Class<?>[] decodeTypes) {
-        this.wrapperHttpMessageCodec.setDecodeTypes(decodeTypes);
+    public void loadPackableMethod(MethodDescriptor methodDescriptor) {
+        if (methodDescriptor instanceof PackableMethod) {
+            packableMethod = (PackableMethod) methodDescriptor;
+            return;
+        }
+        Map<MethodDescriptor, PackableMethod> cacheMap = 
(Map<MethodDescriptor, PackableMethod>) url.getServiceModel()
+                .getServiceMetadata()
+                .getAttributeMap()
+                .computeIfAbsent(PACKABLE_METHOD_CACHE, k -> new 
ConcurrentHashMap<>());
+        packableMethod = cacheMap.computeIfAbsent(methodDescriptor, md -> 
frameworkModel
+                .getExtensionLoader(PackableMethodFactory.class)
+                
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
+                        .getString(DUBBO_PACKABLE_METHOD_FACTORY, DEFAULT_KEY))
+                .create(methodDescriptor, url, mediaType));
     }
 
     @Override
     public void encode(OutputStream outputStream, Object data, Charset 
charset) throws EncodeException {
-        // protobuf
-        // TODO int compressed = 
Identity.MESSAGE_ENCODING.equals(requestMetadata.compressor.getMessageEncoding())
 ? 0 :
-        // 1;
         try {
-            int compressed = 0;
-            outputStream.write(compressed);
-            if (isProtobuf(data)) {
-                ProtobufWriter.write(protobufHttpMessageCodec, outputStream, 
data);
-                return;
-            }
-            // wrapper
-            wrapperHttpMessageCodec.encode(outputStream, data);
-        } catch (IOException e) {
+            outputStream.write(0);
+            byte[] bytes = packableMethod.packResponse(data);
+            writeLength(outputStream, bytes.length);
+            outputStream.write(bytes);

Review Comment:
    **Performance Issue**: Direct use of packResponse without checking the 
data's type or ensuring it's not null could lead to NullPointerException or 
ClassCastException, impacting the robustness of the encoding process. <br> 
**Fix**: Implement a check to ensure that the data is not null and is of a type 
that can be safely cast to the expected type for packing. Provide a clear error 
message or handling strategy for cases where the data is not of the expected 
type. <br> **Code Suggestion**: 
    ```
    +            if (data != null && data instanceof ExpectedDataType) {
    +                byte[] bytes = packResponse(data);
    +                writeLength(outputStream, bytes.length);
    +                outputStream.write(bytes);
    +            } else {
    +                throw new IllegalArgumentException("Data is null or not of 
the expected type");
    +            }
    ```
   
   



##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcCompositeCodec.java:
##########
@@ -16,123 +16,103 @@
  */
 package org.apache.dubbo.rpc.protocol.tri.h12.grpc;
 
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.config.ConfigurationUtils;
+import org.apache.dubbo.common.io.StreamUtils;
+import org.apache.dubbo.common.utils.ArrayUtils;
 import org.apache.dubbo.remoting.http12.exception.DecodeException;
 import org.apache.dubbo.remoting.http12.exception.EncodeException;
 import org.apache.dubbo.remoting.http12.message.HttpMessageCodec;
 import org.apache.dubbo.remoting.http12.message.MediaType;
+import org.apache.dubbo.rpc.model.FrameworkModel;
+import org.apache.dubbo.rpc.model.MethodDescriptor;
+import org.apache.dubbo.rpc.model.PackableMethod;
+import org.apache.dubbo.rpc.model.PackableMethodFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.Charset;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
-import com.google.protobuf.Message;
-
-import static 
org.apache.dubbo.common.constants.CommonConstants.PROTOBUF_MESSAGE_CLASS_NAME;
+import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_KEY;
+import static 
org.apache.dubbo.common.constants.CommonConstants.DUBBO_PACKABLE_METHOD_FACTORY;
 
 public class GrpcCompositeCodec implements HttpMessageCodec {
 
-    private final ProtobufHttpMessageCodec protobufHttpMessageCodec;
+    private static final String PACKABLE_METHOD_CACHE = 
"PACKABLE_METHOD_CACHE";
 
-    private final WrapperHttpMessageCodec wrapperHttpMessageCodec;
+    private final URL url;
 
-    public GrpcCompositeCodec(
-            ProtobufHttpMessageCodec protobufHttpMessageCodec, 
WrapperHttpMessageCodec wrapperHttpMessageCodec) {
-        this.protobufHttpMessageCodec = protobufHttpMessageCodec;
-        this.wrapperHttpMessageCodec = wrapperHttpMessageCodec;
-    }
+    private final FrameworkModel frameworkModel;
+
+    private final String mediaType;
 
-    public void setEncodeTypes(Class<?>[] encodeTypes) {
-        this.wrapperHttpMessageCodec.setEncodeTypes(encodeTypes);
+    private PackableMethod packableMethod;
+
+    public GrpcCompositeCodec(URL url, FrameworkModel frameworkModel, String 
mediaType) {
+        this.url = url;
+        this.frameworkModel = frameworkModel;
+        this.mediaType = mediaType;
     }
 
-    public void setDecodeTypes(Class<?>[] decodeTypes) {
-        this.wrapperHttpMessageCodec.setDecodeTypes(decodeTypes);
+    public void loadPackableMethod(MethodDescriptor methodDescriptor) {
+        if (methodDescriptor instanceof PackableMethod) {
+            packableMethod = (PackableMethod) methodDescriptor;
+            return;
+        }
+        Map<MethodDescriptor, PackableMethod> cacheMap = 
(Map<MethodDescriptor, PackableMethod>) url.getServiceModel()
+                .getServiceMetadata()
+                .getAttributeMap()
+                .computeIfAbsent(PACKABLE_METHOD_CACHE, k -> new 
ConcurrentHashMap<>());
+        packableMethod = cacheMap.computeIfAbsent(methodDescriptor, md -> 
frameworkModel
+                .getExtensionLoader(PackableMethodFactory.class)
+                
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
+                        .getString(DUBBO_PACKABLE_METHOD_FACTORY, DEFAULT_KEY))
+                .create(methodDescriptor, url, mediaType));
     }
 
     @Override
     public void encode(OutputStream outputStream, Object data, Charset 
charset) throws EncodeException {
-        // protobuf
-        // TODO int compressed = 
Identity.MESSAGE_ENCODING.equals(requestMetadata.compressor.getMessageEncoding())
 ? 0 :
-        // 1;
         try {
-            int compressed = 0;
-            outputStream.write(compressed);
-            if (isProtobuf(data)) {
-                ProtobufWriter.write(protobufHttpMessageCodec, outputStream, 
data);
-                return;
-            }
-            // wrapper
-            wrapperHttpMessageCodec.encode(outputStream, data);
-        } catch (IOException e) {
+            outputStream.write(0);
+            byte[] bytes = packableMethod.packResponse(data);
+            writeLength(outputStream, bytes.length);
+            outputStream.write(bytes);
+        } catch (Exception e) {
             throw new EncodeException(e);
         }
     }
 
     @Override
     public Object decode(InputStream inputStream, Class<?> targetType, Charset 
charset) throws DecodeException {
-        if (isProtoClass(targetType)) {
-            return protobufHttpMessageCodec.decode(inputStream, targetType, 
charset);
+        try {
+            byte[] data = StreamUtils.readBytes(inputStream);
+            return packableMethod.parseRequest(data);
+        } catch (Exception e) {
+            throw new DecodeException(e);

Review Comment:
    **Performance Issue**: Catching generic Exception for stream reading and 
data parsing in decode method can hide bugs and make debugging difficult. It 
also indicates potential issues with exception handling where more specific 
exceptions should be caught and handled appropriately. <br> **Fix**: Refine the 
catch block to handle specific exceptions, such as IOException or relevant 
parsing exceptions. This will improve the code's robustness and maintainability 
by making error handling more precise. <br> **Code Suggestion**: 
    ```
    +        } catch (IOException e) {
    +            throw new DecodeException("IO error occurred", e);
    +        } catch (ParsingException e) {
    +            throw new DecodeException("Parsing error occurred", e);
    +        }
    ```
   
   



##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcCompositeCodec.java:
##########
@@ -16,123 +16,103 @@
  */
 package org.apache.dubbo.rpc.protocol.tri.h12.grpc;
 
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.config.ConfigurationUtils;
+import org.apache.dubbo.common.io.StreamUtils;
+import org.apache.dubbo.common.utils.ArrayUtils;
 import org.apache.dubbo.remoting.http12.exception.DecodeException;
 import org.apache.dubbo.remoting.http12.exception.EncodeException;
 import org.apache.dubbo.remoting.http12.message.HttpMessageCodec;
 import org.apache.dubbo.remoting.http12.message.MediaType;
+import org.apache.dubbo.rpc.model.FrameworkModel;
+import org.apache.dubbo.rpc.model.MethodDescriptor;
+import org.apache.dubbo.rpc.model.PackableMethod;
+import org.apache.dubbo.rpc.model.PackableMethodFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.Charset;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
-import com.google.protobuf.Message;
-
-import static 
org.apache.dubbo.common.constants.CommonConstants.PROTOBUF_MESSAGE_CLASS_NAME;
+import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_KEY;
+import static 
org.apache.dubbo.common.constants.CommonConstants.DUBBO_PACKABLE_METHOD_FACTORY;
 
 public class GrpcCompositeCodec implements HttpMessageCodec {
 
-    private final ProtobufHttpMessageCodec protobufHttpMessageCodec;
+    private static final String PACKABLE_METHOD_CACHE = 
"PACKABLE_METHOD_CACHE";
 
-    private final WrapperHttpMessageCodec wrapperHttpMessageCodec;
+    private final URL url;
 
-    public GrpcCompositeCodec(
-            ProtobufHttpMessageCodec protobufHttpMessageCodec, 
WrapperHttpMessageCodec wrapperHttpMessageCodec) {
-        this.protobufHttpMessageCodec = protobufHttpMessageCodec;
-        this.wrapperHttpMessageCodec = wrapperHttpMessageCodec;
-    }
+    private final FrameworkModel frameworkModel;
+
+    private final String mediaType;
 
-    public void setEncodeTypes(Class<?>[] encodeTypes) {
-        this.wrapperHttpMessageCodec.setEncodeTypes(encodeTypes);
+    private PackableMethod packableMethod;
+
+    public GrpcCompositeCodec(URL url, FrameworkModel frameworkModel, String 
mediaType) {
+        this.url = url;
+        this.frameworkModel = frameworkModel;
+        this.mediaType = mediaType;
     }
 
-    public void setDecodeTypes(Class<?>[] decodeTypes) {
-        this.wrapperHttpMessageCodec.setDecodeTypes(decodeTypes);
+    public void loadPackableMethod(MethodDescriptor methodDescriptor) {
+        if (methodDescriptor instanceof PackableMethod) {
+            packableMethod = (PackableMethod) methodDescriptor;
+            return;
+        }
+        Map<MethodDescriptor, PackableMethod> cacheMap = 
(Map<MethodDescriptor, PackableMethod>) url.getServiceModel()
+                .getServiceMetadata()
+                .getAttributeMap()
+                .computeIfAbsent(PACKABLE_METHOD_CACHE, k -> new 
ConcurrentHashMap<>());
+        packableMethod = cacheMap.computeIfAbsent(methodDescriptor, md -> 
frameworkModel
+                .getExtensionLoader(PackableMethodFactory.class)
+                
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
+                        .getString(DUBBO_PACKABLE_METHOD_FACTORY, DEFAULT_KEY))
+                .create(methodDescriptor, url, mediaType));

Review Comment:
    **Performance Issue**: The implementation of loading PackableMethod from a 
ConcurrentHashMap cache involves multiple nested method calls and potentially 
inefficient lookup and insertion operations. <br> **Fix**: Optimize the cache 
access pattern by checking for the existence of the key before attempting to 
computeIfAbsent to minimize unnecessary lambda executions. Additionally, 
consider using a more efficient concurrent data structure or optimizing the 
method descriptor's hashcode and equals methods for faster access. <br> **Code 
Suggestion**: 
    ```
    +        PackableMethod existing = cacheMap.get(methodDescriptor);
    +        if (existing == null) {
    +            synchronized (cacheMap) {
    +                existing = cacheMap.get(methodDescriptor);
    +                if (existing == null) {
    +                    existing = frameworkModel
    +                            
.getExtensionLoader(PackableMethodFactory.class)
    +                            
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
    +                                    
.getString(DUBBO_PACKABLE_METHOD_FACTORY, DEFAULT_KEY))
    +                            .create(methodDescriptor, url, mediaType);
    +                    cacheMap.put(methodDescriptor, existing);
    +                }
    +            }
    +        }
    +        packableMethod = existing;
    ```
   
   



##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcCompositeCodec.java:
##########
@@ -16,123 +16,103 @@
  */
 package org.apache.dubbo.rpc.protocol.tri.h12.grpc;
 
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.config.ConfigurationUtils;
+import org.apache.dubbo.common.io.StreamUtils;
+import org.apache.dubbo.common.utils.ArrayUtils;
 import org.apache.dubbo.remoting.http12.exception.DecodeException;
 import org.apache.dubbo.remoting.http12.exception.EncodeException;
 import org.apache.dubbo.remoting.http12.message.HttpMessageCodec;
 import org.apache.dubbo.remoting.http12.message.MediaType;
+import org.apache.dubbo.rpc.model.FrameworkModel;
+import org.apache.dubbo.rpc.model.MethodDescriptor;
+import org.apache.dubbo.rpc.model.PackableMethod;
+import org.apache.dubbo.rpc.model.PackableMethodFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.Charset;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
-import com.google.protobuf.Message;
-
-import static 
org.apache.dubbo.common.constants.CommonConstants.PROTOBUF_MESSAGE_CLASS_NAME;
+import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_KEY;
+import static 
org.apache.dubbo.common.constants.CommonConstants.DUBBO_PACKABLE_METHOD_FACTORY;
 
 public class GrpcCompositeCodec implements HttpMessageCodec {
 
-    private final ProtobufHttpMessageCodec protobufHttpMessageCodec;
+    private static final String PACKABLE_METHOD_CACHE = 
"PACKABLE_METHOD_CACHE";
 
-    private final WrapperHttpMessageCodec wrapperHttpMessageCodec;
+    private final URL url;
 
-    public GrpcCompositeCodec(
-            ProtobufHttpMessageCodec protobufHttpMessageCodec, 
WrapperHttpMessageCodec wrapperHttpMessageCodec) {
-        this.protobufHttpMessageCodec = protobufHttpMessageCodec;
-        this.wrapperHttpMessageCodec = wrapperHttpMessageCodec;
-    }
+    private final FrameworkModel frameworkModel;
+
+    private final String mediaType;
 
-    public void setEncodeTypes(Class<?>[] encodeTypes) {
-        this.wrapperHttpMessageCodec.setEncodeTypes(encodeTypes);
+    private PackableMethod packableMethod;
+
+    public GrpcCompositeCodec(URL url, FrameworkModel frameworkModel, String 
mediaType) {
+        this.url = url;
+        this.frameworkModel = frameworkModel;
+        this.mediaType = mediaType;
     }
 
-    public void setDecodeTypes(Class<?>[] decodeTypes) {
-        this.wrapperHttpMessageCodec.setDecodeTypes(decodeTypes);
+    public void loadPackableMethod(MethodDescriptor methodDescriptor) {
+        if (methodDescriptor instanceof PackableMethod) {
+            packableMethod = (PackableMethod) methodDescriptor;
+            return;
+        }
+        Map<MethodDescriptor, PackableMethod> cacheMap = 
(Map<MethodDescriptor, PackableMethod>) url.getServiceModel()
+                .getServiceMetadata()
+                .getAttributeMap()
+                .computeIfAbsent(PACKABLE_METHOD_CACHE, k -> new 
ConcurrentHashMap<>());
+        packableMethod = cacheMap.computeIfAbsent(methodDescriptor, md -> 
frameworkModel
+                .getExtensionLoader(PackableMethodFactory.class)
+                
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
+                        .getString(DUBBO_PACKABLE_METHOD_FACTORY, DEFAULT_KEY))
+                .create(methodDescriptor, url, mediaType));

Review Comment:
    **Security Issue**: The dynamic loading and caching mechanism for 
'PackableMethod' instances does not appear to validate or sanitize the 
'MethodDescriptor' or the URL used to retrieve configurations, which could 
potentially be exploited to inject malicious configurations or execute code 
remotely if external input is used to construct these objects. <br> **Fix**: 
Implement validation checks on 'MethodDescriptor' objects and any URLs or 
configurations used to create or retrieve 'PackableMethod' instances. Ensure 
that only trusted sources can influence these objects and that any input used 
in their creation is sanitized. <br> **Code Suggestion**: 
    ```
    +        if (methodDescriptor instanceof PackableMethod) {
    +            packableMethod = (PackableMethod) methodDescriptor;
    +            return;
    +        }
    +        // Validate methodDescriptor and URL before proceeding
    +        if (!isValidMethodDescriptor(methodDescriptor) || 
!isTrustedUrl(url)) {
    +            throw new IllegalArgumentException("Invalid MethodDescriptor 
or URL");
    +        }
    +        Map<MethodDescriptor, PackableMethod> cacheMap = 
(Map<MethodDescriptor, PackableMethod>) url.getServiceModel()
    +                .getServiceMetadata()
    +                .getAttributeMap()
    +                .computeIfAbsent(PACKABLE_METHOD_CACHE, k -> new 
ConcurrentHashMap<>());
    +        packableMethod = cacheMap.computeIfAbsent(methodDescriptor, md -> 
frameworkModel
    +                .getExtensionLoader(PackableMethodFactory.class)
    +                
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
    +                        .getString(DUBBO_PACKABLE_METHOD_FACTORY, 
DEFAULT_KEY))
    +                .create(methodDescriptor, url, mediaType));
    ```
   
   



##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcCompositeCodec.java:
##########
@@ -16,123 +16,103 @@
  */
 package org.apache.dubbo.rpc.protocol.tri.h12.grpc;
 
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.config.ConfigurationUtils;
+import org.apache.dubbo.common.io.StreamUtils;
+import org.apache.dubbo.common.utils.ArrayUtils;
 import org.apache.dubbo.remoting.http12.exception.DecodeException;
 import org.apache.dubbo.remoting.http12.exception.EncodeException;
 import org.apache.dubbo.remoting.http12.message.HttpMessageCodec;
 import org.apache.dubbo.remoting.http12.message.MediaType;
+import org.apache.dubbo.rpc.model.FrameworkModel;
+import org.apache.dubbo.rpc.model.MethodDescriptor;
+import org.apache.dubbo.rpc.model.PackableMethod;
+import org.apache.dubbo.rpc.model.PackableMethodFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.Charset;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
-import com.google.protobuf.Message;
-
-import static 
org.apache.dubbo.common.constants.CommonConstants.PROTOBUF_MESSAGE_CLASS_NAME;
+import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_KEY;
+import static 
org.apache.dubbo.common.constants.CommonConstants.DUBBO_PACKABLE_METHOD_FACTORY;
 
 public class GrpcCompositeCodec implements HttpMessageCodec {
 
-    private final ProtobufHttpMessageCodec protobufHttpMessageCodec;
+    private static final String PACKABLE_METHOD_CACHE = 
"PACKABLE_METHOD_CACHE";
 
-    private final WrapperHttpMessageCodec wrapperHttpMessageCodec;
+    private final URL url;
 
-    public GrpcCompositeCodec(
-            ProtobufHttpMessageCodec protobufHttpMessageCodec, 
WrapperHttpMessageCodec wrapperHttpMessageCodec) {
-        this.protobufHttpMessageCodec = protobufHttpMessageCodec;
-        this.wrapperHttpMessageCodec = wrapperHttpMessageCodec;
-    }
+    private final FrameworkModel frameworkModel;
+
+    private final String mediaType;
 
-    public void setEncodeTypes(Class<?>[] encodeTypes) {
-        this.wrapperHttpMessageCodec.setEncodeTypes(encodeTypes);
+    private PackableMethod packableMethod;
+
+    public GrpcCompositeCodec(URL url, FrameworkModel frameworkModel, String 
mediaType) {
+        this.url = url;
+        this.frameworkModel = frameworkModel;
+        this.mediaType = mediaType;
     }
 
-    public void setDecodeTypes(Class<?>[] decodeTypes) {
-        this.wrapperHttpMessageCodec.setDecodeTypes(decodeTypes);
+    public void loadPackableMethod(MethodDescriptor methodDescriptor) {
+        if (methodDescriptor instanceof PackableMethod) {
+            packableMethod = (PackableMethod) methodDescriptor;
+            return;
+        }
+        Map<MethodDescriptor, PackableMethod> cacheMap = 
(Map<MethodDescriptor, PackableMethod>) url.getServiceModel()
+                .getServiceMetadata()
+                .getAttributeMap()
+                .computeIfAbsent(PACKABLE_METHOD_CACHE, k -> new 
ConcurrentHashMap<>());
+        packableMethod = cacheMap.computeIfAbsent(methodDescriptor, md -> 
frameworkModel
+                .getExtensionLoader(PackableMethodFactory.class)
+                
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
+                        .getString(DUBBO_PACKABLE_METHOD_FACTORY, DEFAULT_KEY))
+                .create(methodDescriptor, url, mediaType));

Review Comment:
    **Scalability Issue**: The implementation of loading and caching 
PackableMethod instances involves accessing a ConcurrentHashMap and potentially 
creating new instances of PackableMethod. This could become a scalability 
bottleneck under high concurrency, as multiple threads might contend for locks 
within ConcurrentHashMap or experience increased latency due to cache misses 
and the subsequent creation of new PackableMethod instances. <br> **Fix**: 
Consider using a more efficient concurrent data structure or caching mechanism 
that minimizes lock contention and efficiently handles high read/write 
concurrency. Evaluate the use of a ConcurrentLinkedQueue with a thread-local 
storage pattern for read-heavy scenarios, ensuring that the creation of new 
PackableMethod instances is minimized and efficiently handled in a lock-free 
manner. <br> **Code Suggestion**: 
    ```
    +import java.util.concurrent.ConcurrentLinkedQueue;
    +import java.lang.ThreadLocal;
    -Map<MethodDescriptor, PackableMethod> cacheMap = (Map<MethodDescriptor, 
PackableMethod>) url.getServiceModel()
    +ThreadLocal<ConcurrentLinkedQueue<PackableMethod>> cacheMap = 
ThreadLocal.withInitial(() -> new ConcurrentLinkedQueue<>());
    +PackableMethod cachedMethod = cacheMap.get().stream().filter(m -> 
m.matches(methodDescriptor)).findFirst().orElseGet(() -> {
    +    PackableMethod newMethod = frameworkModel
    +            .getExtensionLoader(PackableMethodFactory.class)
    +            
.getExtension(ConfigurationUtils.getGlobalConfiguration(url.getApplicationModel())
    +                    .getString(DUBBO_PACKABLE_METHOD_FACTORY, DEFAULT_KEY))
    +            .create(methodDescriptor, url, mediaType);
    +    cacheMap.get().add(newMethod);
    +    return newMethod;
    +});
    +packableMethod = cachedMethod;
    ```
   
   



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to