Attachment 8083 Details for Bug 9322 – this patch implements usage of mmapped bufer

[patch] this patch implements usage of mmapped bufer

fdi_cache.diff (text/plain), 13.75 KB, created by Sergey Lapin on 2006-12-13 03:08:50 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Sergey Lapin

Created: 2006-12-13 03:08:50 UTC

Size: 13.75 KB

patch

obsolete

>FDI cache implementation using mmaped file buffer.
>Reduces memory fragmentation and adds a few kilobytes
>from heap to mapped memory.
>
>diff --git a/hald/device_info.c b/hald/device_info.c
>index e6077f8..ab551e7 100644
>--- a/hald/device_info.c
>+++ b/hald/device_info.c
>@@ -38,8 +38,12 @@
> #include <assert.h>
> #include <dbus/dbus.h>
> #include <dbus/dbus-glib.h>
>+#include <sys/types.h>
> #include <sys/stat.h>
>+#include <fcntl.h>
> #include <math.h>
>+#include <sys/mman.h>
>+#include <errno.h>
> 
> #include "hald.h"
> #include "logger.h"
>@@ -108,17 +112,18 @@ match_type {
> struct rule {
> 	/* typ of tule in the list */
> 	enum rule_type rtype;
>-
>-	/* all rules have a key */
>-	char *key;
>-
> 	/* "match" or "merge" rule */
> 	enum match_type type_match;
> 	enum merge_type type_merge;
> 
>-	char *value;
>-	int value_len;
>+	/* all rules have a key */
>+	char * key;
>+	int key_len;
>+	off_t key_offset;
> 
>+	char * value;
>+	int value_len;
>+	off_t value_offset;
> 	/* if rule does not match, skip to this rule */
> 	struct rule *next_rule;
> };
>@@ -132,6 +137,7 @@ struct fdi_context {
> 	struct rule *rule;
> 
> 	int fdi_rule_size;
>+	int cache_fd;
> 
> 	/* all rules */
> 	GSList* rules;
>@@ -157,7 +163,7 @@ rule_type get_rule_type (const char *str
> 	return RULE_UNKNOWN;
> }
> 
>-/*
>+#ifdef DUMP_RULES
> static char *
> get_rule_type_str (enum rule_type type)
> {
>@@ -183,7 +189,7 @@ get_rule_type_str (enum rule_type type)
> 	}
> 	return "invalid rule type";
> }
>-*/
>+#endif
> 
> static enum
> merge_type get_merge_type (const char *str)
>@@ -207,7 +213,7 @@ merge_type get_merge_type (const char *s
> 	return MERGE_UNKNOWN;
> }
> 
>-/*
>+#ifdef DUMP_RULES
> static char *
> get_merge_type_str (enum merge_type type)
> {
>@@ -233,7 +239,7 @@ get_merge_type_str (enum merge_type type
> 	}
> 	return "invalid merge type";
> }
>-*/
>+#endif
> 
> static enum
> match_type get_match_type(const char *str)
>@@ -277,7 +283,7 @@ match_type get_match_type(const char *st
> 	return MATCH_UNKNOWN;
> }
> 
>-/*
>+#ifdef DUMP_RULES
> static char *
> get_match_type_str (enum match_type type)
> {
>@@ -323,7 +329,7 @@ get_match_type_str (enum match_type type
> 	}
> 	return "invalid match type";
> }
>-*/
>+#endif
> 
> /** Resolve a udi-property path as used in .fdi files.
>  *
>@@ -1053,21 +1059,41 @@ rules_cleanup_list (GSList *fdi_rules)
> 	for (elem = fdi_rules; elem != NULL; elem = g_slist_next (elem)) {
> 		struct rule *rule = elem->data;
> 
>-		g_free (rule->key);
>-		g_free (rule->value);
> 		g_free (rule);
> 	}
> 	g_slist_free (fdi_rules);
> 	fdi_rules = NULL;
> }
> 
>+/* stores key string to data file which we'll mmap next */
>+static void store_key(int fd, struct rule * rule, char * key)
>+{
>+  rule->key = (gchar *) 1;
>+  rule->key_offset = lseek(fd, 0, SEEK_END);
>+  write(fd, key, strlen(key)+1);
>+}
>+
>+/* stores value string to data file which we'll mmap next */
>+static void store_value(int fd, struct rule * rule, char * value, int len)
>+{
>+  if(rule->value_len > 0)
>+    lseek(fd, -1, SEEK_END);
>+  else if(rule->value_len == 0) {
>+    rule->value_offset = lseek(fd, 0, SEEK_END);
>+    rule->value = (gchar *) 1;
>+  }
>+  if(value != NULL) write(fd, value, len);
>+  /* zero-terminating */
>+  write(fd, "", 1);
>+}
>+
> /* expat cb for start, e.g. <match foo=bar */
> static void
> start (void *data, const char *el, const char **attr)
> {
> 	struct fdi_context *fdi_ctx = data;
> 	enum rule_type rtype = get_rule_type (el);
>-	int i;
>+	int i, c;
> 
> 	if (rtype == RULE_UNKNOWN)
> 		return;
>@@ -1076,21 +1102,27 @@ start (void *data, const char *el, const
> 		return;
> 
> 	/* get key and attribute for current rule */
>+	c = 0;
> 	for (i = 0; attr[i] != NULL; i+=2) {
> 		if (strcmp (attr[i], "key") == 0) {
>-			fdi_ctx->rule->key = g_strdup (attr[1]);
>+			c = attr[1][0];
>+			store_key(fdi_ctx->cache_fd, fdi_ctx->rule, (void *) attr[1]);
>+			fdi_ctx->rule->key_len += strlen(attr[1]);
> 			continue;
> 		}
> 		if (rtype == RULE_SPAWN) {
> 			if (strcmp (attr[i], "udi") == 0) {
>-				fdi_ctx->rule->key = g_strdup (attr[1]);
>+				c = attr[1][0];
>+				store_key(fdi_ctx->cache_fd, fdi_ctx->rule, (void *) attr[1]);
>+				fdi_ctx->rule->key_len += strlen(attr[1]);
> 				continue;
> 			}
> 		} else if (rtype == RULE_MATCH) {
> 			fdi_ctx->rule->type_match = get_match_type (attr[i]);
> 			if (fdi_ctx->rule->type_match == MATCH_UNKNOWN)
> 				continue;
>-			fdi_ctx->rule->value = g_strdup (attr[i+1]);
>+			store_value(fdi_ctx->cache_fd, fdi_ctx->rule, (void *) attr[i+1], strlen(attr[i+1]));
>+			fdi_ctx->rule->value_len += strlen(attr[i+1]);
> 		} else {
> 			if (strcmp (attr[i], "type") != 0)
> 				continue;
>@@ -1100,9 +1132,8 @@ start (void *data, const char *el, const
> 		}
> 	}
> 
>-	if (fdi_ctx->rule->key[0] == '\0')
>-		return;
>-
>+	if (c == '\0')
>+	    return;
> 	fdi_ctx->rule->rtype = rtype;
> 
> 	/* match rules remember the current nesting and the label to jump to if not matching */
>@@ -1138,11 +1169,9 @@ cdata (void *data, const char *s, int le
> 	if (len < 1)
> 		return;
> 
>-	/* copy cdata in current context */
>-	fdi_ctx->rule->value = g_realloc (fdi_ctx->rule->value, fdi_ctx->rule->value_len + len+1);
>-	memcpy (&fdi_ctx->rule->value[fdi_ctx->rule->value_len], s, len);
>+	fdi_ctx->rule->value = (gchar *) 1;
>+	store_value(fdi_ctx->cache_fd, fdi_ctx->rule, s, len);
> 	fdi_ctx->rule->value_len += len;
>-	fdi_ctx->rule->value[fdi_ctx->rule->value_len] = '\0';
> }
> 
> /* expat cb for end, e.g. </device> */
>@@ -1169,14 +1198,10 @@ end (void *data, const char *el)
> 	if (fdi_ctx->rule->rtype != rtype)
> 		return;
> 
>-	/* set empty value to empty string */
>-	if (fdi_ctx->rule->value == NULL)
>-		fdi_ctx->rule->value = g_strdup ("");
>-
> 	if (fdi_ctx->fdi_rule_size >= 0) {
> 		fdi_ctx->fdi_rule_size += 
> 			sizeof (struct rule) + 
>-			strlen (fdi_ctx->rule->key) + 
>+			fdi_ctx->rule->key_len + 
> 			fdi_ctx->rule->value_len;
> 	}
> 
>@@ -1187,7 +1212,7 @@ end (void *data, const char *el)
> 
> /* decompile an fdi file into a list of rules as this is quicker than opening then each time we want to search */
> static int
>-rules_add_fdi_file (GSList **fdi_rules, const char *filename, gboolean compute_rule_size)
>+rules_add_fdi_file (GSList **fdi_rules, const char *filename, gboolean compute_rule_size, int fd)
> {
> 	struct fdi_context *fdi_ctx;
> 	char *buf;
>@@ -1202,6 +1227,8 @@ rules_add_fdi_file (GSList **fdi_rules,
> 	fdi_ctx = g_new0 (struct fdi_context ,1);
> 	fdi_ctx->rule = g_new0 (struct rule ,1);
> 	fdi_ctx->fdi_rule_size = compute_rule_size ? 0 : -1;
>+	/* assigning cache file descriptor */
>+	fdi_ctx->cache_fd = fd;
> 
> 	XML_Parser parser = XML_ParserCreate (NULL);
> 	if (parser == NULL) {
>@@ -1221,7 +1248,9 @@ rules_add_fdi_file (GSList **fdi_rules,
> 
> 	/* insert last dummy rule into list */
> 	fdi_ctx->rule->rtype = RULE_EOF;
>-	fdi_ctx->rule->key = g_strdup (filename);
>+	store_key(fdi_ctx->cache_fd, fdi_ctx->rule, filename);
>+	store_value(fdi_ctx->cache_fd, fdi_ctx->rule, NULL, 0);
>+	fdi_ctx->rule->key_len += strlen(filename);
> 	fdi_ctx->rules = g_slist_append (fdi_ctx->rules, fdi_ctx->rule);
> 
> 	/* add rules to external list */
>@@ -1253,12 +1282,12 @@ _alphasort(const struct dirent **a, cons
> 
> /* recurse a directory tree, searching and adding fdi files */
> static int
>-rules_search_and_add_fdi_files (GSList **fdi_rules, const char *dir, int *rules_size)
>+rules_search_and_add_fdi_files (GSList **fdi_rules, const char *dir, int *rules_size, int fd)
> {
> 	int i;
> 	int num_entries;
> 	struct dirent **name_list;
>-
>+	
> 	num_entries = scandir (dir, &name_list, 0, _alphasort);
> 	if (num_entries == -1)
> 		return -1;
>@@ -1274,7 +1303,7 @@ rules_search_and_add_fdi_files (GSList *
> 		if (g_file_test (full_path, (G_FILE_TEST_IS_REGULAR))) {
> 			if (len >= 5 && strcmp(&filename[len - 4], ".fdi") == 0) {
> 				int fdi_rules_size;
>-				fdi_rules_size = rules_add_fdi_file (fdi_rules, full_path, rules_size != NULL);
>+				fdi_rules_size = rules_add_fdi_file (fdi_rules, full_path, rules_size != NULL, fd);
> 				if (fdi_rules_size >= 0) {
> 					if (rules_size != NULL) {
> 						*rules_size += fdi_rules_size;
>@@ -1295,7 +1324,7 @@ rules_search_and_add_fdi_files (GSList *
> 				break;
> 
> 			snprintf (dirname, num_bytes, "%s/%s", dir, filename);
>-			rules_search_and_add_fdi_files (fdi_rules, dirname, rules_size);
>+			rules_search_and_add_fdi_files (fdi_rules, dirname, rules_size, fd);
> 			free (dirname);
> 		}
> 		g_free (full_path);
>@@ -1310,8 +1339,8 @@ rules_search_and_add_fdi_files (GSList *
> 	return 0;
> }
> 
>+#ifdef RULES_DUMP
> /* print the rules to screen, mainly useful for debugging */
>-#if 0
> static void
> rules_dump (GSList *fdi_rules)
> {
>@@ -1336,11 +1365,41 @@ rules_dump (GSList *fdi_rules)
> }
> #endif
> 
>+/* Filling key and value with mmapped values */
>+static void mmap_cache_iter(gpointer data, gpointer ptr)
>+{
>+ struct rule * r = (struct rule *) data;
>+
>+ if(r->key == 1) {
>+  r->key = ptr + r->key_offset;
>+ }
>+ 
>+ if(r->value == 1 && r->value_offset >= 0) {
>+  r->value = ptr + r->value_offset;
>+ }
>+ if(r->key == NULL)
>+   DIE(("Key is NULL!"));
>+}
>+
>+/* Seekeng for leftover NULLs and filling them up with
>+   empty strings             */
>+static void check_null_iter(gpointer data, gpointer fdp)
>+{
>+ struct rule * r = (struct rule *) data;
>+ int fd = (int) fdp;
>+ if(r->value == NULL)
>+      store_value(fd, r, NULL, 0);
>+
>+}
> /* setup the location of the rules */
> void
> di_rules_init (void)
> {
> 	int size;
>+	char * tmpdir, * cachename;
>+	int fd;
>+	struct stat statbuf;
>+	gpointer ptr;
> 	char *hal_fdi_source_preprobe = getenv ("HAL_FDI_SOURCE_PREPROBE");
> 	char *hal_fdi_source_information = getenv ("HAL_FDI_SOURCE_INFORMATION");
> 	char *hal_fdi_source_policy = getenv ("HAL_FDI_SOURCE_POLICY");
>@@ -1348,35 +1407,65 @@ di_rules_init (void)
> 	HAL_INFO (("Loading rules"));
> 
> 	size = 0;
>+	if((tmpdir = getenv("TMPDIR")) == NULL) {
>+	    tmpdir = "/tmp";
>+	}
>+	cachename = g_strconcat(tmpdir, "/fdi_cache", NULL);
>+	unlink(cachename);
>+	fd = open(cachename, O_CREAT|O_WRONLY|O_EXCL, 0644);
>+
>+	if(fd <0)
>+		DIE(("Unable to open fdi cache %s file for writing: %s", cachename, strerror(errno)));
> 
> 	if (hal_fdi_source_preprobe != NULL)
>-		rules_search_and_add_fdi_files (&fdi_rules_preprobe, hal_fdi_source_preprobe, &size);
>+		rules_search_and_add_fdi_files (&fdi_rules_preprobe, hal_fdi_source_preprobe, &size, fd);
> 	else {
>-		rules_search_and_add_fdi_files (&fdi_rules_preprobe, PACKAGE_DATA_DIR "/hal/fdi/preprobe", &size);
>-		rules_search_and_add_fdi_files (&fdi_rules_preprobe, PACKAGE_SYSCONF_DIR "/hal/fdi/preprobe", &size);
>+		rules_search_and_add_fdi_files (&fdi_rules_preprobe, PACKAGE_DATA_DIR "/hal/fdi/preprobe", &size, fd);
>+		rules_search_and_add_fdi_files (&fdi_rules_preprobe, PACKAGE_SYSCONF_DIR "/hal/fdi/preprobe", &size, fd);
> 	}
> 
> 	if (hal_fdi_source_information != NULL)
>-		rules_search_and_add_fdi_files (&fdi_rules_information, hal_fdi_source_information, &size);
>+		rules_search_and_add_fdi_files (&fdi_rules_information, hal_fdi_source_information, &size, fd);
> 	else {
>-		rules_search_and_add_fdi_files (&fdi_rules_information, PACKAGE_DATA_DIR "/hal/fdi/information", &size);
>-		rules_search_and_add_fdi_files (&fdi_rules_information, PACKAGE_SYSCONF_DIR "/hal/fdi/information", &size);
>+		rules_search_and_add_fdi_files (&fdi_rules_information, PACKAGE_DATA_DIR "/hal/fdi/information", &size, fd);
>+		rules_search_and_add_fdi_files (&fdi_rules_information, PACKAGE_SYSCONF_DIR "/hal/fdi/information", &size, fd);
> 	}
> 
> 	if (hal_fdi_source_policy != NULL)
>-		rules_search_and_add_fdi_files (&fdi_rules_policy, hal_fdi_source_policy, &size);
>+		rules_search_and_add_fdi_files (&fdi_rules_policy, hal_fdi_source_policy, &size, fd);
> 	else {
>-		rules_search_and_add_fdi_files (&fdi_rules_policy, PACKAGE_DATA_DIR "/hal/fdi/policy", &size);
>-		rules_search_and_add_fdi_files (&fdi_rules_policy, PACKAGE_SYSCONF_DIR "/hal/fdi/policy", &size);
>+		rules_search_and_add_fdi_files (&fdi_rules_policy, PACKAGE_DATA_DIR "/hal/fdi/policy", &size, fd);
>+		rules_search_and_add_fdi_files (&fdi_rules_policy, PACKAGE_SYSCONF_DIR "/hal/fdi/policy", &size, fd);
>+	}
>+	g_slist_foreach(fdi_rules_preprobe, check_null_iter, (gpointer) fd);
>+	g_slist_foreach(fdi_rules_information, check_null_iter, (gpointer) fd);
>+	g_slist_foreach(fdi_rules_policy, check_null_iter, (gpointer) fd);
>+	close(fd);
>+	if (stat (cachename, &statbuf) != 0) {
>+	    DIE (("Couldn't stat file '%s', errno=%d: %s", cachename, errno, strerror (errno)));
> 	}
>+	if((fd = open(cachename, O_RDONLY)) < 0)
>+	    DIE (("Couldn't open file '%s', errno=%d: %s", cachename, errno, strerror (errno)));
> 
>-	/* dump the rules (commented out as this is expensive) */
>-	/*rules_dump (fdi_rules_preprobe);
>-	  rules_dump (fdi_rules_information);
>-	  rules_dump (fdi_rules_policy);
>-	*/
>+	ptr = mmap (NULL, statbuf.st_size, PROT_READ, MAP_SHARED, fd, 0);
>+	if(ptr == MAP_FAILED)
>+	    DIE (("Couldn't mmap file '%s', errno=%d: %s", cachename, errno, strerror (errno)));
>+
>+	close(fd);
> 
>+	g_slist_foreach(fdi_rules_preprobe, mmap_cache_iter, ptr);
>+	g_slist_foreach(fdi_rules_information, mmap_cache_iter, ptr);
>+	g_slist_foreach(fdi_rules_policy, mmap_cache_iter, ptr);
>+#ifdef RULES_DUMP
>+	/* dump the rules (commented out as this is expensive) */
>+	rules_dump (fdi_rules_preprobe);
>+	rules_dump (fdi_rules_information);
>+	rules_dump (fdi_rules_policy);
>+	
>+#endif
> 	HAL_INFO (("Loading rules done (occupying %d bytes)", size));
>+	g_free(cachename);
>+
> }
> 
> /* cleanup the rules */
>@@ -1397,8 +1486,10 @@ rules_match_and_merge_device (GSList *fd
> {
> 	GSList *elem;
> 
>-	if (fdi_rules == NULL) {
>-		di_rules_cleanup ();
>+	if(fdi_rules_preprobe == NULL &&
>+	           fdi_rules_information == NULL &&
>+		   fdi_rules_policy == NULL)
>+	{
> 		di_rules_init ();
> 	}
> 
>@@ -1409,7 +1500,7 @@ rules_match_and_merge_device (GSList *fd
> 		switch (rule->rtype) {
> 		case RULE_MATCH:
> 			/* skip non-matching rules block */
>-			/*HAL_INFO(("%p match '%s' at %s", rule, rule->key, hal_device_get_udi (d)));*/
>+			HAL_INFO(("%p match '%s' at %s", rule, rule->key, hal_device_get_udi (d)));
> 			if (!handle_match (rule, d)) {
> 				/*HAL_INFO(("no match, skip to rule %s (%p)", get_rule_type_str (rule->next_rule->rtype), rule->next_rule));*/
> 				elem = g_slist_find (elem, rule->next_rule);

Actions: View | Splinter Review

Attachments on bug 9322: 8083

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.