Return-Path: Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 68790 invoked by uid 500); 6 Aug 2002 08:27:42 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Delivered-To: mailing list dev@apr.apache.org Received: (qmail 68776 invoked from network); 6 Aug 2002 08:27:41 -0000 Message-ID: <000f01c23d22$03b06730$96c0b0d0@WROWE04> From: "William A. Rowe, Jr." To: "Apache Portable Runtime Developers" Subject: [PATCH] win32 sendrecv.c cleanup Date: Tue, 6 Aug 2002 01:19:32 -0700 Organization: Covalent Technologies, Inc. MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_0005_01C23CE7.51AA9180" X-Priority: 3 X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook Express 6.00.2600.0000 X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2600.0000 X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N This is a multi-part message in MIME format. ------=_NextPart_000_0005_01C23CE7.51AA9180 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Folks, Please review the attached patch that eliminates the rather nasty allocations that can occur in two situations. One is in apr_sendv() when we have more than 50 items, now simply allocate the vectors on the stack [very portable across win32, from vc5 onwards.] The other simply allocates a static buffer for the apr_sendfile headers and trailers vector concatination, and when we exceed it, we simply sendv out the header or trailer as appropriate. If I hear no objections I'll commit in a day or two. Final question. We 'devolve' to 64kb per sendfile. There is a comment in there about exceeding the timeout. What on earth does unix do so special that it isn't a problem over on that side? A cluestick across my knuckles would be appreciated before I even think about attacking that code. Bill ------=_NextPart_000_0005_01C23CE7.51AA9180 Content-Type: text/plain; name="win32_send_mem.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="win32_send_mem.patch" Index: network_io/win32/sendrecv.c =================================================================== RCS file: /home/cvs/apr/network_io/win32/sendrecv.c,v retrieving revision 1.57 diff -u -r1.57 sendrecv.c --- network_io/win32/sendrecv.c 6 Aug 2002 06:50:17 -0000 1.57 +++ network_io/win32/sendrecv.c 6 Aug 2002 08:22:10 -0000 @@ -71,7 +71,6 @@ * bytes. */ #define MAX_SEGMENT_SIZE 65536 -#define WSABUF_ON_STACK 50 APR_DECLARE(apr_status_t) apr_send(apr_socket_t *sock, const char *buf, apr_size_t *len) @@ -138,26 +137,24 @@ apr_ssize_t rv; int i; DWORD dwBytes = 0; - WSABUF wsabuf[WSABUF_ON_STACK]; + WSABUF *wsabuf = _alloca(sizeof(WSABUF) * nvec); LPWSABUF pWsaBuf = wsabuf; - if (nvec > WSABUF_ON_STACK) { - pWsaBuf = (LPWSABUF) malloc(sizeof(WSABUF) * nvec); - if (!pWsaBuf) - return APR_ENOMEM; - } + if (!wsabuf) + return APR_ENOMEM; + for (i = 0; i < nvec; i++) { - pWsaBuf[i].buf = vec[i].iov_base; - pWsaBuf[i].len = vec[i].iov_len; + wsabuf[i].buf = vec[i].iov_base; + wsabuf[i].len = vec[i].iov_len; } #ifndef _WIN32_WCE - rv = WSASend(sock->socketdes, pWsaBuf, nvec, &dwBytes, 0, NULL, NULL); + rv = WSASend(sock->socketdes, wsabuf, nvec, &dwBytes, 0, NULL, NULL); if (rv == SOCKET_ERROR) { rc = apr_get_netos_error(); } #else for (i = 0; i < nvec; i++) { - rv = send(sock->socketdes, pWsaBuf[i].buf, pWsaBuf[i].len, 0); + rv = send(sock->socketdes, wsabuf[i].buf, wsabuf[i].len, 0); if (rv == SOCKET_ERROR) { rc = apr_get_netos_error(); break; @@ -165,9 +162,6 @@ dwBytes += rv; } #endif - if (nvec > WSABUF_ON_STACK) - free(pWsaBuf); - *nbytes = dwBytes; return rc; } @@ -213,12 +207,12 @@ } -static void collapse_iovec(char **buf, int *len, struct iovec *iovec, int numvec, apr_pool_t *p) +static apr_status_t collapse_iovec(char **off, apr_size_t *len, + struct iovec *iovec, int numvec, + char *buf, apr_size_t buflen) { - int ptr = 0; - if (numvec == 1) { - *buf = iovec[0].iov_base; + *off = iovec[0].iov_base; *len = iovec[0].iov_len; } else { @@ -227,13 +221,19 @@ *len += iovec[i].iov_len; } - *buf = apr_palloc(p, *len); /* Should this be a malloc? */ + if (*len > buflen) { + *len = 0; + return APR_INCOMPLETE; + } + + *off = buf; for (i = 0; i < numvec; i++) { - memcpy((char*)*buf + ptr, iovec[i].iov_base, iovec[i].iov_len); - ptr += iovec[i].iov_len; + memcpy(buf, iovec[i].iov_base, iovec[i].iov_len); + buf += iovec[i].iov_len; } } + return APR_SUCCESS; } @@ -274,7 +274,9 @@ int ptr = 0; int bytes_to_send; /* Bytes to send out of the file (not including headers) */ int disconnected = 0; + int sendv_trailers = 0; HANDLE wait_event; + char hdtrbuf[4096]; if (apr_os_level < APR_WIN_NT) { return APR_ENOTIMPL; @@ -313,8 +315,18 @@ /* Collapse the headers into a single buffer */ if (hdtr && hdtr->numheaders) { ptfb = &tfb; - collapse_iovec((char **)&ptfb->Head, &ptfb->HeadLength, hdtr->headers, - hdtr->numheaders, sock->cntxt); + nbytes = 0; + rv = collapse_iovec((char **)&ptfb->Head, &ptfb->HeadLength, + hdtr->headers, hdtr->numheaders, + hdtrbuf, sizeof(hdtrbuf)); + /* If not enough buffer, punt to sendv */ + if (rv == APR_INCOMPLETE) { + rv = apr_sendv(sock, hdtr->headers, hdtr->numheaders, &nbytes); + if (rv != APR_SUCCESS) + return rv; + *len += nbytes; + ptfb = NULL; + } } while (bytes_to_send) { @@ -327,11 +339,18 @@ /* Collapse the trailers into a single buffer */ if (hdtr && hdtr->numtrailers) { ptfb = &tfb; - collapse_iovec((char**) &ptfb->Tail, &ptfb->TailLength, - hdtr->trailers, hdtr->numtrailers, sock->cntxt); + rv = collapse_iovec((char**) &ptfb->Tail, &ptfb->TailLength, + hdtr->trailers, hdtr->numtrailers, + hdtrbuf + ptfb->HeadLength, + sizeof(hdtrbuf) - ptfb->HeadLength); + if (rv == APR_INCOMPLETE) { + /* If not enough buffer, punt to sendv, later */ + sendv_trailers = 1; + } } /* Disconnect the socket after last send */ - if (flags & APR_SENDFILE_DISCONNECT_SOCKET) { + if ((flags & APR_SENDFILE_DISCONNECT_SOCKET) + && !sendv_trailers) { dwFlags |= TF_REUSE_SOCKET; dwFlags |= TF_DISCONNECT; disconnected = 1; @@ -405,11 +424,19 @@ } if (status == APR_SUCCESS) { + if (sendv_trailers) { + rv = apr_sendv(sock, hdtr->trailers, hdtr->numtrailers, &nbytes); + if (rv != APR_SUCCESS) + return rv; + *len += nbytes; + } + + /* Mark the socket as disconnected, but do not close it. * Note: The application must have stored the socket prior to making * the call to apr_sendfile in order to either reuse it or close it. */ - if (flags & APR_SENDFILE_DISCONNECT_SOCKET) { + if (disconnected) { sock->disconnected = 1; sock->socketdes = INVALID_SOCKET; } ------=_NextPart_000_0005_01C23CE7.51AA9180--