BitoAgent commented on code in PR #13786:
URL: https://github.com/apache/dubbo/pull/13786#discussion_r1569662610
##########
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:
**Performance Issue**: The creation of GrpcCompositeCodec with URL,
frameworkModel, and mediaType parameters simplifies object construction but may
overlook the need for serialization type specification which was previously
determined by UrlUtils.serializationOrDefault(url). This change could
potentially affect performance if the serialization type impacts codec
performance. <br> **Fix**: Re-evaluate the removal of serialization type
determination in codec creation. Consider extending GrpcCompositeCodec's
constructor to include a serialization type parameter or a method to set the
serialization type post-construction, ensuring that the codec's performance is
optimized for the serialization type used. <br> **Code Suggestion**:
```
- return new GrpcCompositeCodec(url, frameworkModel, mediaType);
+ String serializeName = UrlUtils.serializationOrDefault(url);
+ return new GrpcCompositeCodec(url, frameworkModel, mediaType,
serializeName);
```
--
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]
