NLS/strerror efficiency

Gabor Kovesdan gabor at FreeBSD.org
Sat Jan 23 19:34:50 UTC 2010


El 2010. 01. 23. 20:18, Hajimu UMEMOTO escribió:
> The gai_strerror(3) has same issue.  How about the attached patch in
> this mail?
>    
I have a fix for msgcat.c to optimalize catalog handling. As I'm not an 
src committer, delphij@ is helping me in reviewing and approving my 
patches. I've sent the attached patch to him today and I'm waiting for 
his response now. This patch caches the failing and the succesful 
catopen() calls in a thread-safe way and in the latter case it counts 
references to cached catalog.

Still, I consider it a good idea to have a static catalog descriptor in 
strerror.3 and use that instead of always opening and closing the catalog.

Regards,

-- 
Gabor Kovesdan
FreeBSD Volunteer

EMAIL: gabor at FreeBSD.org .:|:. gabor at kovesdan.org
WEB:   http://people.FreeBSD.org/~gabor .:|:. http://kovesdan.org

-------------- next part --------------
Index: nls/msgcat.c
===================================================================
--- nls/msgcat.c	(revisión: 202658)
+++ nls/msgcat.c	(copia de trabajo)
@@ -39,6 +39,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/mman.h>
+#include <sys/queue.h>
 
 #include <arpa/inet.h>		/* for ntohl() */
 
@@ -47,6 +48,7 @@
 #include <limits.h>
 #include <locale.h>
 #include <nl_types.h>
+#include <pthread.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -57,26 +59,74 @@
 
 #define _DEFAULT_NLS_PATH "/usr/share/nls/%L/%N.cat:/usr/share/nls/%N/%L:/usr/local/share/nls/%L/%N.cat:/usr/local/share/nls/%N/%L"
 
+#define RLOCK(fail)	{ if (_pthread_rwlock_rdlock(&rwlock) != 0) return (fail); }
+#define WLOCK(fail)	{ if (_pthread_rwlock_wrlock(&rwlock) != 0) return (fail); }
+#define UNLOCK(fail)	{ if (_pthread_rwlock_unlock(&rwlock) != 0) return (fail); }
+
 #define	NLERR		((nl_catd) -1)
 #define NLRETERR(errc)  { errno = errc; return (NLERR); }
+#define SAVEFAIL(n, e)	{ WLOCK(NLERR); \
+			  np = malloc(sizeof(struct catentry)); \
+			  if (np != NULL) { \
+			  	np->name = strdup(n); \
+				np->caterrno = e; \
+			  	SLIST_INSERT_HEAD(&flist, np, list); \
+			  } \
+			  UNLOCK(NLERR); \
+			}
 
-static nl_catd load_msgcat(const char *);
+static nl_catd load_msgcat(const char *, const char *);
 
+static pthread_rwlock_t		 rwlock;
+
+struct catentry {
+	SLIST_ENTRY(catentry)	 list;
+	char			*name;
+	char			*path;
+	int			 caterrno;
+	nl_catd			 catd;
+	int			 refcount;
+};
+
+SLIST_HEAD(listhead, catentry) flist =
+    SLIST_HEAD_INITIALIZER(flist);
+
 nl_catd
 catopen(const char *name, int type)
 {
-	int             spcleft, saverr;
-	char            path[PATH_MAX];
-	char            *nlspath, *lang, *base, *cptr, *pathP, *tmpptr;
-	char            *cptr1, *plang, *pter, *pcode;
-	struct stat     sbuf;
+	int		 spcleft, saverr;
+	char		 path[PATH_MAX];
+	char		*nlspath, *lang, *base, *cptr, *pathP, *tmpptr;
+	char		*cptr1, *plang, *pter, *pcode;
+	struct stat	 sbuf;
+	struct catentry	*np;
 
 	if (name == NULL || *name == '\0')
 		NLRETERR(EINVAL);
 
+	if ((rwlock == (pthread_rwlock_t)0) &&
+	    (_pthread_rwlock_init(&rwlock, NULL) != 0))
+		return (NLERR);
+
+	/* Try to get it from the cache first */
+	RLOCK(NLERR);
+	SLIST_FOREACH(np, &flist, list) {
+		if (strcmp(np->name, name) == 0) {
+			if (np->caterrno != 0) {
+				UNLOCK(NLERR);
+				NLRETERR(np->caterrno);
+			} else {
+				UNLOCK(NLERR);
+				np->refcount++;
+				return (np->catd);
+			}
+		}
+	}
+	UNLOCK(NLERR);
+
 	/* is it absolute path ? if yes, load immediately */
 	if (strchr(name, '/') != NULL)
-		return (load_msgcat(name));
+		return (load_msgcat(name, name));
 
 	if (type == NL_CAT_LOCALE)
 		lang = setlocale(LC_MESSAGES, NULL);
@@ -85,7 +135,7 @@
 
 	if (lang == NULL || *lang == '\0' || strlen(lang) > ENCODING_LEN ||
 	    (lang[0] == '.' &&
-	     (lang[1] == '\0' || (lang[1] == '.' && lang[2] == '\0'))) ||
+	    (lang[1] == '\0' || (lang[1] == '.' && lang[2] == '\0'))) ||
 	    strchr(lang, '/') != NULL)
 		lang = "C";
 
@@ -166,7 +216,7 @@
 			if (stat(path, &sbuf) == 0) {
 				free(plang);
 				free(base);
-				return (load_msgcat(path));
+				return (load_msgcat(path, name));
 			}
 		} else {
 			tmpptr = (char *)name;
@@ -182,20 +232,20 @@
 char *
 catgets(nl_catd catd, int set_id, int msg_id, const char *s)
 {
-	struct _nls_cat_hdr *cat_hdr;
-	struct _nls_set_hdr *set_hdr;
-	struct _nls_msg_hdr *msg_hdr;
-	int l, u, i, r;
+	struct _nls_cat_hdr	*cat_hdr;
+	struct _nls_set_hdr	*set_hdr;
+	struct _nls_msg_hdr	*msg_hdr;
+	int			 l, u, i, r;
 
 	if (catd == NULL || catd == NLERR) {
 		errno = EBADF;
 		/* LINTED interface problem */
-		return (char *) s;
-}
+		return ((char *)s);
+	}
 
 	cat_hdr = (struct _nls_cat_hdr *)catd->__data; 
 	set_hdr = (struct _nls_set_hdr *)(void *)((char *)catd->__data
-			+ sizeof(struct _nls_cat_hdr));
+	    + sizeof(struct _nls_cat_hdr));
 
 	/* binary search, see knuth algorithm b */
 	l = 0;
@@ -228,7 +278,7 @@
 				} else {
 					l = i + 1;
 				}
-}
+			}
 
 			/* not found */
 			goto notfound;
@@ -238,25 +288,40 @@
 		} else {
 			l = i + 1;
 		}
-}
+	}
 
 notfound:
 	/* not found */
 	errno = ENOMSG;
 	/* LINTED interface problem */
-	return (char *) s;
+	return ((char *)s);
 }
 
 int
 catclose(nl_catd catd)
 {
+	struct catentry		*np;
+
 	if (catd == NULL || catd == NLERR) {
 		errno = EBADF;
 		return (-1);
 	}
 
-	munmap(catd->__data, (size_t)catd->__size);
-	free(catd);
+	WLOCK(-1);
+	SLIST_FOREACH(np, &flist, list) {
+		if ((np->catd->__size == catd->__size) &&
+		    memcmp((const void *)np->catd, (const void *)catd, np->catd->__size) == 0) {
+			np->refcount--;
+			if (np->refcount == 0) {
+				munmap(catd->__data, (size_t)catd->__size);
+				free(catd);
+				SLIST_REMOVE(&flist, np, catentry, list);
+				free(np);
+			}
+			break;
+		}
+	}
+	UNLOCK(-1);
 	return (0);
 }
 
@@ -265,19 +330,38 @@
  */
 
 static nl_catd
-load_msgcat(const char *path)
+load_msgcat(const char *path, const char *name)
 {
-	struct stat st;
-	nl_catd catd;
-	void *data;
-	int fd;
+	struct stat	 st;
+	nl_catd		 catd;
+	struct catentry	*np;
+	void		*data;
+	int		 fd;
 
-	/* XXX: path != NULL? */
+	if (path == NULL) {
+		errno = EINVAL;
+		return (NLERR);
+	}
 
-	if ((fd = _open(path, O_RDONLY)) == -1)
+	/* One more try in cache; if it was not found by name,
+	   it might still be found by absolute path. */
+	RLOCK(NLERR);
+	SLIST_FOREACH(np, &flist, list) {
+		if (strcmp(np->path, path) == 0) {
+			np->refcount++;
+			UNLOCK(NLERR);
+			return (np->catd);
+		}
+	}
+	UNLOCK(NLERR);
+
+	if ((fd = _open(path, O_RDONLY)) == -1) {
+		SAVEFAIL(name, errno);
 		return (NLERR);
+	}
 
 	if (_fstat(fd, &st) != 0) {
+		SAVEFAIL(name, errno);
 		_close(fd);
 		return (NLERR);
 	}
@@ -286,22 +370,37 @@
 	    (off_t)0);
 	_close(fd);
 
-	if (data == MAP_FAILED)
+	if (data == MAP_FAILED) {
+		SAVEFAIL(name, errno);
 		return (NLERR);
+	}
 
 	if (ntohl((u_int32_t)((struct _nls_cat_hdr *)data)->__magic) !=
 	    _NLS_MAGIC) {
+		SAVEFAIL(name, errno);
 		munmap(data, (size_t)st.st_size);
 		NLRETERR(EINVAL);
 	}
 
 	if ((catd = malloc(sizeof (*catd))) == NULL) {
+		SAVEFAIL(name, errno);
 		munmap(data, (size_t)st.st_size);
 		return (NLERR);
 	}
 
 	catd->__data = data;
 	catd->__size = (int)st.st_size;
+
+	WLOCK(NLERR);
+	if ((np = malloc(sizeof(struct catentry))) != NULL) {
+		np->name = strdup(name);
+		np->path = strdup(path);
+		np->catd = catd;
+		np->refcount = 1;
+		np->caterrno = 0;
+		SLIST_INSERT_HEAD(&flist, np, list);
+	}
+	UNLOCK(NLERR);
 	return (catd);
 }
 


More information about the freebsd-current mailing list