Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15440 )
Change subject: rpc: reduce context switches and receive calls ...................................................................... Patch Set 1: (4 comments) > Patch Set 1: > > (2 comments) > > Would it be possible to add some correctness related tests for this change? The code path is covered pretty well by the benchmark in the commit message (and really by every other distributed test we have). I didn't think it was worth trying to write a really targeted unit test for InboundTransfer here since functional tests cover it. http://gerrit.cloudera.org:8080/#/c/15440/1/src/kudu/rpc/transfer.h File src/kudu/rpc/transfer.h: http://gerrit.cloudera.org:8080/#/c/15440/1/src/kudu/rpc/transfer.h@70 PS1, Line 70: faststring&& > Yeah, the Google Style Guide doesn't like rvalue references outside of move Done http://gerrit.cloudera.org:8080/#/c/15440/1/src/kudu/rpc/transfer.h@73 PS1, Line 73: faststring* extra_4 > nit: doc this? Done http://gerrit.cloudera.org:8080/#/c/15440/1/src/kudu/rpc/transfer.cc File src/kudu/rpc/transfer.cc: http://gerrit.cloudera.org:8080/#/c/15440/1/src/kudu/rpc/transfer.cc@104 PS1, Line 104: const int kExtraReadLength = kMsgLengthPrefixLength; > static constexpr Done http://gerrit.cloudera.org:8080/#/c/15440/1/src/kudu/rpc/transfer.cc@163 PS1, Line 163: extra_4->clear(); > DCHECK extra_read <= kExtraReadLength? Done -- To view, visit http://gerrit.cloudera.org:8080/15440 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32c5e4d146c25be8e90665a0cb8385fcd017b15c Gerrit-Change-Number: 15440 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Thu, 19 Mar 2020 21:40:27 +0000 Gerrit-HasComments: Yes
