Date: Sat, 23 Jan 2010 20:34:35 +0100 From: Gabor Kovesdan <gabor@FreeBSD.org> To: Hajimu UMEMOTO <ume@FreeBSD.org> Cc: Kostik Belousov <kostikbel@gmail.com>, attilio@FreeBSD.org, current@FreeBSD.org Subject: Re: NLS/strerror efficiency Message-ID: <4B5B4F4B.3030201@FreeBSD.org> In-Reply-To: <yge1vhgvdd3.wl%ume@mahoroba.org> References: <20100119212019.GL59590@deviant.kiev.zoral.com.ua> <4B56CACF.50508@FreeBSD.org> <yge1vhgvdd3.wl%ume@mahoroba.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] 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@FreeBSD.org .:|:. gabor@kovesdan.org WEB: http://people.FreeBSD.org/~gabor .:|:. http://kovesdan.org [-- Attachment #2 --] 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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4B5B4F4B.3030201>