Quoting Daniel Stenberg <daniel_at_haxx.se>:

>
> (curl-library should be a better list to discuss this on)

My apologies, the website said curl-users was for bugs, so thought I'd start
there since it affects the command line tool as well.

> Since you say the CPU is the bottle-neck, it could very well be the "double
> copying" => http://curl.haxx.se/mail/lib-2006-11/0125.html
>
> Ravi Pratap said he'd give it a shot to fix, but clearly he or anyone else
> have not done it yet.

Yes, this is the problem. I drew up a patch (to 7.16.0) below that doesn't use
the copy when pipelining is not active. I confirmed the problem is fixed. A few
comments:
- I can't test pipelining, someone else will have to verify this patch doesn't
break it. I'm not even sure IsPipeliningSession will be set correctly.
- patched line 506 "bytesfromsocket = MIN...." is the same check that is in
transfer.c:Curl_readwrite():316. It appears 322 always resets the value set in
316? I don't like duplicating stuff so something should be fixed here.
- It seems setting a recieve buffer <BUFSIZE wasn't working before (for
downloads) since that value was enforced on the above mentioned transfer.c:316
which was always changed. This patch may fix that too
- If you decide not to apply the patch, at least consider applying the fix to
MIN()

Yeah, I know it's slightly hackish, but I was trying to isolate the changes to
just that area. I'm not too thrilled with that function getting so big though.
The patch may need a little adjusting to fit any code-style rules you have.

Thanks -Matt

--- ../curl/curl-7.16.0/lib/sendf.c 2006-10-23 16:34:56.000000000 -0400
+++ curl-7.16.0/lib/sendf.c 2007-01-03 15:50:35.000000000 -0500
@@ -438,7 +438,7 @@
   return CURLE_OK;
 }

-#define MIN(a,b) (a < b ? a : b)
+#define MIN(a,b) ((a) < (b) ? (a) : (b))

 void Curl_read_rewind(struct connectdata *conn,
                       size_t extraBytesRead)
@@ -472,8 +472,10 @@
               ssize_t *n) /* amount bytes read */
 {
   ssize_t nread;
- size_t bytestocopy = MIN(conn->buf_len - conn->read_pos, sizerequested);
   size_t bytesfromsocket = 0;
+ char* BufferToFill = NULL;
+
+ bool IsPipeliningSession = (conn->data->multi &&
Curl_multi_canPipeline(conn->data->multi));

   /* Set 'num' to 0 or 1, depending on which socket that has been sent here.
      If it is the second socket, we set num to 1. Otherwise to 0. This lets
@@ -482,33 +484,41 @@

   *n=0; /* reset amount to zero */

- /* Copy from our master buffer first if we have some unread data there*/
- if (bytestocopy > 0) {
- memcpy(buf, conn->master_buffer + conn->read_pos, bytestocopy);
- conn->read_pos += bytestocopy;
- conn->bits.stream_was_rewound = FALSE;
-
- *n = (ssize_t)bytestocopy;
- return CURLE_OK;
- }
+ /* If session can pipeline, check connection buffer */
+ if(IsPipeliningSession) {
+ size_t bytestocopy = MIN(conn->buf_len - conn->read_pos, sizerequested);
+
+ /* Copy from our master buffer first if we have some unread data there*/
+ if (bytestocopy > 0) {
+ memcpy(buf, conn->master_buffer + conn->read_pos, bytestocopy);
+ conn->read_pos += bytestocopy;
+ conn->bits.stream_was_rewound = FALSE;

- /* If we come here, it means that there is no data to read from the buffer,
- * so we read from the socket */
- bytesfromsocket = MIN(sizerequested, sizeof(conn->master_buffer));
+ *n = (ssize_t)bytestocopy;
+ return CURLE_OK;
+ }
+ /* If we come here, it means that there is no data to read from the buffer,
+ * so we read from the socket */
+ bytesfromsocket = MIN(sizerequested, sizeof(conn->master_buffer));
+ BufferToFill = conn->master_buffer;
+ }
+ else {
+ bytesfromsocket = MIN(sizerequested, conn->data->set.buffer_size ?
conn->data->set.buffer_size : BUFSIZE);
+ BufferToFill = buf;
+ }

   if(conn->ssl[num].use) {
- nread = Curl_ssl_recv(conn, num, conn->master_buffer, bytesfromsocket);
+ nread = Curl_ssl_recv(conn, num, BufferToFill, bytesfromsocket);

     if(nread == -1)
       return -1; /* -1 from Curl_ssl_recv() means EWOULDBLOCK */
   }
   else {
     if(conn->sec_complete)
- nread = Curl_sec_read(conn, sockfd, conn->master_buffer,
+ nread = Curl_sec_read(conn, sockfd, BufferToFill,
                             bytesfromsocket);
     else
- nread = sread(sockfd, conn->master_buffer, bytesfromsocket);
-
+ nread = sread(sockfd, BufferToFill, bytesfromsocket);
     if(-1 == nread) {
       int err = Curl_sockerrno();
 #ifdef USE_WINSOCK
@@ -521,10 +531,11 @@
   }

   if (nread >= 0) {
- memcpy(buf, conn->master_buffer, nread);
-
- conn->buf_len = nread;
- conn->read_pos = nread;
+ if(IsPipeliningSession) {
+ memcpy(buf, conn->master_buffer, nread);
+ conn->buf_len = nread;
+ conn->read_pos = nread;
+ }
     *n = nread;
   }
Received on 2007-01-03