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
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;
}
