From 571762565da7f1686d1d9e882d5d8dd54c430e3f Mon Sep 17 00:00:00 2001
From: Dmitry Simonenko <pmwkaa@gmail.com>
Date: Wed, 25 Sep 2013 13:12:03 +0400
Subject: [PATCH] shared-arena: review fixes, slab_q moved to separate file.

---
 include/qbuf.h   | 102 +++++++++++++++++++++++++++++++++++++++++++++++
 include/salloc.h |   4 +-
 src/box/tuple.cc |   2 +-
 src/salloc.cc    |  93 +++++-------------------------------------
 src/tarantool.cc |   2 +-
 5 files changed, 117 insertions(+), 86 deletions(-)
 create mode 100644 include/qbuf.h

diff --git a/include/qbuf.h b/include/qbuf.h
new file mode 100644
index 0000000000..3feb743171
--- /dev/null
+++ b/include/qbuf.h
@@ -0,0 +1,102 @@
+#ifndef TARANTOOL_QBUF_H_INCLUDED
+#define TARANTOOL_QBUF_H_INCLUDED
+/*
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <stdlib.h>
+
+#define QBUF_WATERMARK (512 * sizeof(void*))
+
+struct qbuf {
+	char *buf;
+	size_t bottom; /* advanced by batch free */
+	size_t top;
+	size_t size;   /* total buffer size */
+};
+
+static inline int
+qbuf_init(struct qbuf *q, size_t size) {
+	q->size = size;
+	q->bottom = 0;
+	q->top = 0;
+	q->buf = (char*)malloc(size);
+	return (q->buf == NULL ? -1 : 0);
+}
+
+static inline void
+qbuf_free(struct qbuf *q) {
+	if (q->buf) {
+		free(q->buf);
+		q->buf = NULL;
+	}
+}
+
+static inline int
+qbuf_n(struct qbuf *q) {
+	return (q->top - q->bottom) / sizeof(void*);
+}
+
+#ifndef unlikely
+# define unlikely __builtin_expect(!! (EXPR), 0)
+#endif
+
+static inline int
+qbuf_push(struct qbuf *q, void *ptr)
+{
+	/* reduce memory allocation and memmove
+	 * effect by reusing free pointers buffer space only after the
+	 * watermark frees reached. */
+	if (unlikely(q->bottom >= QBUF_WATERMARK)) {
+		memmove(q->buf, q->buf + q->bottom, q->bottom);
+		q->top -= q->bottom;
+		q->bottom = 0;
+	}
+	if (unlikely((q->top + sizeof(void*)) > q->size)) {
+		size_t newsize = q->size * 2;
+		char *ptr = (char*)realloc((void*)q->buf, newsize);
+		if (unlikely(ptr == NULL))
+			return -1;
+		q->buf = ptr;
+		q->size = newsize;
+	}
+	memcpy(q->buf + q->top, (char*)&ptr, sizeof(ptr));
+	q->top += sizeof(void*);
+	return 0;
+}
+
+static inline void*
+qbuf_pop(struct qbuf *q) {
+	if (unlikely(q->bottom == q->top))
+		return NULL;
+	void *ret = *(void**)(q->buf + q->bottom);
+	q->bottom += sizeof(void*);
+	return ret;
+}
+
+#endif
diff --git a/include/salloc.h b/include/salloc.h
index 0f8d3662f9..fe43308f9d 100644
--- a/include/salloc.h
+++ b/include/salloc.h
@@ -37,10 +37,10 @@ struct tbuf;
 bool salloc_init(size_t size, size_t minimal, double factor);
 void salloc_free(void);
 void *salloc(size_t size, const char *what);
-void sfree(void *ptr, const char *what);
+void sfree(void *ptr);
 void slab_validate();
 
-void salloc_reattach(void);
+void salloc_protect(void);
 void salloc_batch_mode(bool mode);
 
 /** Statistics on utilization of a single slab class. */
diff --git a/src/box/tuple.cc b/src/box/tuple.cc
index 581920fd7c..0daec17367 100644
--- a/src/box/tuple.cc
+++ b/src/box/tuple.cc
@@ -229,7 +229,7 @@ tuple_free(struct tuple *tuple)
 	say_debug("tuple_free(%p)", tuple);
 	assert(tuple->refs == 0);
 	char *ptr = (char *) tuple - tuple_format(tuple)->field_map_size;
-	sfree(ptr, "tuple");
+	sfree(ptr);
 }
 
 /**
diff --git a/src/salloc.cc b/src/salloc.cc
index 3ba155dd3a..0a66d1d85f 100644
--- a/src/salloc.cc
+++ b/src/salloc.cc
@@ -40,6 +40,7 @@
 #include <third_party/queue.h>
 #include "tarantool/util.h"
 #include <tbuf.h>
+#include <qbuf.h>
 #include <say.h>
 #include "exception.h"
 
@@ -60,77 +61,6 @@ static const size_t MAX_SLAB_ITEM = 1 << 20;
 /* updated in slab_classes_init, depends on salloc_init params */
 size_t MAX_SLAB_ITEM_COUNT;
 
-/*
- *  slab delayed free queue.
-*/
-#define SLAB_Q_WATERMARK (512 * sizeof(void*))
-
-struct slab_q {
-	char *buf;
-	size_t bottom; /* advanced by batch free */
-	size_t top;
-	size_t size;   /* total buffer size */
-};
-
-static inline int
-slab_qinit(struct slab_q *q, size_t size) {
-	q->size = size;
-	q->bottom = 0;
-	q->top = 0;
-	q->buf = (char*)malloc(size);
-	return (q->buf == NULL ? -1 : 0);
-}
-
-static inline void
-slab_qfree(struct slab_q *q) {
-	if (q->buf) {
-		free(q->buf);
-		q->buf = NULL;
-	}
-}
-
-#ifndef unlikely
-# define unlikely __builtin_expect(!! (EXPR), 0)
-#endif
-
-static inline int
-slab_qpush(struct slab_q *q, void *ptr)
-{
-	/* reduce memory allocation and memmove
-	 * effect by reusing free pointers buffer space only after the
-	 * watermark frees reached. */
-	if (unlikely(q->bottom >= SLAB_Q_WATERMARK)) {
-		memmove(q->buf, q->buf + q->bottom, q->bottom);
-		q->top -= q->bottom;
-		q->bottom = 0;
-	}
-	if (unlikely((q->top + sizeof(void*)) > q->size)) {
-		size_t newsize = q->size * 2;
-		char *ptr = (char*)realloc((void*)q->buf, newsize);
-		if (unlikely(ptr == NULL))
-			return -1;
-		q->buf = ptr;
-		q->size = newsize;
-	}
-	memcpy(q->buf + q->top, (char*)&ptr, sizeof(ptr));
-	q->top += sizeof(void*);
-	return 0;
-}
-
-static inline int
-slab_qn(struct slab_q *q) {
-	return (q->top - q->bottom) / sizeof(void*);
-}
-
-static inline void*
-slab_qpop(struct slab_q *q) {
-	if (unlikely(q->bottom == q->top))
-		return NULL;
-	void *ret = *(void**)(q->buf + q->bottom);
-	q->bottom += sizeof(void*);
-	return ret;
-}
-
 struct slab_item {
 	struct slab_item *next;
 };
@@ -161,7 +91,7 @@ struct arena {
 	size_t mmap_size;
 	bool delayed_free_mode;
 	size_t delayed_free_batch;
-	struct slab_q delayed_q;
+	struct qbuf delayed_q;
 	void *base;
 	size_t size;
 	size_t used;
@@ -207,7 +137,7 @@ arena_init(struct arena *arena, size_t size)
 	arena->delayed_free_mode = false;
 	arena->delayed_free_batch = 100;
 
-	int rc = slab_qinit(&arena->delayed_q, 4096);
+	int rc = qbuf_init(&arena->delayed_q, 4096);
 	if (rc == -1)
 		return false;
 
@@ -219,7 +149,7 @@ arena_init(struct arena *arena, size_t size)
 				PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0);
 	if (arena->mmap_base == MAP_FAILED) {
 		say_syserror("mmap");
-		slab_qfree(&arena->delayed_q);
+		qbuf_free(&arena->delayed_q);
 		return false;
 	}
 
@@ -230,7 +160,7 @@ arena_init(struct arena *arena, size_t size)
 	return true;
 }
 
-void salloc_reattach(void) {
+void salloc_protect(void) {
 	mprotect(arena.mmap_base, arena.mmap_size, PROT_READ);
 }
 
@@ -271,7 +201,7 @@ salloc_free(void)
 {
 	if (arena.mmap_base != NULL)
 		munmap(arena.mmap_base, arena.mmap_size);
-	slab_qfree(&arena.delayed_q);
+	qbuf_free(&arena.delayed_q);
 	memset(&arena, 0, sizeof(struct arena));
 }
 
@@ -389,23 +319,22 @@ static void
 sfree_batch(void)
 {
 	ssize_t batch = arena.delayed_free_batch;
-	size_t n = slab_qn(&arena.delayed_q);
+	size_t n = qbuf_n(&arena.delayed_q);
 	while (batch-- > 0 && n-- > 0) {
-		void *ptr = slab_qpop(&arena.delayed_q);
+		void *ptr = qbuf_pop(&arena.delayed_q);
 		assert(ptr != NULL);
 		sfree_do(ptr);
 	}
 }
 
 void
-sfree(void *ptr, const char *what)
+sfree(void *ptr)
 {
 	if (ptr == NULL)
 		return;
 	if (arena.delayed_free_mode) {
-		if (slab_qpush(&arena.delayed_q, ptr) == -1)
-			tnt_raise(LoggedError, ER_MEMORY_ISSUE, arena.delayed_q.size * 2,
-				  "slab allocator", what);
+		if (qbuf_push(&arena.delayed_q, ptr) == -1)
+			panic("failed to add tuple to the delayed queue");
 		return;
 	}
 	sfree_batch();
diff --git a/src/tarantool.cc b/src/tarantool.cc
index 22860a1dfe..0954edb2c8 100644
--- a/src/tarantool.cc
+++ b/src/tarantool.cc
@@ -345,7 +345,7 @@ snapshot(void)
 		return (WIFSIGNALED(status) ? EINTR : WEXITSTATUS(status));
 	}
 
-	salloc_reattach();
+	salloc_protect();
 
 	fiber_set_name(fiber, "dumper");
 	set_proc_title("dumper (%" PRIu32 ")", getppid());
-- 
GitLab