Your message dated Tue, 16 Jun 2009 23:26:36 +0100
with message-id <[email protected]>
and subject line Extremly slow browsing of folders - getgrgid/getpwuid cache 
patch
has caused the Debian Bug report #273942,
regarding Extremly slow browsing of folders - getgrgid/getpwuid cache patch
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact [email protected]
immediately.)


-- 
273942: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=273942
Debian Bug Tracking System
Contact [email protected] with problems
--- Begin Message ---
Package: mutt
Version: 1.5.6-20040818+1
Severity: wishlist
Tags: patch


Recently I have switched my workplace over to an ldap directory
service from YP(NIS).

When this was done browsing folders in mutt became *very* slow, felt
like using it over a 1200baud modem :-(

I am fully aware of (and use) nscd cacheing demon 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.

-- 
Regards Vincent
http://www.kyllikki.org/
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


--- End Message ---
--- Begin Message ---
Hi,
I'm closing the bug according to my last correspondence.
Please see the entire bug log to know more.

Thanks for your report.

Cheers
Antonio


--- End Message ---

Reply via email to