[icinga-checkins] icinga.org: icinga-core/mfriedrich/workers: libicinga: use assignment rather than copying in buf2kvvec() #2955

git at icinga.org git at icinga.org
Sun Aug 5 23:32:12 CEST 2012


Module: icinga-core
Branch: mfriedrich/workers
Commit: ed894e8bbd4894c74ca2f79341f9f420bb61c595
URL:    https://git.icinga.org/?p=icinga-core.git;a=commit;h=ed894e8bbd4894c74ca2f79341f9f420bb61c595

Author: Michael Friedrich <michael.friedrich at univie.ac.at>
Date:   Sun Aug  5 15:27:46 2012 +0200

libicinga: use assignment rather than copying in buf2kvvec() #2955

this adds the flags directive to buf2kvvec() calls, allowing to set to
copy or append all data.

basically, we don't need to get fresh memory for each checkresults
reported in by the workers. since new memory would inevitable 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.

refs #2955

---

 base/workers.c   |   27 ++++++++++-----------------
 lib/kvvec.c      |   50 +++++++++++++++++++++++++++++++++++---------------
 lib/kvvec.h      |   18 +++++++++++++++++-
 lib/test-kvvec.c |   20 +++++++++++++++++---
 lib/worker.c     |    3 ++-
 5 files changed, 81 insertions(+), 37 deletions(-)

diff --git a/base/workers.c b/base/workers.c
index 9d164c5..a305c78 100644
--- a/base/workers.c
+++ b/base/workers.c
@@ -383,8 +383,10 @@ static int handle_worker_result(int sd, int events, void *arg) {
 	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));
@@ -398,45 +400,37 @@ static int handle_worker_result(int sd, int events, void *arg) {
 	}
 
 	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) {
+			/* 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;
@@ -513,7 +507,6 @@ static int handle_worker_result(int sd, int events, void *arg) {
 			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--;
 	}
diff --git a/lib/kvvec.c b/lib/kvvec.c
index 870c415..6e32ea1 100644
--- a/lib/kvvec.c
+++ b/lib/kvvec.c
@@ -249,13 +249,13 @@ int kvvec_capacity(struct kvvec *kvv) {
  * 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,
+int buf2kvvec_prealloc(struct kvvec *kvv, char *str,
 		unsigned int len, const char kvsep,
-		const char pair_sep) {
+		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) {
@@ -274,12 +274,15 @@ struct kvvec *buf2kvvec_prealloc(struct kvvec *kvv, const char *str,
 	}
 
 	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_resize(kvv, num_pairs < 0)) {
+		return -1;
+	}
 
 	offset = 0;
 	for (i = 0; i < num_pairs; i++) {
@@ -288,7 +291,7 @@ struct kvvec *buf2kvvec_prealloc(struct kvvec *kvv, const char *str,
 
 		/* 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);
@@ -302,35 +305,52 @@ struct kvvec *buf2kvvec_prealloc(struct kvvec *kvv, const char *str,
 
 		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) {
+		const char pair_sep, int flags) {
 	struct kvvec *kvv;
 
 	kvv = kvvec_create(len / 20);
 	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;
 }
diff --git a/lib/kvvec.h b/lib/kvvec.h
index 06ade53..6e24654 100644
--- a/lib/kvvec.h
+++ b/lib/kvvec.h
@@ -78,6 +78,10 @@ struct kvvec {
 /** 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
  * @param hint Number of key/value pairs we expect to store
@@ -189,7 +193,19 @@ extern struct kvvec_buf *kvvec2buf(struct kvvec *kvv, char kv_sep, char pair_sep
  * @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(const 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 seperating key/value pairs
+ * @param pair_sep Character seperating 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__ */
diff --git a/lib/test-kvvec.c b/lib/test-kvvec.c
index b71af14..78ae7fa 100644
--- a/lib/test-kvvec.c
+++ b/lib/test-kvvec.c
@@ -84,10 +84,12 @@ static void add_vars(struct kvvec *kvv, const char **ary, int len) {
 
 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);
 
@@ -96,9 +98,13 @@ int main(int argc, char **argv) {
 
 	/* 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",
@@ -121,6 +127,11 @@ int main(int argc, char **argv) {
 			       kv2->key, kv2->value, kv2->key_len, kv2->value_len);
 		}
 	}
+
+	if (kvvb2->buflen == kvvb->buflen)
+		printf("PASS: kvvb2->buflen == kvvb->buflen\n");
+	if(kvvb2->bufsize == kvvb->bufsize)
+		printf("PASS: kvvb2->bufsize == kvvb->bufsize");
 	if (kvvb2->buflen == kvvb->buflen && kvvb2->bufsize == kvvb->bufsize &&
 	        !memcmp(kvvb2->buf, kvvb->buf, kvvb->bufsize)) {
 		printf("PASS: kvvec -> buf -> kvvec conversion works flawlessly\n");
@@ -131,6 +142,9 @@ int main(int argc, char **argv) {
 
 	free(kvvb->buf);
 	free(kvvb);
+	free(kvvb2->buf);
+	free(kvvb2)
 	kvvec_destroy(kvv, 1);
+	kvvec_destroy(kvv3, KVVEC_FREE_ALL);
 	return 0;
 }
diff --git a/lib/worker.c b/lib/worker.c
index d65dbd4..7666f19 100644
--- a/lib/worker.c
+++ b/lib/worker.c
@@ -578,7 +578,8 @@ static int receive_command(int sd, int events, void *discard) {
 	 */
 	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);
 	}





More information about the icinga-checkins mailing list