Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/16209 )
Change subject: tls_socket: avoid cork/uncork dance for small writevs ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/16209/1/src/kudu/security/tls_socket.h File src/kudu/security/tls_socket.h: http://gerrit.cloudera.org:8080/#/c/16209/1/src/kudu/security/tls_socket.h@64 PS1, Line 64: // Socket-local buffer used by Writev(). : faststring buf_; > This variable isn't used across Write() calls or such. Using a local variab it's used across Write calls in that we don't want to pay the malloc/free cost in each call http://gerrit.cloudera.org:8080/#/c/16209/1/src/kudu/security/tls_socket.cc File src/kudu/security/tls_socket.cc: http://gerrit.cloudera.org:8080/#/c/16209/1/src/kudu/security/tls_socket.cc@127 PS1, Line 127: int > nit: size_t to match the type of the total_size variable? I benchmarked the syscall overhead for setsockopt and redid the math here. It does work out that a bit larger of a buffer is actually beneficial. http://gerrit.cloudera.org:8080/#/c/16209/1/src/kudu/security/tls_socket.cc@134 PS1, Line 134: // TODO(todd) why int32 in once place and int64 in the other? > Write() call does take int32_t. Is this TODO still relevant? that's the issue -- Write takes int32 but Writev takes int64, so we need this local int32_t temporary here, which is kinda weird http://gerrit.cloudera.org:8080/#/c/16209/1/src/kudu/security/tls_socket.cc@138 PS1, Line 138: return s; > Should this be I don't think it's necessary since the Write() call itself already handles returning OK in that case. http://gerrit.cloudera.org:8080/#/c/16209/1/src/kudu/security/tls_socket.cc@144 PS1, Line 144: if (use_cork_) > While you are here optimizing the number of syscalls and the transfer of sm yep, thanks -- To view, visit http://gerrit.cloudera.org:8080/16209 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6abf790a38878b41862655e643782a7624b763f Gerrit-Change-Number: 16209 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Bankim Bhavsar <ban...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Thu, 23 Jul 2020 23:31:38 +0000 Gerrit-HasComments: Yes
