First cut at speeding up SFTP. Generic download-management code in

sftp.c, and psftp.c now uses that instead of going it alone. Should
in principle be easily installed in PSCP as well, but I haven't done
it yet; also it only handles downloads, not uploads, and finally it
doesn't yet properly calculate the correct number of parallel
requests to queue. Still, it's a start, and in my own tests it
seemed to perform as expected (download speed suddenly became
roughly what you'd expect from the available bandwidth, and
decreased by roughly the expected number of round-trip times).

git-svn-id: svn://svn.tartarus.org/sgt/putty@3468 cda61777-01e9-0310-a592-d414129be87e

diff --git a/psftp.c b/psftp.c
index 5389659..9e697d5 100644 (file)
--- a/psftp.c
+++ b/psftp.c
@@ -370,6 +370,7 @@ int sftp_general_get(struct sftp_command *cmd, int restart)
     struct fxp_handle *fh;
     struct sftp_packet *pktin;
     struct sftp_request *req, *rreq;
+    struct fxp_xfer *xfer;
     char *fname, *outfname;
     uint64 offset;
     FILE *fp;
@@ -439,41 +440,43 @@ int sftp_general_get(struct sftp_command *cmd, int restart)
      * thus put up a progress bar.
      */
     ret = 1;
-    while (1) {
-       char buffer[4096];
-       int len;
+    xfer = xfer_download_init(fh, offset);
+    while (!xfer_download_done(xfer)) {
+       void *vbuf;
+       int ret, len;
        int wpos, wlen;
 
-       sftp_register(req = fxp_read_send(fh, offset, sizeof(buffer)));
-       rreq = sftp_find_request(pktin = sftp_recv());
-       assert(rreq == req);
-       len = fxp_read_recv(pktin, rreq, buffer, sizeof(buffer));
+       xfer_download_queue(xfer);
+       pktin = sftp_recv();
+       ret = xfer_download_gotpkt(xfer, pktin);
 
-       if ((len == -1 && fxp_error_type() == SSH_FX_EOF) || len == 0)
-           break;
-       if (len == -1) {
-           printf("error while reading: %s\n", fxp_error());
-           ret = 0;
-           break;
+       if (ret < 0) {
+            printf("error while reading: %s\n", fxp_error());
+            ret = 0;
        }
 
-       wpos = 0;
-       while (wpos < len) {
-           wlen = fwrite(buffer, 1, len - wpos, fp);
-           if (wlen <= 0) {
-               printf("error while writing local file\n");
+       while (xfer_download_data(xfer, &vbuf, &len)) {
+           unsigned char *buf = (unsigned char *)vbuf;
+
+           wpos = 0;
+           while (wpos < len) {
+               wlen = fwrite(buf + wpos, 1, len - wpos, fp);
+               if (wlen <= 0) {
+                   printf("error while writing local file\n");
+                   ret = 0;
+                   xfer_set_error(xfer);
+               }
+               wpos += wlen;
+           }
+           if (wpos < len) {          /* we had an error */
                ret = 0;
-               break;
+               xfer_set_error(xfer);
            }
-           wpos += wlen;
        }
-       if (wpos < len) {              /* we had an error */
-           ret = 0;
-           break;
-       }
-       offset = uint64_add32(offset, len);
     }
 
+    xfer_cleanup(xfer);
+
     fclose(fp);
 
     sftp_register(req = fxp_close_send(fh));

diff --git a/sftp.c b/sftp.c
index b9dc44b..fef207f 100644 (file)
--- a/sftp.c
+++ b/sftp.c
@@ -6,6 +6,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <assert.h>
+#include <limits.h>
 
 #include "misc.h"
 #include "int64.h"
@@ -257,6 +258,7 @@ struct sftp_packet *sftp_recv(void)
 struct sftp_request {
     unsigned id;
     int registered;
+    void *userdata;
 };
 
 static int sftp_reqcmp(void *av, void *bv)
@@ -327,6 +329,7 @@ static struct sftp_request *sftp_alloc_request(void)
     r = snew(struct sftp_request);
     r->id = low + 1 + REQUEST_ID_OFFSET;
     r->registered = 0;
+    r->userdata = NULL;
     add234(sftp_requests, r);
     return r;
 }
@@ -1018,3 +1021,239 @@ void fxp_free_name(struct fxp_name *name)
     sfree(name->longname);
     sfree(name);
 }
+
+/*
+ * Store user data in an sftp_request structure.
+ */
+void *fxp_get_userdata(struct sftp_request *req)
+{
+    return req->userdata;
+}
+
+void fxp_set_userdata(struct sftp_request *req, void *data)
+{
+    req->userdata = data;
+}
+
+/*
+ * A wrapper to go round fxp_read_* and fxp_write_*, which manages
+ * the queueing of multiple read/write requests.
+ */
+
+struct req {
+    char *buffer;
+    int len, retlen, complete;
+    uint64 offset;
+    struct req *next, *prev;
+};
+
+struct fxp_xfer {
+    uint64 offset, furthestdata, filesize;
+    int nreqs, req_max, eof, err;
+    struct fxp_handle *fh;
+    struct req *head, *tail;
+};
+
+static struct fxp_xfer *xfer_init(struct fxp_handle *fh, uint64 offset)
+{
+    struct fxp_xfer *xfer = snew(struct fxp_xfer);
+
+    xfer->fh = fh;
+    xfer->offset = offset;
+    xfer->head = xfer->tail = NULL;
+    xfer->nreqs = 0;
+    xfer->req_max = 4;                /* FIXME: set properly! */
+    xfer->err = 0;
+    xfer->filesize = uint64_make(ULONG_MAX, ULONG_MAX);
+    xfer->furthestdata = uint64_make(0, 0);
+
+    return xfer;
+}
+
+int xfer_download_done(struct fxp_xfer *xfer)
+{
+    /*
+     * We're finished if we've seen EOF _and_ there are no
+     * outstanding requests.
+     */
+    return (xfer->eof || xfer->err) && !xfer->head;
+}
+
+void xfer_download_queue(struct fxp_xfer *xfer)
+{
+    while (xfer->nreqs < xfer->req_max && !xfer->eof) {
+       /*
+        * Queue a new read request.
+        */
+       struct req *rr;
+       struct sftp_request *req;
+
+       rr = snew(struct req);
+       rr->offset = xfer->offset;
+       rr->complete = 0;
+       if (xfer->tail) {
+           xfer->tail->next = rr;
+           rr->prev = xfer->tail;
+       } else {
+           xfer->head = rr;
+           rr->prev = NULL;
+       }
+       xfer->tail = rr;
+       rr->next = NULL;
+
+       rr->len = 4096;
+       rr->buffer = snewn(rr->len, char);
+       sftp_register(req = fxp_read_send(xfer->fh, rr->offset, rr->len));
+       fxp_set_userdata(req, rr);
+
+       xfer->offset = uint64_add32(xfer->offset, rr->len);
+       xfer->nreqs++;
+
+#ifdef DEBUG_DOWNLOAD
+       { char buf[40]; uint64_decimal(rr->offset, buf); printf("queueing read request %p at %s\n", rr, buf); }
+#endif
+    }
+}
+
+struct fxp_xfer *xfer_download_init(struct fxp_handle *fh, uint64 offset)
+{
+    struct fxp_xfer *xfer = xfer_init(fh, offset);
+
+    xfer->eof = FALSE;
+    xfer_download_queue(xfer);
+
+    return xfer;
+}
+
+int xfer_download_gotpkt(struct fxp_xfer *xfer, struct sftp_packet *pktin)
+{
+    struct sftp_request *rreq;
+    struct req *rr;
+
+    rreq = sftp_find_request(pktin);
+    rr = (struct req *)fxp_get_userdata(rreq);
+    if (!rr)
+       return 0;                      /* this packet isn't ours */
+    rr->retlen = fxp_read_recv(pktin, rreq, rr->buffer, rr->len);
+#ifdef DEBUG_DOWNLOAD
+    printf("read request %p has returned [%d]\n", rr, rr->retlen);
+#endif
+
+    if ((rr->retlen < 0 && fxp_error_type()==SSH_FX_EOF) || rr->retlen == 0) {
+       xfer->eof = TRUE;
+       rr->complete = -1;
+#ifdef DEBUG_DOWNLOAD
+       printf("setting eof\n");
+#endif
+    } else if (rr->retlen < 0) {
+       /* some error other than EOF; signal it back to caller */
+       return -1;
+    }
+
+    rr->complete = 1;
+
+    /*
+     * Special case: if we have received fewer bytes than we
+     * actually read, we should do something. For the moment I'll
+     * just throw an ersatz FXP error to signal this; the SFTP
+     * draft I've got says that it can't happen except on special
+     * files, in which case seeking probably has very little
+     * meaning and so queueing an additional read request to fill
+     * up the gap sounds like the wrong answer. I'm not sure what I
+     * should be doing here - if it _was_ a special file, I suspect
+     * I simply shouldn't have been queueing multiple requests in
+     * the first place...
+     */
+    if (rr->retlen > 0 && uint64_compare(xfer->furthestdata, rr->offset) < 0) {
+       xfer->furthestdata = rr->offset;
+#ifdef DEBUG_DOWNLOAD
+       { char buf[40];
+       uint64_decimal(xfer->furthestdata, buf);
+       printf("setting furthestdata = %s\n", buf); }
+#endif
+    }
+
+    if (rr->retlen < rr->len) {
+       uint64 filesize = uint64_add32(rr->offset,
+                                      (rr->retlen < 0 ? 0 : rr->retlen));
+#ifdef DEBUG_DOWNLOAD
+       { char buf[40];
+       uint64_decimal(filesize, buf);
+       printf("short block! trying filesize = %s\n", buf); }
+#endif
+       if (uint64_compare(xfer->filesize, filesize) > 0) {
+           xfer->filesize = filesize;
+#ifdef DEBUG_DOWNLOAD
+           printf("actually changing filesize\n");
+#endif     
+       }
+    }
+
+    if (uint64_compare(xfer->furthestdata, xfer->filesize) > 0) {
+       fxp_error_message = "received a short buffer from FXP_READ, but not"
+           " at EOF";
+       fxp_errtype = -1;
+       xfer_set_error(xfer);
+       return -1;
+    }
+
+    return 1;
+}
+
+void xfer_set_error(struct fxp_xfer *xfer)
+{
+    xfer->err = 1;
+}
+
+int xfer_download_data(struct fxp_xfer *xfer, void **buf, int *len)
+{
+    void *retbuf = NULL;
+    int retlen = 0;
+
+    /*
+     * Discard anything at the head of the rr queue with complete <
+     * 0; return the first thing with complete > 0.
+     */
+    while (xfer->head && xfer->head->complete && !retbuf) {
+       struct req *rr = xfer->head;
+
+       if (rr->complete > 0) {
+           retbuf = rr->buffer;
+           retlen = rr->retlen;
+#ifdef DEBUG_DOWNLOAD
+           printf("handing back data from read request %p\n", rr);
+#endif
+       }
+#ifdef DEBUG_DOWNLOAD
+       else
+           printf("skipping failed read request %p\n", rr);
+#endif
+
+       xfer->head = xfer->head->next;
+       if (xfer->head)
+           xfer->head->prev = NULL;
+       else
+           xfer->tail = NULL;
+       sfree(rr);
+       xfer->nreqs--;
+    }
+
+    if (retbuf) {
+       *buf = retbuf;
+       *len = retlen;
+       return 1;
+    } else
+       return 0;
+}
+
+void xfer_cleanup(struct fxp_xfer *xfer)
+{
+    struct req *rr;
+    while (xfer->head) {
+       rr = xfer->head;
+       xfer->head = xfer->head->next;
+       sfree(rr->buffer);
+       sfree(rr);
+    }
+    sfree(xfer);
+}

diff --git a/sftp.h b/sftp.h
index 77aa2d9..5f8b38b 100644 (file)
--- a/sftp.h
+++ b/sftp.h
@@ -207,6 +207,12 @@ struct fxp_name *fxp_dup_name(struct fxp_name *name);
 void fxp_free_name(struct fxp_name *name);
 
 /*
+ * Store user data in an sftp_request structure.
+ */
+void *fxp_get_userdata(struct sftp_request *req);
+void fxp_set_userdata(struct sftp_request *req, void *data);
+
+/*
  * These functions might well be temporary placeholders to be
  * replaced with more useful similar functions later. They form the
  * main dispatch loop for processing incoming SFTP responses.
@@ -214,3 +220,19 @@ void fxp_free_name(struct fxp_name *name);
 void sftp_register(struct sftp_request *req);
 struct sftp_request *sftp_find_request(struct sftp_packet *pktin);
 struct sftp_packet *sftp_recv(void);
+
+/*
+ * A wrapper to go round fxp_read_* and fxp_write_*, which manages
+ * the queueing of multiple read/write requests.
+ */
+
+struct fxp_xfer;
+
+struct fxp_xfer *xfer_download_init(struct fxp_handle *fh, uint64 offset);
+int xfer_download_done(struct fxp_xfer *xfer);
+void xfer_download_queue(struct fxp_xfer *xfer);
+int xfer_download_gotpkt(struct fxp_xfer *xfer, struct sftp_packet *pktin);
+int xfer_download_data(struct fxp_xfer *xfer, void **buf, int *len);
+
+void xfer_set_error(struct fxp_xfer *xfer);
+void xfer_cleanup(struct fxp_xfer *xfer);