From 1fe264101af84f1faddfb0816df77ab69db89b42 Mon Sep 17 00:00:00 2001 From: Kevin McCarthy Date: Mon, 2 Jan 2017 18:08:17 -0800 Subject: [PATCH] Add a pattern_cache_t to speed up a few repeated matches. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Vincent Lefèvre reported experiencing an index display performance issue. This occurred with messages containing many recipients. He had many index color lines containing ~l. The ~l ended up being run over and over on these messages, resulting in a noticable slowdown displaying the index. This patch adds caching for just a few of the pattern operations (~l, ~u, ~p, ~P) that are potentially expensive and also don't have arguments. The caching is only enabled for operations repeatedly matching against the same message: color, hooks, scoring. The caching is fairly targeted, but isn't that invasive or complicated. --- curs_main.c | 12 ++++--- hook.c | 13 +++++-- mutt.h | 17 +++++++++ pattern.c | 101 +++++++++++++++++++++++++++++++++++++++++++--------- protos.h | 2 +- score.c | 4 ++- 6 files changed, 124 insertions(+), 25 deletions(-) diff --git a/curs_main.c b/curs_main.c index c1d4ae760..227f567da 100644 --- a/curs_main.c +++ b/curs_main.c @@ -376,7 +376,7 @@ static void update_index (MUTTMENU *menu, CONTEXT *ctx, int check, if (mutt_pattern_exec (ctx->limit_pattern, MUTT_MATCH_FULL_ADDRESS, - ctx, ctx->hdrs[j])) + ctx, ctx->hdrs[j], NULL)) { assert (ctx->vcount < ctx->msgcount); ctx->hdrs[j]->virtual = ctx->vcount; @@ -2441,15 +2441,19 @@ int mutt_index_menu (void) void mutt_set_header_color (CONTEXT *ctx, HEADER *curhdr) { COLOR_LINE *color; + pattern_cache_t cache; if (!curhdr) return; + memset (&cache, 0, sizeof (cache)); + for (color = ColorIndexList; color; color = color->next) - if (mutt_pattern_exec (color->color_pattern, MUTT_MATCH_FULL_ADDRESS, ctx, curhdr)) - { + if (mutt_pattern_exec (color->color_pattern, MUTT_MATCH_FULL_ADDRESS, ctx, curhdr, + &cache)) + { curhdr->pair = color->pair; return; - } + } curhdr->pair = ColorDefs[MT_COLOR_NORMAL]; } diff --git a/hook.c b/hook.c index d8f678a8b..0cb403440 100644 --- a/hook.c +++ b/hook.c @@ -360,6 +360,7 @@ void mutt_message_hook (CONTEXT *ctx, HEADER *hdr, int type) { BUFFER err, token; HOOK *hook; + pattern_cache_t cache; current_hook_type = type; @@ -367,13 +368,15 @@ void mutt_message_hook (CONTEXT *ctx, HEADER *hdr, int type) err.dsize = STRING; err.data = safe_malloc (err.dsize); mutt_buffer_init (&token); + memset (&cache, 0, sizeof (cache)); for (hook = Hooks; hook; hook = hook->next) { if(!hook->command) continue; if (hook->type & type) - if ((mutt_pattern_exec (hook->pattern, 0, ctx, hdr) > 0) ^ hook->rx.not) + if ((mutt_pattern_exec (hook->pattern, 0, ctx, hdr, &cache) > 0) ^ hook->rx.not) + { if (mutt_parse_rc_line (hook->command, &token, &err) != 0) { FREE (&token.data); @@ -384,6 +387,10 @@ void mutt_message_hook (CONTEXT *ctx, HEADER *hdr, int type) return; } + /* Executing arbitrary commands could affect the pattern results, + * so the cache has to be wiped */ + memset (&cache, 0, sizeof (cache)); + } } FREE (&token.data); FREE (&err.data); @@ -395,7 +402,9 @@ static int mutt_addr_hook (char *path, size_t pathlen, int type, CONTEXT *ctx, HEADER *hdr) { HOOK *hook; + pattern_cache_t cache; + memset (&cache, 0, sizeof (cache)); /* determine if a matching hook exists */ for (hook = Hooks; hook; hook = hook->next) { @@ -403,7 +412,7 @@ mutt_addr_hook (char *path, size_t pathlen, int type, CONTEXT *ctx, HEADER *hdr) continue; if (hook->type & type) - if ((mutt_pattern_exec (hook->pattern, 0, ctx, hdr) > 0) ^ hook->rx.not) + if ((mutt_pattern_exec (hook->pattern, 0, ctx, hdr, &cache) > 0) ^ hook->rx.not) { mutt_make_string (path, pathlen, hook->command, ctx, hdr); return 0; diff --git a/mutt.h b/mutt.h index 4d80f34b9..23129ccb5 100644 --- a/mutt.h +++ b/mutt.h @@ -874,6 +874,23 @@ typedef struct pattern_t } p; } pattern_t; +/* This is used when a message is repeatedly pattern matched against. + * e.g. for color, scoring, hooks. It caches a few of the potentially slow + * operations. + * Each entry has a value of 0 = unset, 1 = false, 2 = true + */ +typedef struct +{ + int list_all; /* ^~l */ + int list_one; /* ~l */ + int sub_all; /* ^~u */ + int sub_one; /* ~u */ + int pers_recip_all; /* ^~p */ + int pers_recip_one; /* ~p */ + int pers_from_all; /* ^~P */ + int pers_from_one; /* ~P */ +} pattern_cache_t; + /* ACL Rights */ enum { diff --git a/pattern.c b/pattern.c index f1d46699b..8462e4d17 100644 --- a/pattern.c +++ b/pattern.c @@ -995,19 +995,19 @@ pattern_t *mutt_pattern_comp (/* const */ char *s, int flags, BUFFER *err) } static int -perform_and (pattern_t *pat, pattern_exec_flag flags, CONTEXT *ctx, HEADER *hdr) +perform_and (pattern_t *pat, pattern_exec_flag flags, CONTEXT *ctx, HEADER *hdr, pattern_cache_t *cache) { for (; pat; pat = pat->next) - if (mutt_pattern_exec (pat, flags, ctx, hdr) <= 0) + if (mutt_pattern_exec (pat, flags, ctx, hdr, cache) <= 0) return 0; return 1; } static int -perform_or (struct pattern_t *pat, pattern_exec_flag flags, CONTEXT *ctx, HEADER *hdr) +perform_or (struct pattern_t *pat, pattern_exec_flag flags, CONTEXT *ctx, HEADER *hdr, pattern_cache_t *cache) { for (; pat; pat = pat->next) - if (mutt_pattern_exec (pat, flags, ctx, hdr) > 0) + if (mutt_pattern_exec (pat, flags, ctx, hdr, cache) > 0) return 1; return 0; } @@ -1094,7 +1094,7 @@ static int match_threadcomplete(struct pattern_t *pat, pattern_exec_flag flags, return 0; h = t->message; if(h) - if(mutt_pattern_exec(pat, flags, ctx, h)) + if(mutt_pattern_exec(pat, flags, ctx, h, NULL)) return 1; if(up && (a=match_threadcomplete(pat, flags, ctx, t->parent,1,1,1,0))) @@ -1108,17 +1108,44 @@ static int match_threadcomplete(struct pattern_t *pat, pattern_exec_flag flags, return 0; } -/* flags - MUTT_MATCH_FULL_ADDRESS match both personal and machine address */ + +/* Sets a value in the pattern_cache_t cache entry. + * Normalizes the "true" value to 2. */ +static void set_pattern_cache_value (int *cache_entry, int value) +{ + *cache_entry = value ? 2 : 1; +} + +/* Returns 1 if the cache value is set and has a true value. + * 0 otherwise (even if unset!) */ +static int get_pattern_cache_value (int cache_entry) +{ + return cache_entry == 2; +} + +static int is_pattern_cache_set (int cache_entry) +{ + return cache_entry != 0; +} + + +/* + * flags: MUTT_MATCH_FULL_ADDRESS - match both personal and machine address + * cache: For repeated matches against the same HEADER, passing in non-NULL will + * store some of the cacheable pattern matches in this structure. */ int -mutt_pattern_exec (struct pattern_t *pat, pattern_exec_flag flags, CONTEXT *ctx, HEADER *h) +mutt_pattern_exec (struct pattern_t *pat, pattern_exec_flag flags, CONTEXT *ctx, HEADER *h, + pattern_cache_t *cache) { + int result; + int *cache_entry; + switch (pat->op) { case MUTT_AND: - return (pat->not ^ (perform_and (pat->child, flags, ctx, h) > 0)); + return (pat->not ^ (perform_and (pat->child, flags, ctx, h, cache) > 0)); case MUTT_OR: - return (pat->not ^ (perform_or (pat->child, flags, ctx, h) > 0)); + return (pat->not ^ (perform_or (pat->child, flags, ctx, h, cache) > 0)); case MUTT_THREAD: return (pat->not ^ match_threadcomplete(pat->child, flags, ctx, h->thread, 1, 1, 1, 1)); case MUTT_ALL: @@ -1198,13 +1225,53 @@ mutt_pattern_exec (struct pattern_t *pat, pattern_exec_flag flags, CONTEXT *ctx, return (pat->not ^ match_adrlist (pat, flags & MUTT_MATCH_FULL_ADDRESS, 2, h->env->to, h->env->cc)); case MUTT_LIST: /* known list, subscribed or not */ - return (pat->not ^ mutt_is_list_cc (pat->alladdr, h->env->to, h->env->cc)); + if (cache) + { + cache_entry = pat->alladdr ? &cache->list_all : &cache->list_one; + if (!is_pattern_cache_set (*cache_entry)) + set_pattern_cache_value (cache_entry, + mutt_is_list_cc (pat->alladdr, h->env->to, h->env->cc)); + result = get_pattern_cache_value (*cache_entry); + } + else + result = mutt_is_list_cc (pat->alladdr, h->env->to, h->env->cc); + return (pat->not ^ result); case MUTT_SUBSCRIBED_LIST: - return (pat->not ^ mutt_is_list_recipient (pat->alladdr, h->env->to, h->env->cc)); + if (cache) + { + cache_entry = pat->alladdr ? &cache->sub_all : &cache->sub_one; + if (!is_pattern_cache_set (*cache_entry)) + set_pattern_cache_value (cache_entry, + mutt_is_list_recipient (pat->alladdr, h->env->to, h->env->cc)); + result = get_pattern_cache_value (*cache_entry); + } + else + result = mutt_is_list_recipient (pat->alladdr, h->env->to, h->env->cc); + return (pat->not ^ result); case MUTT_PERSONAL_RECIP: - return (pat->not ^ match_user (pat->alladdr, h->env->to, h->env->cc)); + if (cache) + { + cache_entry = pat->alladdr ? &cache->pers_recip_all : &cache->pers_recip_one; + if (!is_pattern_cache_set (*cache_entry)) + set_pattern_cache_value (cache_entry, + match_user (pat->alladdr, h->env->to, h->env->cc)); + result = get_pattern_cache_value (*cache_entry); + } + else + result = match_user (pat->alladdr, h->env->to, h->env->cc); + return (pat->not ^ result); case MUTT_PERSONAL_FROM: - return (pat->not ^ match_user (pat->alladdr, h->env->from, NULL)); + if (cache) + { + cache_entry = pat->alladdr ? &cache->pers_from_all : &cache->pers_from_one; + if (!is_pattern_cache_set (*cache_entry)) + set_pattern_cache_value (cache_entry, + match_user (pat->alladdr, h->env->from, NULL)); + result = get_pattern_cache_value (*cache_entry); + } + else + result = match_user (pat->alladdr, h->env->from, NULL); + return (pat->not ^ result); case MUTT_COLLAPSED: return (pat->not ^ (h->collapsed && h->num_hidden > 1)); case MUTT_CRYPT_SIGN: @@ -1364,7 +1431,7 @@ int mutt_pattern_func (int op, char *prompt) Context->hdrs[i]->limited = 0; Context->hdrs[i]->collapsed = 0; Context->hdrs[i]->num_hidden = 0; - if (mutt_pattern_exec (pat, MUTT_MATCH_FULL_ADDRESS, Context, Context->hdrs[i])) + if (mutt_pattern_exec (pat, MUTT_MATCH_FULL_ADDRESS, Context, Context->hdrs[i], NULL)) { Context->hdrs[i]->virtual = Context->vcount; Context->hdrs[i]->limited = 1; @@ -1380,7 +1447,7 @@ int mutt_pattern_func (int op, char *prompt) for (i = 0; i < Context->vcount; i++) { mutt_progress_update (&progress, i, -1); - if (mutt_pattern_exec (pat, MUTT_MATCH_FULL_ADDRESS, Context, Context->hdrs[Context->v2r[i]])) + if (mutt_pattern_exec (pat, MUTT_MATCH_FULL_ADDRESS, Context, Context->hdrs[Context->v2r[i]], NULL)) { switch (op) { @@ -1540,7 +1607,7 @@ int mutt_search_command (int cur, int op) { /* remember that we've already searched this message */ h->searched = 1; - if ((h->matched = (mutt_pattern_exec (SearchPattern, MUTT_MATCH_FULL_ADDRESS, Context, h) > 0))) + if ((h->matched = (mutt_pattern_exec (SearchPattern, MUTT_MATCH_FULL_ADDRESS, Context, h, NULL) > 0))) { mutt_clear_error(); if (msg && *msg) diff --git a/protos.h b/protos.h index def09ea5f..c29d26665 100644 --- a/protos.h +++ b/protos.h @@ -406,7 +406,7 @@ int mutt_wctoutf8 (char *s, unsigned int c, size_t buflen); #define new_pattern() safe_calloc(1, sizeof (pattern_t)) -int mutt_pattern_exec (struct pattern_t *pat, pattern_exec_flag flags, CONTEXT *ctx, HEADER *h); +int mutt_pattern_exec (struct pattern_t *pat, pattern_exec_flag flags, CONTEXT *ctx, HEADER *h, pattern_cache_t *); pattern_t *mutt_pattern_comp (/* const */ char *s, int flags, BUFFER *err); void mutt_check_simple (char *s, size_t len, const char *simple); void mutt_pattern_free (pattern_t **pat); diff --git a/score.c b/score.c index 50e9cb830..a2718ba1d 100644 --- a/score.c +++ b/score.c @@ -129,11 +129,13 @@ int mutt_parse_score (BUFFER *buf, BUFFER *s, unsigned long data, BUFFER *err) void mutt_score_message (CONTEXT *ctx, HEADER *hdr, int upd_ctx) { SCORE *tmp; + pattern_cache_t cache; + memset (&cache, 0, sizeof (cache)); hdr->score = 0; /* in case of re-scoring */ for (tmp = Score; tmp; tmp = tmp->next) { - if (mutt_pattern_exec (tmp->pat, 0, NULL, hdr) > 0) + if (mutt_pattern_exec (tmp->pat, 0, NULL, hdr, &cache) > 0) { if (tmp->exact || tmp->val == 9999 || tmp->val == -9999) { -- 2.40.0