Shaozheng528 commented on issue #3167:
URL: https://github.com/apache/brpc/issues/3167#issuecomment-3663343270
> > 是的，当前方案只是将存在rdma宏定义的方法简单抽取出来，然后拆分到不同transport子类上
>
>
这种方法看似简单，但实际实现可能会遇到问题，这些方法里面会访问Socket的私有成员，这就意味着具体Transport的实现里面，还要访问Socket的私有成员。这样并没有从根本上解决Socket和Transport解耦的问题，而且会引入新的问题——Socket里和Transport无关的一些代码被复制到多个Transport的实现代码里，增加了维护成本。
>
> 因此，我不建议这种改法，我觉得可以这样：
>
> 1. 将Socket里面，和特定transport相关的私有成员抽取出来到各自的transport中，如
>
> class Socket {
> private:
> Transport* _transport;
> };
>
> class TcpTransport {
> private:
> int _fd;
> // ...其他tcp独有的成员
> };
>
> class RdmaTransport {
> private:
> rdma::RdmaEndpoint* _rdma_ep;
> RdmaState _rdma_state;
> TcpTransport* _tcp_transport; // RDMA状态关闭时, fallback到tcp
> // ...其他rdma独有的成员
> };
> 2. 把Socket里面，#if BRPC_WITH_RDMA 相关的代码抽取到transport中，如
>
> StartWrite里面
>
> if (_conn) {
> butil::IOBuf* data_arr[1] = { &req->data };
> nw = _conn->CutMessageIntoFileDescriptor(fd(), data_arr, 1);
> } else {
> #if BRPC_WITH_RDMA
> if (_rdma_ep && _rdma_state != RDMA_OFF) {
> butil::IOBuf* data_arr[1] = { &req->data };
> nw = _rdma_ep->CutFromIOBufList(data_arr, 1);
> } else {
> #else
> {
> #endif
> nw = req->data.cut_into_file_descriptor(fd());
> }
> }
> 这段代码可以改成
>
> if (_conn) {
> butil::IOBuf* data_arr[1] = { &req->data };
> nw = _conn->CutMessageIntoFileDescriptor(fd(), data_arr, 1);
> } else {
> nw = _transport->CutFromIOBuf(&req->data);
> }
> 然后各自的transport里面
>
> int TcpTransport::CutFromIOBuf(butil::IOBuf* buf) {
> return buf->cut_into_file_descriptor(_fd);
> }
>
> int RdmaTransport::CutFromIOBuf(butil::IOBuf* buf) {
> if (_rdma_ep && _rdma_state != RDMA_OFF) {
> butil::IOBuf* data_arr[1] = { buf };
> return _rdma_ep->CutFromIOBufList(data_arr, 1);
> } else {
> return _tcp_transport->CutFromIOBuf(buf);
> }
> }
这种改法看着确实比将整个函数下推更好，脑测应该可以满足目前RDMA和TCP的需求，但是我有一个疑问，就是这种实现只能新增一些操作，不能减少一些操作，如果后续加入的传输层协议对目前的非宏定义部分的操作有一些不需要的也会因为这个架构而被迫执行响应的操作，可能会有一些不必要的性能损耗？
那个时候再将对应的操作下推到其他协议中来避免这个问题可以吗？
或者对这一点损耗不进行处理（如果损耗很小）？
或者可以确定不会出现这种问题？
--
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]
