From f135cbdd4501a77e219c47f20e63bee8887236e6 Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Mon, 7 Jul 2014 12:57:03 +0000 Subject: [PATCH] * fhandler_socket.cc (fhandler_socket::send_internal): Improve loop to write streams in chunks of wmem() bytes to raise performance when writing small buffers. Rename variables and add comments to help understanding the code in years to come. --- winsup/cygwin/ChangeLog | 7 ++++ winsup/cygwin/fhandler_socket.cc | 58 +++++++++++++++++++++++--------- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 6763fa821..121f3e601 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,10 @@ +2014-07-07 Corinna Vinschen + + * fhandler_socket.cc (fhandler_socket::send_internal): Improve loop to + write streams in chunks of wmem() bytes to raise performance when + writing small buffers. Rename variables and add comments to help + understanding the code in years to come. + 2014-07-07 Corinna Vinschen * passwd.cc (pg_ent::enumerate_ad): Revert to simply skipping a domain diff --git a/winsup/cygwin/fhandler_socket.cc b/winsup/cygwin/fhandler_socket.cc index 2c3c75c91..dbcd2619c 100644 --- a/winsup/cygwin/fhandler_socket.cc +++ b/winsup/cygwin/fhandler_socket.cc @@ -1664,8 +1664,8 @@ inline ssize_t fhandler_socket::send_internal (struct _WSAMSG *wsamsg, int flags) { ssize_t res = 0; - DWORD ret = 0, err = 0, sum = 0, off = 0; - WSABUF buf; + DWORD ret = 0, err = 0, sum = 0; + WSABUF out_buf[wsamsg->dwBufferCount]; bool use_sendmsg = false; DWORD wait_flags = flags & MSG_DONTWAIT; bool nosignal = !!(flags & MSG_NOSIGNAL); @@ -1673,19 +1673,41 @@ fhandler_socket::send_internal (struct _WSAMSG *wsamsg, int flags) flags &= (MSG_OOB | MSG_DONTROUTE); if (wsamsg->Control.len > 0) use_sendmsg = true; - for (DWORD i = 0; i < wsamsg->dwBufferCount; - off >= wsamsg->lpBuffers[i].len && (++i, off = 0)) - { - /* CV 2009-12-02: Don't split datagram messages. */ - /* FIXME: Look for a way to split a message into the least number of - pieces to minimize the number of WsaSendTo calls. */ + /* Workaround for MSDN KB 823764: Split a message into chunks <= SO_SNDBUF. + in_idx is the index of the current lpBuffers from the input wsamsg buffer. + in_off is used to keep track of the next byte to write from a wsamsg + buffer which only gets partially written. */ + for (DWORD in_idx = 0, in_off = 0; + in_idx < wsamsg->dwBufferCount; + in_off >= wsamsg->lpBuffers[in_idx].len && (++in_idx, in_off = 0)) + { + /* Split a message into the least number of pieces to minimize the + number of WsaSendTo calls. Don't split datagram messages (bad idea). + out_idx is the index of the next buffer in the out_buf WSABUF, + also the number of buffers given to WSASendTo. + out_len is the number of bytes in the buffers given to WSASendTo. + Don't split datagram messages (very bad idea). */ + DWORD out_idx = 0; + DWORD out_len = 0; if (get_socket_type () == SOCK_STREAM) { - buf.buf = wsamsg->lpBuffers[i].buf + off; - buf.len = wsamsg->lpBuffers[i].len - off; - /* See net.cc:fdsock() and MSDN KB 823764 */ - if (buf.len >= (unsigned) wmem ()) - buf.len = (unsigned) wmem (); + do + { + out_buf[out_idx].buf = wsamsg->lpBuffers[in_idx].buf + in_off; + out_buf[out_idx].len = wsamsg->lpBuffers[in_idx].len - in_off; + out_len += out_buf[out_idx].len; + out_idx++; + } + while (out_len < (unsigned) wmem () + && (in_off = 0, ++in_idx < wsamsg->dwBufferCount)); + /* Tweak len of the last out_buf buffer so the entire number of bytes + is less than wmem (). */ + if (out_len > (unsigned) wmem ()) + out_buf[out_idx - 1].len -= out_len - (unsigned) wmem (); + /* Add the bytes written from the current last buffer to in_off, + so in_off points to the next byte to be written from that buffer, + or beyond which lets the outper loop skip to the next buffer. */ + in_off += out_buf[out_idx - 1].len; } do @@ -1693,7 +1715,7 @@ fhandler_socket::send_internal (struct _WSAMSG *wsamsg, int flags) if (use_sendmsg) res = WSASendMsg (get_socket (), wsamsg, flags, &ret, NULL, NULL); else if (get_socket_type () == SOCK_STREAM) - res = WSASendTo (get_socket (), &buf, 1, &ret, flags, + res = WSASendTo (get_socket (), out_buf, out_idx, &ret, flags, wsamsg->name, wsamsg->namelen, NULL, NULL); else res = WSASendTo (get_socket (), wsamsg->lpBuffers, @@ -1711,9 +1733,13 @@ fhandler_socket::send_internal (struct _WSAMSG *wsamsg, int flags) if (!res) { - off += ret; sum += ret; - if (get_socket_type () != SOCK_STREAM) + /* For streams, return to application if the number of bytes written + is less than the number of bytes we intended to write in a single + call to WSASendTo. Otherwise we would have to add code to + backtrack in the input buffers, which is questionable. There was + probably a good reason we couldn't write more. */ + if (get_socket_type () != SOCK_STREAM || ret < out_len) break; } else if (is_nonblocking () || err != WSAEWOULDBLOCK) -- 2.43.5