From ed0571f89577d77de6c042454f61f9c055230095 Mon Sep 17 00:00:00 2001 From: Ross Delinger Date: Tue, 12 Jan 2016 16:08:38 -0500 Subject: [PATCH 1/3] Add a new build flag to disable the pool allocator and pass all git_pool_malloc calls straight to git__malloc --- CMakeLists.txt | 5 +++ src/pool.c | 108 +++++++++++++++++++++++++++++++++------------- src/pool.h | 31 +++++++++++++ tests/core/pool.c | 6 +++ 4 files changed, 120 insertions(+), 30 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 40a52bc01..7e5a88918 100644 --- a/CMakeLists.txt +++ b/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 ) diff --git a/src/pool.c b/src/pool.c index e519b75bb..b4fc50fca 100644 --- a/src/pool.c +++ b/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; -} diff --git a/src/pool.h b/src/pool.h index d16bd349a..0f9532db3 100644 --- a/src/pool.h +++ b/src/pool.h @@ -9,8 +9,13 @@ #include "common.h" +#ifdef GIT_DEBUG_POOL +#include "vector.h" +#endif + typedef struct git_pool_page git_pool_page; +#ifndef GIT_DEBUG_POOL /** * Chunked allocator. * @@ -33,6 +38,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 +127,9 @@ extern char *git_pool_strcat(git_pool *pool, const char *a, const char *b); /* * Misc utilities */ +#ifndef _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 diff --git a/tests/core/pool.c b/tests/core/pool.c index c43c1db67..b07da0abd 100644 --- a/tests/core/pool.c +++ b/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); } From f1260e03d976d6597cc0d6e37abe82eb9f093365 Mon Sep 17 00:00:00 2001 From: Ross Delinger Date: Fri, 19 Feb 2016 09:13:40 -0500 Subject: [PATCH 2/3] Remove unnecessary ifdef in pool.h --- src/pool.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/pool.h b/src/pool.h index 0f9532db3..1cae48fd3 100644 --- a/src/pool.h +++ b/src/pool.h @@ -8,10 +8,7 @@ #define INCLUDE_pool_h__ #include "common.h" - -#ifdef GIT_DEBUG_POOL #include "vector.h" -#endif typedef struct git_pool_page git_pool_page; From 93e16642280ab637f8688b8c2146b11f95f98325 Mon Sep 17 00:00:00 2001 From: Ross Delinger Date: Fri, 26 Feb 2016 12:51:13 -0500 Subject: [PATCH 3/3] Fixed typo in one of the ifndef's in pool.h used to enable/disable debug mode --- src/pool.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pool.h b/src/pool.h index 1cae48fd3..e0fafa997 100644 --- a/src/pool.h +++ b/src/pool.h @@ -124,7 +124,7 @@ extern char *git_pool_strcat(git_pool *pool, const char *a, const char *b); /* * Misc utilities */ -#ifndef _DEBUG_POOL +#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);