Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 25 Jan 2010 00:34:46 +0100
From:      =?ISO-8859-1?Q?G=E1bor_K=F6vesd=E1n?= <gabor@FreeBSD.org>
To:        Kostik Belousov <kostikbel@gmail.com>
Cc:        attilio@freebsd.org, Xin LI <delphij@delphij.net>, Hajimu UMEMOTO <ume@freebsd.org>, Peter Jeremy <peterjeremy@acm.org>, current@freebsd.org
Subject:   Re: NLS/strerror efficiency
Message-ID:  <4B5CD916.1060003@FreeBSD.org>
In-Reply-To: <20100124113718.GC3877@deviant.kiev.zoral.com.ua>
References:  <20100119212019.GL59590@deviant.kiev.zoral.com.ua> <4B56CACF.50508@FreeBSD.org> <yge1vhgvdd3.wl%ume@mahoroba.org> <4B5B4F4B.3030201@FreeBSD.org> <20100124091911.GI31243@server.vk2pj.dyndns.org> <4B5C27B9.1080805@FreeBSD.org> <20100124113718.GC3877@deviant.kiev.zoral.com.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------060809070007000604060304
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Kostik Belousov wrote:
>> I'll fix up my patch to do that.
>>     
Here's my new patch with a little program to demonstrate that it works 
as expected even if locale is changed between various 
strerror(3)/strsignal(3) calls.

Gabor

--------------060809070007000604060304
Content-Type: text/x-patch;
 name="msgcat.c.diff"
Content-Transfer-Encoding: 8bit
Content-Disposition: inline;
 filename="msgcat.c.diff"

Index: nls/msgcat.c
===================================================================
--- nls/msgcat.c	(revisión: 202658)
+++ nls/msgcat.c	(copia de trabajo)
@@ -1,5 +1,6 @@
 /***********************************************************
 Copyright 1990, by Alfalfa Software Incorporated, Cambridge, Massachusetts.
+Copyright 2010, Gabor Kovesdan <gabor@FreeBSD.org>
 
                         All Rights Reserved
 
@@ -39,6 +40,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 +49,7 @@
 #include <limits.h>
 #include <locale.h>
 #include <nl_types.h>
+#include <pthread.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -57,38 +60,94 @@
 
 #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 *, const char *);
 
+static pthread_rwlock_t		 rwlock;
+
+struct catentry {
+	SLIST_ENTRY(catentry)	 list;
+	char			*name;
+	char			*path;
+	int			 caterrno;
+	nl_catd			 catd;
+	char			*lang;
+	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);
 
-	/* is it absolute path ? if yes, load immediately */
 	if (strchr(name, '/') != NULL)
-		return (load_msgcat(name));
+		lang = NULL;
+	else {
+		if (type == NL_CAT_LOCALE)
+			lang = setlocale(LC_MESSAGES, NULL);
+		else
+			lang = getenv("LANG");
 
-	if (type == NL_CAT_LOCALE)
-		lang = setlocale(LC_MESSAGES, NULL);
-	else
-		lang = getenv("LANG");
+		if (lang == NULL || *lang == '\0' || strlen(lang) > ENCODING_LEN ||
+		    (lang[0] == '.' &&
+		    (lang[1] == '\0' || (lang[1] == '.' && lang[2] == '\0'))) ||
+		    strchr(lang, '/') != NULL)
+			lang = "C";
+	}
 
-	if (lang == NULL || *lang == '\0' || strlen(lang) > ENCODING_LEN ||
-	    (lang[0] == '.' &&
-	     (lang[1] == '\0' || (lang[1] == '.' && lang[2] == '\0'))) ||
-	    strchr(lang, '/') != NULL)
-		lang = "C";
+	/* Init rwlock used to protect cache */
+	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) {
+				/* Found cached failing entry */
+				UNLOCK(NLERR);
+				NLRETERR(np->caterrno);
+			} else if (strcmp(np->lang, lang) == 0) {
+				/* Found cached successful entry */
+				np->refcount++;
+				UNLOCK(NLERR);
+				return (np->catd);
+			}
+		}
+	}
+	UNLOCK(NLERR);
+
+	/* is it absolute path ? if yes, load immediately */
+	if (strchr(name, '/') != NULL)
+		return (load_msgcat(name, name, lang));
+
 	if ((plang = cptr1 = strdup(lang)) == NULL)
 		return (NLERR);
 	if ((cptr = strchr(cptr1, '@')) != NULL)
@@ -166,7 +225,7 @@
 			if (stat(path, &sbuf) == 0) {
 				free(plang);
 				free(base);
-				return (load_msgcat(path));
+				return (load_msgcat(path, name, lang));
 			}
 		} else {
 			tmpptr = (char *)name;
@@ -182,20 +241,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 +287,7 @@
 				} else {
 					l = i + 1;
 				}
-}
+			}
 
 			/* not found */
 			goto notfound;
@@ -238,25 +297,41 @@
 		} 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);
+	/* Remove from cache if not referenced any more */
+	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 +340,38 @@
  */
 
 static nl_catd
-load_msgcat(const char *path)
+load_msgcat(const char *path, const char *name, const char *lang)
 {
-	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 +380,39 @@
 	    (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;
+
+	/* Caching opened catalog */
+	WLOCK(NLERR);
+	if ((np = malloc(sizeof(struct catentry))) != NULL) {
+		np->name = strdup(name);
+		np->path = strdup(path);
+		np->catd = catd;
+		np->lang = (lang == NULL) ? NULL : strdup(lang);
+		np->refcount = 1;
+		np->caterrno = 0;
+		SLIST_INSERT_HEAD(&flist, np, list);
+	}
+	UNLOCK(NLERR);
 	return (catd);
 }
 

--------------060809070007000604060304--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4B5CD916.1060003>