Introduce $imap_pipeline_depth.

This lets users control the number of commands that mutt will queue up
before sending them to the server. Setting this to 0 disables
pipelining, which should close #2892.

diff --git a/ChangeLog b/ChangeLog
index bee69d1a8151c8c275864be0e0cdb56fb8983f2b..06a209fff50d74c3d2f9226ee18c34cca9cacdfa 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,19 +1,30 @@
-2008-08-24 17:06 +0200  Rocco Rutte  <pdmef@gmx.net>  (fec35f9ad30f)
+2008-08-25 00:16 -0700  Brendan Cully  <brendan@kublai.com>  (53d9210aa4ee)
 
-       * doc/manual.xml.head: Manual: Fix typo
+       * imap/command.c, imap/imap.c, imap/imap_private.h, imap/message.c:
+       Rework IMAP command queueing to allow pipelining to be disabled.
+       IDLE handling has been better abstracted, and there are fewer entry
+       points to the IMAP command issuing machinery. Any commands that are
+       simply queued may be executed whenever the pipeline fills, instead
+       of requiring explicit handling in the caller.
 
-2008-08-24 17:03 +0200  Rocco Rutte  <pdmef@gmx.net>  (a2fc5e7c77ae)
+       Tested on my Cyrus server, but I wouldn't be surprise if this causes
+       new problems.
 
-       * ChangeLog, doc/manual.xml.head: Manual: Fix style and typos. Noted
-       by Vincent Lefevre, see #3109.
+2008-08-24 20:01 +0200  Rocco Rutte  <pdmef@gmx.net>  (045c5942e1ad)
+
+       * doc/manual.xml.head: Manual: Fix DTD validation error and
+       message header display section
+
+       * doc/manual.xml.head: Manual: Fix typo
+
+       * doc/manual.xml.head: Manual: Fix style and typos. Noted by
+       Vincent Lefevre, see #3109.
 
 2008-08-23 15:25 -0700  Brendan Cully  <brendan@kublai.com>  (1f9849496bc2)
 
        * main.c: Whitespace cleanup
 
-2008-08-23 15:21 -0700  Brendan Cully  <brendan@kublai.com>  (63efed810906)
-
-       * ChangeLog, main.c: Fix infinite loop with "mutt -", introduced in
+       * main.c: Fix infinite loop with "mutt -", introduced in
        [31c9e9727d42]. Treats - as a non-option argument. It would be
        reasonable to treat it as an error instead.
 

diff --git a/UPDATING b/UPDATING
index 0da8c21c5b82aafc73b03440e95fea5469f5553a..41f7bf680216c64af75243eea72a9ce3d818de25 100644 (file)
--- a/UPDATING
+++ b/UPDATING
@@ -6,6 +6,8 @@ The keys used are:
 
 hg tip:
 
+  + $imap_pipeline_depth controls the number of commands that mutt can issue
+    to an IMAP server before it must collect the responses
   + $ssl_client_cert available with gnutls as well as openssl
   + 'mime_lookup application/octet-stream' added to system Muttrc
 

diff --git a/globals.h b/globals.h
index efecd8cf58aa13315c160847f9c32ac9283b2d5a..4921daa182fb793bae0c04f5033456b49a48e860 100644 (file)
--- a/globals.h
+++ b/globals.h
@@ -209,6 +209,7 @@ WHERE short ScoreThresholdFlag;
 
 #ifdef USE_IMAP
 WHERE short ImapKeepalive;
+WHERE short ImapPipelineDepth;
 #endif
 
 /* flags for received signals */

diff --git a/imap/command.c b/imap/command.c
index 412f8f0665e4383e21b5222bde6dd7130429c244..95269bbe7dc078d1cb1b82a09a4e3e34f336ab01 100644 (file)
--- a/imap/command.c
+++ b/imap/command.c
@@ -157,7 +157,7 @@ int imap_cmd_step (IMAP_DATA* idata)
        if (!stillrunning)
        {
          /* first command in queue has finished - move queue pointer up */
-         idata->lastcmd = (idata->lastcmd + 1) % IMAP_PIPELINE_DEPTH;
+         idata->lastcmd = (idata->lastcmd + 1) % idata->cmdslots;
        }
        cmd->state = cmd_status (idata->buf);
        /* bogus - we don't know which command result to return here. Caller
@@ -168,7 +168,7 @@ int imap_cmd_step (IMAP_DATA* idata)
        stillrunning++;
     }
 
-    c = (c + 1) % IMAP_PIPELINE_DEPTH;
+    c = (c + 1) % idata->cmdslots;
   }
   while (c != idata->nextcmd);
 
@@ -308,7 +308,7 @@ int imap_cmd_idle (IMAP_DATA* idata)
 
 static int cmd_queue_full (IMAP_DATA* idata)
 {
-  if ((idata->nextcmd + 1) % IMAP_PIPELINE_DEPTH == idata->lastcmd)
+  if ((idata->nextcmd + 1) % idata->cmdslots == idata->lastcmd)
     return 1;
 
   return 0;
@@ -327,7 +327,7 @@ static IMAP_COMMAND* cmd_new (IMAP_DATA* idata)
   }
 
   cmd = idata->cmds + idata->nextcmd;
-  idata->nextcmd = (idata->nextcmd + 1) % IMAP_PIPELINE_DEPTH;
+  idata->nextcmd = (idata->nextcmd + 1) % idata->cmdslots;
 
   snprintf (cmd->seq, sizeof (cmd->seq), "a%04u", idata->seqno++);
   if (idata->seqno > 9999)

diff --git a/imap/imap.c b/imap/imap.c
index 5b080a8e7d1fe12d5091a6c6d0e9d32c136f712b..fa778030bcd536be7bf92d09da37ad03dc42b105 100644 (file)
--- a/imap/imap.c
+++ b/imap/imap.c
@@ -486,7 +486,7 @@ void imap_close_connection(IMAP_DATA* idata)
   mutt_socket_close (idata->conn);
   idata->state = IMAP_DISCONNECTED;
   idata->seqno = idata->nextcmd = idata->lastcmd = idata->status = 0;
-  memset (idata->cmds, 0, sizeof (IMAP_COMMAND) * IMAP_PIPELINE_DEPTH);
+  memset (idata->cmds, 0, sizeof (IMAP_COMMAND) * idata->cmdslots);
 }
 
 /* imap_get_flags: Make a simple list out of a FLAGS response.

diff --git a/imap/imap_private.h b/imap/imap_private.h
index adfe242e5c62048c6c272735b05c8f334793bdf5..330136fd276cbc0bb973d15000216a8a2e5bbde1 100644 (file)
--- a/imap/imap_private.h
+++ b/imap/imap_private.h
@@ -54,10 +54,6 @@
 /* number of entries in the hash table */
 #define IMAP_CACHE_LEN 10
 
-/* number of commands that can be batched into a single request
- * ( - 1, for the easy way to detect ring buffer wrap) */
-#define IMAP_PIPELINE_DEPTH 15
-
 #define SEQLEN 5
 
 #define IMAP_REOPEN_ALLOW     (1<<0)
@@ -190,7 +186,8 @@ typedef struct
   void* cmddata;
 
   /* command queue */
-  IMAP_COMMAND cmds[IMAP_PIPELINE_DEPTH];
+  IMAP_COMMAND* cmds;
+  int cmdslots;
   int nextcmd;
   int lastcmd;
   BUFFER* cmdbuf;

diff --git a/imap/util.c b/imap/util.c
index 785d8f3b097e8c950fab7d3ca53c37e79464e341..eb3a1a16651a2f16d10dc0edbb091430bbd2b5ae 100644 (file)
--- a/imap/util.c
+++ b/imap/util.c
@@ -354,6 +354,13 @@ IMAP_DATA* imap_new_idata (void)
   if (!(idata->cmdbuf = mutt_buffer_init (NULL)))
     FREE (&idata);
 
+  idata->cmdslots = ImapPipelineDepth + 2;
+  if (!(idata->cmds = safe_calloc(idata->cmdslots, sizeof(*idata->cmds))))
+  {
+    mutt_buffer_free(&idata->cmdbuf);
+    FREE (&idata);
+  }
+
   return idata;
 }
 
@@ -369,6 +376,7 @@ void imap_free_idata (IMAP_DATA** idata)
   mutt_buffer_free(&(*idata)->cmdbuf);
   FREE (&(*idata)->buf);
   mutt_bcache_close (&(*idata)->bcache);
+  FREE (&(*idata)->cmds);
   FREE (idata);                /* __FREE_CHECKED__ */
 }
 

diff --git a/init.c b/init.c
index 22d3aeb97b376045f10ef813acc1bd1afb1c0efc..29e33fc06ee21d4e472dea0386d901955bdc4048 100644 (file)
--- a/init.c
+++ b/init.c
@@ -2116,6 +2116,13 @@ static int parse_set (BUFFER *tmp, BUFFER *s, unsigned long data, BUFFER *err)
        else
          *ptr = -*ptr;
       }
+#ifdef USE_IMAP
+      else if (mutt_strcmp (MuttVars[idx].option, "imap_pipeline_depth") == 0)
+      {
+        if (*ptr < 0)
+          *ptr = 0;
+      }
+#endif
     }
     else if (DTYPE (MuttVars[idx].type) == DT_QUAD)
     {

diff --git a/init.h b/init.h
index 41dd8b6ccb5b3bd276f991214f94b78ccd617b84..1e9ad3d617af84c105663368c8f12470d0efeb52 100644 (file)
--- a/init.h
+++ b/init.h
@@ -969,6 +969,17 @@ struct option_t MuttVars[] = {
   ** but can make closing an IMAP folder somewhat slower. This option
   ** exists to appease speed freaks.
   */
+  { "imap_pipeline_depth", DT_NUM,  R_NONE, UL &ImapPipelineDepth, 15 },
+  /*
+  ** .pp
+  ** Controls the number of IMAP commands that may be queued up before they
+  ** are sent to the server. A deeper pipeline reduces the amount of time
+  ** mutt must wait for the server, and can make IMAP servers feel much
+  ** more responsive. But not all servers correctly handle pipelined commands,
+  ** so if you have problems you might want to try setting this variable to 0.
+  ** .pp
+  ** \fBNote:\fP Changes to this variable have no effect on open connections.
+  */
   { "imap_servernoise",                DT_BOOL, R_NONE, OPTIMAPSERVERNOISE, 1 },
   /*
   ** .pp