On Wednesday, 28 March 2007 at 09:55, Marc MERLIN wrote: > [executive summary: each color index ~h rule does not cause any stats in the > Maildir/.snd/ directory when I _open_ the folder (apparently, the header cache > is used for that). However each of my sent messages is stated and opened four > times when mutt resyncs the already opened folder, that is 4 times per ~h line > that is processed] [executive summary: the resync calls mutt_set_flag on every single message in the maildir, on the assumption that the call is free if flags haven't actually changed. It turns out this call can be very expensive, because it calls mutt_set_header_color every single time. That's four calls (one for each maildir flag) per message.] > In hindsight, it's a bit obvious now, but the rules in question that > caused the dramatic slowdown, are lines like this (I had 12) > color index black brightyellow '~h List-Id:.*bugtraq' > color index brightblue black '~h hits\=3' > color index brightblue brightyellow '~h post\ from' > > mutt can process ~f, ~C and ~p from the header cache, but not ~h. Or at > least it can't do it on resync apparently (unless the colors are cached > in the header cache file, which makes for a fast open, but a slow resync?) > > There is still a clear problem. Just adding a single: > > color index black brightyellow '~h List-Id:.*bugtraq' > > causes 4 stats and opens for _each_ message: > > If I add a second ~h line, I get 8 fstat and open calls _per_ message. > Now, that explains why I was seeing 5mn rsync times. > > The unanswered questions are: > 1) why can mutt use the header cache to run the ~h rules when the folder > is opened, but not when it's resynced it could, if it avoided calling mutt_set_flag when flags haven't changed. > 2) 4 fstat and open calls per message in the folder? That's obviously at > least 3 too many, but most likely 4 times too many. yep. I've ignored this for now. > 3) Why does adding a message to a folder cause mutt to ignore its entire > cache and rebuild a brand new one at resync time, scanning all messages > multiple times, when at folder open time, mutt looks smart enough to use > the cache for already cached messages, and only open/index/scan whatever > few messages that weren't in the cache already? mutt_set_flag is dumb. > Ok, we made some headway. Is that enough to get the code fixed? :) I think these changes (pushed a few minutes ago) should do the trick. Let us know how it goes.
# HG changeset patch
# User Brendan Cully <brendan@xxxxxxxxxx>
# Date 1176425343 25200
# Node ID 7447c7f9ab8dc8d5cda84f36dd5d744e2f959572
# Parent 3ee4effbb8e8aa5c5f4628fecf17ad057251493c
Only update header color in mutt_set_flag if flag has changed (see #1216, #1931)
diff -r 3ee4effbb8e8 -r 7447c7f9ab8d ChangeLog
--- a/ChangeLog Thu Apr 12 11:54:05 2007 -0700
+++ b/ChangeLog Thu Apr 12 17:49:03 2007 -0700
@@ -1,4 +1,8 @@ 2007-04-12 09:16 -0700 Brendan Cully <
-2007-04-12 09:16 -0700 Brendan Cully <brendan@xxxxxxxxxx> (eb5db1cd5251)
+2007-04-12 11:54 -0700 Brendan Cully <brendan@xxxxxxxxxx> (3ee4effbb8e8)
+
+ * init.h: Make $header_cache_compress default to set
+
+ * imap/message.c: Simplify IMAP message fetch loop slightly
* imap/message.c: Handle a missing or corrupted header cache entry
(closes #2676) If imap_hcache_get fails, stop parsing the header
diff -r 3ee4effbb8e8 -r 7447c7f9ab8d flags.c
--- a/flags.c Thu Apr 12 11:54:05 2007 -0700
+++ b/flags.c Thu Apr 12 17:49:03 2007 -0700
@@ -35,6 +35,7 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
int deleted = ctx->deleted;
int tagged = ctx->tagged;
int flagged = ctx->flagged;
+ int update = 0;
if (ctx->readonly && flag != M_TAG)
return; /* don't modify anything if we are read-only */
@@ -51,6 +52,7 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
if (!h->deleted && !ctx->readonly)
{
h->deleted = 1;
+ update = 1;
if (upd_ctx) ctx->deleted++;
#ifdef USE_IMAP
/* deleted messages aren't treated as changed elsewhere so that the
@@ -66,6 +68,7 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
else if (h->deleted)
{
h->deleted = 0;
+ update = 1;
if (upd_ctx) ctx->deleted--;
#ifdef USE_IMAP
/* see my comment above */
@@ -97,6 +100,7 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
{
if (h->read || h->old)
{
+ update = 1;
h->old = 0;
if (upd_ctx) ctx->new++;
if (h->read)
@@ -110,6 +114,7 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
}
else if (!h->read)
{
+ update = 1;
if (!h->old)
if (upd_ctx) ctx->new--;
h->read = 1;
@@ -128,6 +133,7 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
{
if (!h->old)
{
+ update = 1;
h->old = 1;
if (!h->read)
if (upd_ctx) ctx->new--;
@@ -137,6 +143,7 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
}
else if (h->old)
{
+ update = 1;
h->old = 0;
if (!h->read)
if (upd_ctx) ctx->new++;
@@ -154,6 +161,7 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
{
if (!h->read)
{
+ update = 1;
h->read = 1;
if (upd_ctx) ctx->unread--;
if (!h->old)
@@ -164,6 +172,7 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
}
else if (h->read)
{
+ update = 1;
h->read = 0;
if (upd_ctx) ctx->unread++;
if (!h->old)
@@ -182,6 +191,7 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
{
if (!h->replied)
{
+ update = 1;
h->replied = 1;
if (!h->read)
{
@@ -196,6 +206,7 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
}
else if (h->replied)
{
+ update = 1;
h->replied = 0;
h->changed = 1;
if (upd_ctx) ctx->changed = 1;
@@ -211,6 +222,7 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
{
if (!h->flagged)
{
+ update = 1;
h->flagged = bf;
if (upd_ctx) ctx->flagged++;
h->changed = 1;
@@ -219,6 +231,7 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
}
else if (h->flagged)
{
+ update = 1;
h->flagged = 0;
if (upd_ctx) ctx->flagged--;
h->changed = 1;
@@ -231,19 +244,22 @@ void _mutt_set_flag (CONTEXT *ctx, HEADE
{
if (!h->tagged)
{
+ update = 1;
h->tagged = 1;
if (upd_ctx) ctx->tagged++;
}
}
else if (h->tagged)
{
+ update = 1;
h->tagged = 0;
if (upd_ctx) ctx->tagged--;
}
break;
}
- mutt_set_header_color(ctx, h);
+ if (update)
+ mutt_set_header_color(ctx, h);
/* if the message status has changed, we need to invalidate the cached
* search results so that any future search will match the current status
# HG changeset patch
# User Brendan Cully <brendan@xxxxxxxxxx>
# Date 1176425731 25200
# Node ID 73894f3f1943c01e43c632ea3b65c1300d1dcc8e
# Parent 7447c7f9ab8dc8d5cda84f36dd5d744e2f959572
Only call mutt_set_flag when necessary when checking for maildir changes (fixes
#1216)
diff -r 7447c7f9ab8d -r 73894f3f1943 mh.c
--- a/mh.c Thu Apr 12 17:49:03 2007 -0700
+++ b/mh.c Thu Apr 12 17:55:31 2007 -0700
@@ -1726,16 +1726,19 @@ static void maildir_update_flags (CONTEX
*/
int context_changed = ctx->changed;
- /* user didn't modify this message. alter the flags to
- * match the current state on disk. This may not actually
- * do anything, but we can't tell right now. mutt_set_flag()
- * will just ignore the call if the status bits are
- * already properly set.
- */
- mutt_set_flag (ctx, o, M_FLAG, n->flagged);
- mutt_set_flag (ctx, o, M_REPLIED, n->replied);
- mutt_set_flag (ctx, o, M_READ, n->read);
- mutt_set_flag (ctx, o, M_OLD, n->old);
+ /* user didn't modify this message. alter the flags to match the
+ * current state on disk. This may not actually do
+ * anything. mutt_set_flag() will just ignore the call if the status
+ * bits are already properly set, but it is still faster not to pass
+ * through it */
+ if (o->flagged != n->flagged)
+ mutt_set_flag (ctx, o, M_FLAG, n->flagged);
+ if (o->replied != n->replied)
+ mutt_set_flag (ctx, o, M_REPLIED, n->replied);
+ if (o->read != n->read)
+ mutt_set_flag (ctx, o, M_READ, n->read);
+ if (o->old != n->old)
+ mutt_set_flag (ctx, o, M_OLD, n->old);
/* mutt_set_flag() will set this, but we don't need to
* sync the changes we made because we just updated the
Attachment:
pgp6wIxLd3itD.pgp
Description: PGP signature