<<< Date Index >>>     <<< Thread Index >>>

Re: Extremly slow browsing of folders



I shouldnt reply to my own mail I suppose but I have completed teh
"alternate" method from my previous mail

> 
> An alternate, more invasive, fix would be to build a distinct more
> persistent cache of looked up uids and access through that, this would
> reduce the lookups even further if all the folders belong to the same
> user/group (a very common occurrence)
> 

There has been a question of if this is "worthwhile" or that I should
use nscd (caching daemon). I am fully aware of (and use) these
daemons but there was still an issue (in my specific configuration
this severe performance issue exists, maybe I'm unlucky :-( )

just to be certain I parameterised this a bit more with an strace
(strace -r -ff -o mutttrace mutt)

when a browse view is displayed each line when printed with the
folder_format_str() function, uses both a getgrgid and a getpwuid
. The c library converts these into 26 syscalls for the getgrgid and
28 for the getpwuid so each line uses 54 system calls (which accounts
for some 0.3s on my system) these system calls are retrieving a the
group and password files to parse to find the relevant data and doing
a fair bit of work.

So 54 calls for each line and maybe 20 odd lines thats well over 1000
calls which seems to take an appreciable time even with a caching nscd
in front of my ldap services.

Note every time the cursors moved the item moved onto is reexamined also.

The attached patch reduces this overhead to a single getgrgid/getpwuid
call on my users typical mail dir this has reduced system load and
improved responsiveness to the extent the flow of user complaints I
have been receiving has stopped.

This is all the time I have for this so I hope its of some use to
someone. If not, oh well it worked for me ;-)

-- 
Regards Vincent

diff -urb mutt-1.5.6.orig/browser.c mutt-1.5.6/browser.c
--- mutt-1.5.6.orig/browser.c   2004-09-28 20:59:42.000000000 +0100
+++ mutt-1.5.6/browser.c        2004-09-28 20:56:00.000000000 +0100
@@ -49,6 +49,7 @@
 {
   struct folder_file *ff;
   int num;
+  struct browser_state *state;
 } FOLDER;
 
 static char LastDir[_POSIX_PATH_MAX] = "";
@@ -136,6 +137,97 @@
     return 0;
 }
 
+void
+init_group_passwd_cache(struct browser_state *state)
+{
+    struct group * gr;
+    struct passwd * pw;
+
+    state->grcache = safe_malloc (sizeof(struct group_cache));
+    state->grcache->gr = safe_malloc (sizeof(struct group));
+    state->grcache->grbuf = safe_malloc (sysconf(_SC_GETGR_R_SIZE_MAX));
+    state->grcache->gid=getgid();
+
+    getgrgid_r(state->grcache->gid,
+              state->grcache->gr,
+              state->grcache->grbuf,
+              sysconf(_SC_GETGR_R_SIZE_MAX),
+              &gr);
+
+    state->pwcache = safe_malloc (sizeof(struct passwd_cache));
+    state->pwcache->pw = safe_malloc (sizeof(struct passwd));
+    state->pwcache->pwbuf = safe_malloc (sysconf(_SC_GETPW_R_SIZE_MAX));
+    state->pwcache->uid=getuid();
+
+    getpwuid_r(state->pwcache->uid,
+              state->pwcache->pw,
+              state->pwcache->pwbuf,
+              sysconf(_SC_GETPW_R_SIZE_MAX),
+              &pw);
+
+}
+
+
+struct group *
+cached_getgrgid (struct browser_state *state, gid_t gid)
+{
+    struct group * gr;
+
+    /* check if cache in use */
+    if(state->grcache == NULL)
+       return getgrgid (gid);
+
+    /* check if the cached version is what we want */
+    if(state->grcache->gid == gid)
+       return state->grcache->gr;
+
+    /* cache miss, call getgrgid_r */
+    getgrgid_r(gid,
+              state->grcache->gr,
+              state->grcache->grbuf,
+              sysconf(_SC_GETGR_R_SIZE_MAX),
+              &gr);
+
+    /* invalidate cache if getgrgid_r failed */
+    if (gr)
+       state->grcache->gid = gid;
+    else
+       state->grcache->gid = -1;
+
+    return gr;
+
+}
+
+struct passwd *
+cached_getpwuid (struct browser_state *state, uid_t uid)
+{
+    struct passwd *pw;
+
+    /* check if cache in use */
+    if(state->pwcache == NULL)
+       return getpwuid (uid);
+
+    /* check if the cached version is what we want */
+    if(state->pwcache->uid == uid)
+       return state->pwcache->pw;
+
+    /* cache miss, update entry */
+    getpwuid_r(uid,
+              state->pwcache->pw,
+              state->pwcache->pwbuf,
+              sysconf(_SC_GETPW_R_SIZE_MAX),
+              &pw);
+
+    /* invalidate cache */
+    if (pw)
+       state->pwcache->uid = uid;
+    else
+       state->pwcache->uid = -1;
+
+    return pw;
+
+}
+
 static const char *
 folder_format_str (char *dest, size_t destlen, char op, const char *src,
                   const char *fmt, const char *ifstring, const char 
*elsestring,
@@ -218,7 +310,7 @@
     case 'g':
       if (folder->ff->st != NULL)
       {
-       if ((gr = getgrgid (folder->ff->st->st_gid)))
+       if ((gr = cached_getgrgid (folder->state, folder->ff->st->st_gid)))
          mutt_format_s (dest, destlen, fmt, gr->gr_name);
        else
        {
@@ -276,7 +368,7 @@
     case 'u':
       if (folder->ff->st != NULL)
       {
-       if ((pw = getpwuid (folder->ff->st->st_uid)))
+       if ((pw = cached_getpwuid (folder->state, folder->ff->st->st_uid)))
          mutt_format_s (dest, destlen, fmt, pw->pw_name);
        else
        {
@@ -315,7 +407,7 @@
     memset (&state->entry[state->entrylen], 0,
            sizeof (struct folder_file) * 256);
     if (m)
-      m->data = state->entry;
+      m->data = state;
   }
 
   if (mbuf && mbuf->magic == M_MAILDIR && mbuf->mtime)
@@ -349,7 +441,10 @@
   state->imap_browse = 0;
 #endif
   if (menu)
-    menu->data = state->entry;
+    menu->data = state;
+
+  init_group_passwd_cache(state);
+
 }
 
 static int examine_directory (MUTTMENU *menu, struct browser_state *state,
@@ -469,15 +564,17 @@
 
 int select_file_search (MUTTMENU *menu, regex_t *re, int n)
 {
-  return (regexec (re, ((struct folder_file *) menu->data)[n].name, 0, NULL, 
0));
+  return (regexec (re, ((struct browser_state *)menu->data)->entry[n].name, 0, 
NULL, 0));
 }
 
 void folder_entry (char *s, size_t slen, MUTTMENU *menu, int num)
 {
+  struct browser_state * state = (struct browser_state *)menu->data;
   FOLDER folder;
 
-  folder.ff = &((struct folder_file *) menu->data)[num];
+  folder.ff = &(state->entry[num]);
   folder.num = num;
+  folder.state= state;
   
   mutt_FormatString (s, slen, NONULL(FolderFormat), folder_format_str, 
       (unsigned long) &folder, M_FORMAT_ARROWCURSOR);
@@ -519,7 +616,7 @@
 
 int file_tag (MUTTMENU *menu, int n, int m)
 {
-  struct folder_file *ff = &(((struct folder_file *)menu->data)[n]);
+  struct folder_file *ff = &(((struct browser_state *)menu->data)->entry[n]);
   int ot;
   if (S_ISDIR (ff->mode) || (S_ISLNK (ff->mode) && link_is_dir (LastDir, 
ff->name)))
   {
@@ -637,7 +735,7 @@
   menu->make_entry = folder_entry;
   menu->search = select_file_search;
   menu->title = title;
-  menu->data = state.entry;
+  menu->data = &state;
   if (multiple)
     menu->tag = file_tag;
 
@@ -758,7 +856,7 @@
              init_state (&state, NULL);
              state.imap_browse = 1;
              imap_browse (LastDir, &state);
-             menu->data = state.entry;
+             menu->data = &state;
            }
            else
 #endif
@@ -869,7 +967,7 @@
          init_state (&state, NULL);
          state.imap_browse = 1;
          imap_browse (LastDir, &state);
-         menu->data = state.entry;
+         menu->data = &state;
          menu->current = 0; 
          menu->top = 0; 
          init_menu (&state, menu, title, sizeof (title), buffy);
@@ -939,7 +1037,7 @@
            init_state (&state, NULL);
            state.imap_browse = 1;
            imap_browse (LastDir, &state);
-           menu->data = state.entry;
+           menu->data = &state;
            menu->current = 0; 
            menu->top = 0; 
            init_menu (&state, menu, title, sizeof (title), buffy);
@@ -1018,7 +1116,7 @@
              init_state (&state, NULL);
              state.imap_browse = 1;
              imap_browse (LastDir, &state);
-             menu->data = state.entry;
+             menu->data = &state;
              init_menu (&state, menu, title, sizeof (title), buffy);
            }
            else
@@ -1103,7 +1201,7 @@
          init_state (&state, NULL);
          state.imap_browse = 1;
          imap_browse (LastDir, &state);
-         menu->data = state.entry;
+         menu->data = &state;
        }
 #endif
        else if (examine_directory (menu, &state, LastDir, prefix) == -1)
diff -urb mutt-1.5.6.orig/browser.h mutt-1.5.6/browser.h
--- mutt-1.5.6.orig/browser.h   2002-12-11 11:19:39.000000000 +0000
+++ mutt-1.5.6/browser.h        2004-09-28 19:59:30.000000000 +0100
@@ -20,6 +20,21 @@
 #ifndef _BROWSER_H
 #define _BROWSER_H 1
 
+struct group_cache
+{
+    gid_t gid; /* gid this entry coresponds to */
+    struct group *gr;
+    char * grbuf;
+};
+
+struct passwd_cache
+{
+    uid_t uid; /* uid this entry coresponds to */
+    struct passwd *pw;
+    char * pwbuf;
+};
+
+
 struct folder_file
 {
   mode_t mode;
@@ -46,6 +61,10 @@
   struct folder_file *entry;
   short entrylen; /* number of real entries */
   short entrymax;  /* max entry */
+
+  struct group_cache *grcache; /* cached group */
+  struct passwd_cache *pwcache; /* cached passwd */
+
 #ifdef USE_IMAP
   short imap_browse;
   char *folder;

Attachment: signature.asc
Description: Digital signature