From 1608d7b79c59f15f294ee19df6a1ca0d20d2aee3 Mon Sep 17 00:00:00 2001 From: Brendan Cully Date: Mon, 25 Aug 2008 00:52:17 -0700 Subject: [PATCH] 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. --- ChangeLog | 27 +++++++++++++++++++-------- UPDATING | 2 ++ globals.h | 1 + imap/command.c | 8 ++++---- imap/imap.c | 2 +- imap/imap_private.h | 7 ++----- imap/util.c | 8 ++++++++ init.c | 7 +++++++ init.h | 11 +++++++++++ 9 files changed, 55 insertions(+), 18 deletions(-) diff --git a/ChangeLog b/ChangeLog index bee69d1a8..06a209fff 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,19 +1,30 @@ -2008-08-24 17:06 +0200 Rocco Rutte (fec35f9ad30f) +2008-08-25 00:16 -0700 Brendan Cully (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 (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 (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 (1f9849496bc2) * main.c: Whitespace cleanup -2008-08-23 15:21 -0700 Brendan Cully (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 0da8c21c5..41f7bf680 100644 --- 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 efecd8cf5..4921daa18 100644 --- 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 412f8f066..95269bbe7 100644 --- 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 5b080a8e7..fa778030b 100644 --- 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 adfe242e5..330136fd2 100644 --- 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 785d8f3b0..eb3a1a166 100644 --- 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 22d3aeb97..29e33fc06 100644 --- 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 41dd8b6cc..1e9ad3d61 100644 --- 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 -- 2.40.0