Hello Andrew Wong, I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/15716 to review the following change. Change subject: rpc: avoid an extra epoll cycle when data can be sent immediately ...................................................................... rpc: avoid an extra epoll cycle when data can be sent immediately Prior to this patch, when queueing outbound data in the reactor thread, we would always enqueue the transfer, set the epoll watcher on the socket to wait for the "writable" state, and then go back into the epoll loop. In most cases, there is sufficient buffer space, so the next epoll loop would return immediately. We'd then perform the write, see nothing left in the transfer queue, and disable the "write" watcher again. This whole dance would result in a sequence of epoll_ctl / epolL_wait / send / epoll_ctl for every outbound message. This patch changes the behavior so that, when we enqueue a transfer, if the transfer queue was previously empty (ie we hadn't gotten blocked on another transfer), we now try writing it immediately. Only if it fails to fully write do we enable the epoll watching. This saves a pair of epoll_ctl calls and an epoll_wait for both the request and response sides of every RPC. I tested this using a new benchmark I'm working on which sends 20480 synchronous RPCs from a single thread. I used 'perf trace' to count syscalls: Without the patch: Performance counter stats for 'build/latest/bin/rpc_stub-test --gtest_filter=*Trans* --benchmark_total_mb=40960 --benchmark_method_pattern=fd*mmap*reuse*reuse) -benchmark-setup-pattern=all-same-*': 81,924 syscalls:sys_enter_epoll_ctl (4 per RPC) 122,886 syscalls:sys_enter_epoll_wait (6 per RPC) With the patch: Performance counter stats for 'build/latest/bin/rpc_stub-test --gtest_filter=*Trans* --benchmark_total_mb=40960 --benchmark_method_pattern=fd*mmap*reuse*reuse) -benchmark-setup-pattern=all-same-*': 6 syscalls:sys_enter_epoll_ctl (0 per RPC) 81,927 syscalls:sys_enter_epoll_wait (4 per RPC) I also benchmarked with: rpc-bench --gtest_filter=\*Calls -client-threads=1 \ -server-reactors=1 --gtest_repeat=10 2>&1 | grep Reqs/sec and ran a t-test on the results: Welch Two Sample t-test data: before and after t = -5.5671, df = 12.065, p-value = 0.00012 alternative hypothesis: true difference in means is not equal to 0 95 percent confidence interval: -2700.794 -1182.046 sample estimates: mean of x mean of y 25353.48 27294.90 so 95% confidence interval this improves throughput between 5 and 10%. Change-Id: I30af102224d5db2cb526b4c2ae981d6e9defd82a --- M src/kudu/rpc/connection.cc M src/kudu/rpc/connection.h 2 files changed, 23 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/15716/1 -- To view, visit http://gerrit.cloudera.org:8080/15716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I30af102224d5db2cb526b4c2ae981d6e9defd82a Gerrit-Change-Number: 15716 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Andrew Wong <andrew.w...@cloudera.com>