Merge pull request #3577 from rossdylan/rossdylan/pooldebug

Add a new build flag to disable the pool allocator

CMakeLists.txt

@@ -41,6 +41,11 @@ OPTION( USE_SSH				"Link with libssh to enable SSH support" ON )
 OPTION( USE_GSSAPI			"Link with libgssapi for SPNEGO auth"   OFF )
 OPTION( VALGRIND			"Configure build for valgrind"			OFF )
 OPTION( CURL			"User curl for HTTP if available" ON)
+OPTION( DEBUG_POOL			"Enable debug pool allocator"			OFF )
+
+IF(DEBUG_POOL)
+	ADD_DEFINITIONS(-DGIT_DEBUG_POOL)
+ENDIF()
 
 IF(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
 	SET( USE_ICONV ON )

src/pool.c

@@ -28,6 +28,7 @@ uint32_t git_pool__system_page_size(void)
 	return size;
 }
 
+#ifndef GIT_DEBUG_POOL
 void git_pool_init(git_pool *pool, uint32_t item_size)
 {
 	assert(pool);
@@ -50,18 +51,6 @@ void git_pool_clear(git_pool *pool)
 	pool->pages = NULL;
 }
 
-void git_pool_swap(git_pool *a, git_pool *b)
-{
-	git_pool temp;
-
-	if (a == b)
-		return;
-
-	memcpy(&temp, a, sizeof(temp));
-	memcpy(a, b, sizeof(temp));
-	memcpy(b, &temp, sizeof(temp));
-}
-
 static void *pool_alloc_page(git_pool *pool, uint32_t size)
 {
 	git_pool_page *page;
@@ -95,6 +84,83 @@ static void *pool_alloc(git_pool *pool, uint32_t size)
 	return ptr;
 }
 
+uint32_t git_pool__open_pages(git_pool *pool)
+{
+	uint32_t ct = 0;
+	git_pool_page *scan;
+	for (scan = pool->pages; scan != NULL; scan = scan->next) ct++;
+	return ct;
+}
+
+bool git_pool__ptr_in_pool(git_pool *pool, void *ptr)
+{
+	git_pool_page *scan;
+	for (scan = pool->pages; scan != NULL; scan = scan->next)
+		if ((void *)scan->data <= ptr &&
+			(void *)(((char *)scan->data) + scan->size) > ptr)
+			return true;
+	return false;
+}
+
+#else
+
+static int git_pool__ptr_cmp(const void * a, const void * b)
+{
+	if(a > b) {
+		return 1;
+	}
+	if(a < b) {
+		return -1;
+	}
+	else {
+		return 0;
+	}
+}
+
+void git_pool_init(git_pool *pool, uint32_t item_size)
+{
+	assert(pool);
+	assert(item_size >= 1);
+
+	memset(pool, 0, sizeof(git_pool));
+	pool->item_size = item_size;
+	pool->page_size = git_pool__system_page_size();
+	git_vector_init(&pool->allocations, 100, git_pool__ptr_cmp);
+}
+
+void git_pool_clear(git_pool *pool)
+{
+	git_vector_free_deep(&pool->allocations);
+}
+
+static void *pool_alloc(git_pool *pool, uint32_t size) {
+	void *ptr = NULL;
+	if((ptr = git__malloc(size)) == NULL) {
+		return NULL;
+	}
+	git_vector_insert_sorted(&pool->allocations, ptr, NULL);
+	return ptr;
+}
+
+bool git_pool__ptr_in_pool(git_pool *pool, void *ptr)
+{
+	size_t pos;
+	return git_vector_bsearch(&pos, &pool->allocations, ptr) != GIT_ENOTFOUND;
+}
+#endif
+
+void git_pool_swap(git_pool *a, git_pool *b)
+{
+	git_pool temp;
+
+	if (a == b)
+		return;
+
+	memcpy(&temp, a, sizeof(temp));
+	memcpy(a, b, sizeof(temp));
+	memcpy(b, &temp, sizeof(temp));
+}
+
 static uint32_t alloc_size(git_pool *pool, uint32_t count)
 {
 	const uint32_t align = sizeof(void *) - 1;
@@ -168,21 +234,3 @@ char *git_pool_strcat(git_pool *pool, const char *a, const char *b)
 	}
 	return ptr;
 }
-
-uint32_t git_pool__open_pages(git_pool *pool)
-{
-	uint32_t ct = 0;
-	git_pool_page *scan;
-	for (scan = pool->pages; scan != NULL; scan = scan->next) ct++;
-	return ct;
-}
-
-bool git_pool__ptr_in_pool(git_pool *pool, void *ptr)
-{
-	git_pool_page *scan;
-	for (scan = pool->pages; scan != NULL; scan = scan->next)
-		if ((void *)scan->data <= ptr &&
-			(void *)(((char *)scan->data) + scan->size) > ptr)
-			return true;
-	return false;
-}

src/pool.h

@@ -8,9 +8,11 @@
 #define INCLUDE_pool_h__
 
 #include "common.h"
+#include "vector.h"
 
 typedef struct git_pool_page git_pool_page;
 
+#ifndef GIT_DEBUG_POOL
 /**
  * Chunked allocator.
  *
@@ -33,6 +35,30 @@ typedef struct {
 	uint32_t page_size;  /* size of page in bytes */
 } git_pool;
 
+#else
+
+/**
+ * Debug chunked allocator.
+ *
+ * Acts just like `git_pool` but instead of actually pooling allocations it
+ * passes them through to `git__malloc`. This makes it possible to easily debug
+ * systems that use `git_pool` using valgrind.
+ *
+ * In order to track allocations during the lifetime of the pool we use a
+ * `git_vector`. When the pool is deallocated everything in the vector is
+ * freed.
+ *
+ * `API is exactly the same as the standard `git_pool` with one exception.
+ * Since we aren't allocating pages to hand out in chunks we can't easily
+ * implement `git_pool__open_pages`.
+ */
+typedef struct {
+	git_vector allocations;
+	uint32_t item_size;
+	uint32_t page_size;
+} git_pool;
+#endif
+
 /**
  * Initialize a pool.
  *
@@ -98,7 +124,9 @@ extern char *git_pool_strcat(git_pool *pool, const char *a, const char *b);
 /*
  * Misc utilities
  */
+#ifndef GIT_DEBUG_POOL
 extern uint32_t git_pool__open_pages(git_pool *pool);
+#endif
 extern bool git_pool__ptr_in_pool(git_pool *pool, void *ptr);
 
 #endif

tests/core/pool.c

@@ -31,8 +31,10 @@ void test_core_pool__1(void)
 	for (i = 2010; i > 0; i--)
 		cl_assert(git_pool_malloc(&p, i) != NULL);
 
+#ifndef GIT_DEBUG_POOL
 	/* with fixed page size, allocation must end up with these values */
 	cl_assert_equal_i(591, git_pool__open_pages(&p));
+#endif
 	git_pool_clear(&p);
 
 	git_pool_init(&p, 1);
@@ -41,8 +43,10 @@ void test_core_pool__1(void)
 	for (i = 2010; i > 0; i--)
 		cl_assert(git_pool_malloc(&p, i) != NULL);
 
+#ifndef GIT_DEBUG_POOL
 	/* with fixed page size, allocation must end up with these values */
 	cl_assert_equal_i(sizeof(void *) == 8 ? 575 : 573, git_pool__open_pages(&p));
+#endif
 	git_pool_clear(&p);
 }
 
@@ -69,8 +73,10 @@ void test_core_pool__2(void)
 		cl_git_pass(git_oid_fromstr(oid, oid_hex));
 	}
 
+#ifndef GIT_DEBUG_POOL
 	/* with fixed page size, allocation must end up with these values */
 	cl_assert_equal_i(sizeof(void *) == 8 ? 55 : 45, git_pool__open_pages(&p));
+#endif
 	git_pool_clear(&p);
 }