[Nagios-checkins] SF.net SVN: nagios:[2041] nagioscore/trunk

ageric at users.sourceforge.net ageric at users.sourceforge.net
Thu Aug 2 00:46:49 UTC 2012


Revision: 2041
          http://nagios.svn.sourceforge.net/nagios/?rev=2041&view=rev
Author:   ageric
Date:     2012-08-02 00:46:49 +0000 (Thu, 02 Aug 2012)
Log Message:
-----------
lib/kvvec: Use assignment rather than copying in buf2kvvec()

This saves an enormous amount of work, since we no longer have to
get fresh memory for each check result reported in by the workers.
Since new memory would inevitably be located in a different page
than the I/O cache buffer we've got allocated for the worker, that
means we can now avoid at least one cache-line fill and a (relatively)
huge constant overhead of check result reception.

Signed-off-by: Andreas Ericsson <ae at op5.se>

Modified Paths:
--------------
    nagioscore/trunk/base/workers.c
    nagioscore/trunk/lib/kvvec.c
    nagioscore/trunk/lib/kvvec.h
    nagioscore/trunk/lib/test-kvvec.c
    nagioscore/trunk/lib/worker.c

Modified: nagioscore/trunk/base/workers.c
===================================================================
--- nagioscore/trunk/base/workers.c	2012-08-02 00:46:29 UTC (rev 2040)
+++ nagioscore/trunk/base/workers.c	2012-08-02 00:46:49 UTC (rev 2041)
@@ -369,8 +369,10 @@
 	char *buf;
 	unsigned long size;
 	int ret;
+	static struct kvvec kvv = KVVEC_INITIALIZER;
 
 	ret = iocache_read(wp->ioc, wp->sd);
+
 	if (ret < 0) {
 		logit(NSLOG_RUNTIME_WARNING, TRUE, "iocache_read() from worker %d returned %d: %s\n",
 			  wp->pid, ret, strerror(errno));
@@ -384,45 +386,37 @@
 	}
 
 	while ((buf = iocache_use_delim(wp->ioc, MSG_DELIM, MSG_DELIM_LEN, &size))) {
-		struct kvvec *kvv;
 		int job_id = -1;
-		char *key, *value;
 		worker_job *job;
 		wproc_result wpres;
 
-		kvv = buf2kvvec(buf, size, '=', '\0');
-		if (!kvv) {
-			/* XXX FIXME log an error */
+		/* log messages are handled first */
+		if (size > 5 && !memcmp(buf, "log=", 4)) {
+			logit(NSLOG_INFO_MESSAGE, TRUE, "worker %d: %s\n", wp->pid, buf + 4);
 			continue;
 		}
 
-		key = kvv->kv[0].key;
-		value = kvv->kv[0].value;
-
-		/* log messages are handled first */
-		if (kvv->kv_pairs == 1 && !strcmp(key, "log")) {
-			logit(NSLOG_INFO_MESSAGE, TRUE, "worker %d: %s\n", wp->pid, value);
-			kvvec_destroy(kvv, KVVEC_FREE_ALL);
+		/* for everything else we need to actually parse */
+		if (buf2kvvec_prealloc(&kvv, buf, size, '=', '\0', KVVEC_ASSIGN) <= 0) {
+			/* XXX FIXME log an error */
 			continue;
 		}
 
 		memset(&wpres, 0, sizeof(wpres));
 		wpres.job_id = -1;
 		wpres.type = -1;
-		wpres.response = kvv;
-		parse_worker_result(&wpres, kvv);
+		wpres.response = &kvv;
+		parse_worker_result(&wpres, &kvv);
 
 		job = get_job(wp, wpres.job_id);
 		if (!job) {
 			logit(NSLOG_RUNTIME_WARNING, TRUE, "Worker job with id '%d' doesn't exist on worker %d.\n",
 				  job_id, wp->pid);
-			kvvec_destroy(kvv, KVVEC_FREE_ALL);
 			continue;
 		}
 		if (wpres.type != job->type) {
 			logit(NSLOG_RUNTIME_WARNING, TRUE, "Worker %d claims job %d is type %d, but we think it's type %d\n",
 				  wp->pid, job->id, wpres.type, job->type);
-			kvvec_destroy(kvv, KVVEC_FREE_ALL);
 			break;
 		}
 		oj = (wproc_object_job *)job->arg;
@@ -499,7 +493,6 @@
 			logit(NSLOG_RUNTIME_WARNING, TRUE, "Worker %d: Unknown jobtype: %d\n", wp->pid, job->type);
 			break;
 		}
-		kvvec_destroy(kvv, KVVEC_FREE_ALL);
 		destroy_job(wp, job);
 		wp->jobs_running--;
 	}

Modified: nagioscore/trunk/lib/kvvec.c
===================================================================
--- nagioscore/trunk/lib/kvvec.c	2012-08-02 00:46:29 UTC (rev 2040)
+++ nagioscore/trunk/lib/kvvec.c	2012-08-02 00:46:49 UTC (rev 2041)
@@ -233,14 +233,14 @@
  * much use, but it's nifty for ipc where only computers are
  * involved, and it will parse the kvvec2buf() produce nicely.
  */
-struct kvvec *buf2kvvec_prealloc(struct kvvec *kvv, const char *str,
-				 unsigned int len, const char kvsep,
-				 const char pair_sep)
+int buf2kvvec_prealloc(struct kvvec *kvv, char *str,
+			unsigned int len, const char kvsep,
+			const char pair_sep, int flags)
 {
 	unsigned int num_pairs = 0, i, offset = 0;
 
 	if (!str || !len || !kvv)
-		return NULL;
+		return -1;
 
 	/* first we count the number of key/value pairs */
 	while (offset < len) {
@@ -259,12 +259,15 @@
 	}
 
 	if (!num_pairs) {
-		return NULL;
+		return 0;
 	}
 
 	/* make sure the key/value vector is large enough */
-	if (kvvec_capacity(kvv) < num_pairs && kvvec_grow(kvv, num_pairs) < 0)
-		return NULL;
+	if (!(flags & KVVEC_APPEND)) {
+		kvvec_init(kvv, num_pairs);
+	} else if (kvvec_capacity(kvv) < num_pairs && kvvec_grow(kvv, num_pairs) < 0) {
+		return -1;
+	}
 
 	offset = 0;
 	for (i = 0; i < num_pairs; i++) {
@@ -273,7 +276,7 @@
 
 		/* keys can't begin with nul bytes */
 		if (offset && str[offset] == '\0') {
-			return kvv;
+			return kvv->kv_pairs;
 		}
 
 		key_end_ptr = memchr(str + offset, kvsep, len - offset);
@@ -287,30 +290,42 @@
 
 		kv = &kvv->kv[kvv->kv_pairs++];
 		kv->key_len = (unsigned long)key_end_ptr - ((unsigned long)str + offset);
-		kv->key = malloc(kv->key_len + 1);
-		memcpy(kv->key, str + offset, kv->key_len);
+		if (flags & KVVEC_COPY) {
+			kv->key = malloc(kv->key_len + 1);
+			memcpy(kv->key, str + offset, kv->key_len);
+		} else {
+			kv->key = str + offset;
+		}
 		kv->key[kv->key_len] = 0;
 
 		offset += kv->key_len + 1;
 
 		if (str[offset] == pair_sep) {
 			kv->value_len = 0;
-			kv->value = strdup("");
+			if (flags & KVVEC_COPY) {
+				kv->value = strdup("");
+			} else {
+				kv->value = "";
+			}
 		} else {
 			kv->value_len = (unsigned long)kv_end_ptr - ((unsigned long)str + offset);
-			kv->value = malloc(kv->value_len + 1);
+			if (flags & KVVEC_COPY) {
+				kv->value = malloc(kv->value_len + 1);
+				memcpy(kv->value, str + offset, kv->value_len);
+			} else {
+				kv->value = str + offset;
+			}
 			kv->value[kv->value_len] = 0;
-			memcpy(kv->value, str + offset, kv->value_len);
 		}
 
 		offset += kv->value_len + 1;
 	}
 
-	return kvv;
+	return i;
 }
 
-struct kvvec *buf2kvvec(const char *str, unsigned int len, const char kvsep,
-			const char pair_sep)
+struct kvvec *buf2kvvec(char *str, unsigned int len, const char kvsep,
+			const char pair_sep, int flags)
 {
 	struct kvvec *kvv;
 
@@ -318,5 +333,9 @@
 	if (!kvv)
 		return NULL;
 
-	return buf2kvvec_prealloc(kvv, str, len, kvsep, pair_sep);
+	if (buf2kvvec_prealloc(kvv, str, len, kvsep, pair_sep, flags) >= 0)
+		return kvv;
+
+	free(kvv);
+	return NULL;
 }

Modified: nagioscore/trunk/lib/kvvec.h
===================================================================
--- nagioscore/trunk/lib/kvvec.h	2012-08-02 00:46:29 UTC (rev 2040)
+++ nagioscore/trunk/lib/kvvec.h	2012-08-02 00:46:49 UTC (rev 2041)
@@ -53,6 +53,10 @@
 /** Free both keys and values when destroying a kv vector */
 #define KVVEC_FREE_ALL    (KVVEC_FREE_KEYS | KVVEC_FREE_VALUES)
 
+#define KVVEC_ASSIGN      0 /**< Assign from buf in buf2kvvec_prealloc() */
+#define KVVEC_COPY        1 /**< Copy from buf in buf2kvvec_prealloc() */
+#define KVVEC_APPEND      2 /**< Don't reset kvvec in buf2kvvec_prealloc() */
+
 /**
  * Initialize a previously allocated key/value vector
  *
@@ -165,7 +169,19 @@
  * @param len Length of buffer to convert
  * @param kvsep Character separating key and value
  * @param pair_sep Character separating key/value pairs
+ * @return The created key/value vector
  */
-extern struct kvvec *buf2kvvec(const char *str, unsigned int len, const char kvsep, const char pair_sep);
+extern struct kvvec *buf2kvvec(char *str, unsigned int len, const char kvsep, const char pair_sep, int flags);
 
+/**
+ * Parse a buffer into the pre-allocated key/value vector. Immensely
+ * useful for ipc in combination with kvvec2buf().
+ *
+ * @param str The buffer to convert to a key/value vector
+ * @param len Length of buffer to convert
+ * @param kvsep Character separating key and value
+ * @param pair_sep Character separating key/value pairs
+ * @return The number of pairs in the created key/value vector
+ */
+extern int buf2kvvec_prealloc(struct kvvec *kvv, char *str, unsigned int len, const char kvsep, const char pair_sep, int flags);
 #endif /* INCLUDE_kvvec_h__ */

Modified: nagioscore/trunk/lib/test-kvvec.c
===================================================================
--- nagioscore/trunk/lib/test-kvvec.c	2012-08-02 00:46:29 UTC (rev 2040)
+++ nagioscore/trunk/lib/test-kvvec.c	2012-08-02 00:46:49 UTC (rev 2041)
@@ -62,10 +62,12 @@
 int main(int argc, char **argv)
 {
 	int i;
-	struct kvvec *kvv, *kvv2;
+	struct kvvec *kvv, *kvv2, *kvv3;
 	struct kvvec_buf *kvvb, *kvvb2;
 
 	kvv = kvvec_create(1);
+	kvv2 = kvvec_create(1);
+	kvv3 = kvvec_create(1);
 	add_vars(kvv, test_data, 1239819);
 	add_vars(kvv, (const char **)argv + 1, argc - 1);
 
@@ -74,10 +76,14 @@
 
 	/* kvvec2buf -> buf2kvvec -> kvvec2buf -> buf2kvvec conversion */
 	kvvb = kvvec2buf(kvv, KVSEP, PAIRSEP, OVERALLOC);
-	kvv2 = buf2kvvec(kvvb->buf, kvvb->buflen, KVSEP, PAIRSEP);
+	kvv3 = buf2kvvec(kvvb->buf, kvvb->buflen, KVSEP, PAIRSEP, KVVEC_COPY);
+	kvvb2 = kvvec2buf(kvv3, KVSEP, PAIRSEP, OVERALLOC);
+
+	buf2kvvec_prealloc(kvv2, kvvb->buf, kvvb->buflen, KVSEP, PAIRSEP, KVVEC_ASSIGN);
 	kvvec_foreach(kvv2, kvv, walker);
-	kvvb2 = kvvec2buf(kvv2, KVSEP, PAIRSEP, OVERALLOC);
 
+	kvvb = kvvec2buf(kvv, KVSEP, PAIRSEP, OVERALLOC);
+
 	if (kvv->kv_pairs != kvv2->kv_pairs) {
 		printf("Failure: kvvec2buf -> buf2kvvec fails to get pairs teamed up (delta: %d)\n",
 			   kvv->kv_pairs - kvv2->kv_pairs);
@@ -99,6 +105,11 @@
 				   kv2->key, kv2->value, kv2->key_len, kv2->value_len);
 		}
 	}
+	if (kvvb2->buflen == kvvb->buflen)
+		printf("PASS: kvvb->buflen == kvvb->buflen\n");
+	if (kvvb2->bufsize == kvvb->bufsize)
+		printf("PASS: kvvb2->bufsize = kvvb->bufsize\n");
+
 	if (kvvb2->buflen == kvvb->buflen && kvvb2->bufsize == kvvb->bufsize &&
 		!memcmp(kvvb2->buf, kvvb->buf, kvvb->bufsize))
 	{
@@ -110,6 +121,9 @@
 
 	free(kvvb->buf);
 	free(kvvb);
+	free(kvvb2->buf);
+	free(kvvb2);
 	kvvec_destroy(kvv, 1);
+	kvvec_destroy(kvv3, KVVEC_FREE_ALL);
 	return 0;
 }

Modified: nagioscore/trunk/lib/worker.c
===================================================================
--- nagioscore/trunk/lib/worker.c	2012-08-02 00:46:29 UTC (rev 2040)
+++ nagioscore/trunk/lib/worker.c	2012-08-02 00:46:49 UTC (rev 2041)
@@ -556,7 +556,8 @@
 	 */
 	while ((buf = iocache_use_delim(ioc, MSG_DELIM, MSG_DELIM_LEN_RECV, &size))) {
 		struct kvvec *kvv;
-		kvv = buf2kvvec(buf, (unsigned int)size, KV_SEP, PAIR_SEP);
+		/* we must copy vars here, as we preserve them for the response */
+		kvv = buf2kvvec(buf, (unsigned int)size, KV_SEP, PAIR_SEP, KVVEC_COPY);
 		if (kvv)
 			spawn_job(kvv);
 	}

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.





More information about the Nagios-commits mailing list