Adar Dembo has posted comments on this change. Change subject: KUDU-1410 (part 2). rpc: clean up generated RPC service code ......................................................................
Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/2795/5/src/kudu/rpc/protoc-gen-krpc.cc File src/kudu/rpc/protoc-gen-krpc.cc: Line 487 I'm trying to gauge the difference in dispatch expense. Before, we'd have: 1. A vtable lookup (to get to the right Handle()). 2. A long string of if statements that would compare the pointers belonging to the RPC name and the call name. 3. Dispatch as a direct function call. After, we have: 1. A vtable lookup (to get to the single Handle() belonging to GeneratedServiceIf). 2. a single map lookup to get the RpcMethodInfo. 3. Dispatch as a call into a lambda, followed by a direct function call. If this is right, isn't the latter more expensive, on account of the double dispatch and map lookup? http://gerrit.cloudera.org:8080/#/c/2795/5/src/kudu/rpc/service_if.h File src/kudu/rpc/service_if.h: Line 61: std::function<void(const google::protobuf::Message* req, Isn't it more conventional to pass 'req' as cref if it's read-only? Line 93: void Handle(InboundCall* incoming) override; Shouldn't it also be defined as virtual? -- To view, visit http://gerrit.cloudera.org:8080/2795 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7b42a5ba1de59315660a79b125e70938c3a1b2e Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-HasComments: Yes
