{"thread":{"id":"48107","subject":"[PATCH 0/3] Extract memory pool logic into reusable component","startedAt":"2018-03-21T16:42:22Z","lastAt":"2018-04-17T16:43:09Z","messageCount":29,"participants":["jameson.miller81@gmail.com","Junio C Hamano","Jameson Miller","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"342420","messageId":"20180321164152.204869-1-jamill@microsoft.com","threadId":"48107","inReplyTo":null,"subject":"[PATCH 0/3] Extract memory pool logic into reusable component","fromName":"","fromEmail":"jameson.miller81@gmail.com","sentAt":"2018-03-21T16:41:49Z","receivedAt":"2018-03-21T16:42:22Z","isPatch":true,"sender":{"key":"jameson.miller81@gmail.com","avatar":null},"body":"From: Jameson Miller \n\nThis patch series extracts the memory pool implementation, currently\nused by fast-import, into a generalized component. This memory pool\ncan then be generally used by any component that needs a pool of\nmemory.\n\nThis patch is in preparation for a change to speed up the loading of\nindexes with a large number of cache entries by reducing the number of\nmalloc() calls. For a rough example of how this component will be used\nin this capacity, please see [1].\n\n[1] https://github.com/jamill/git/compare/block_allocation_base...jamill:block_allocation\n\nJameson Miller (3):\n fast-import: rename mem_pool to fi_mem_pool\n Introduce a reusable memory pool type\n fast-import: use built-in mem pool\n\n Makefile | 1 +\n fast-import.c | 50 ++++------------------\n mem-pool.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n mem-pool.h | 48 ++++++++++++++++++++++\n 4 files changed, 186 insertions(+), 43 deletions(-)\n create mode 100644 mem-pool.c\n create mode 100644 mem-pool.h\n\n-- \n2.14.3\n\n"},{"id":"342421","messageId":"20180321164152.204869-2-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH 1/3] fast-import: rename mem_pool to fi_mem_pool","fromName":"","fromEmail":"jameson.miller81@gmail.com","sentAt":"2018-03-21T16:41:50Z","receivedAt":"2018-03-21T16:42:27Z","isPatch":true,"sender":{"key":"jameson.miller81@gmail.com","avatar":null},"body":"From: Jameson Miller \n\nRename the mem_pool variables and structs in fast-import.c that will\nconflict with an upcoming global mem_pool type.\n\nSigned-off-by: Jameson Miller \n---\n fast-import.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 58ef360da4..4e68acc156 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -209,8 +209,8 @@ struct last_object {\n \tunsigned no_swap : 1;\n };\n \n-struct mem_pool {\n-\tstruct mem_pool *next_pool;\n+struct fi_mem_pool {\n+\tstruct fi_mem_pool *next_pool;\n \tchar *next_free;\n \tchar *end;\n \tuintmax_t space[FLEX_ARRAY]; /* more */\n@@ -304,9 +304,9 @@ static int global_argc;\n static const char **global_argv;\n \n /* Memory pools */\n-static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);\n+static size_t fi_mem_pool_alloc = 2*1024*1024 - sizeof(struct fi_mem_pool);\n static size_t total_allocd;\n-static struct mem_pool *mem_pool;\n+static struct fi_mem_pool *mem_pool;\n \n /* Atom management */\n static unsigned int atom_table_sz = 4451;\n@@ -636,7 +636,7 @@ static unsigned int hc_str(const char *s, size_t len)\n \n static void *pool_alloc(size_t len)\n {\n-\tstruct mem_pool *p;\n+\tstruct fi_mem_pool *p;\n \tvoid *r;\n \n \t/* round up to a 'uintmax_t' alignment */\n@@ -648,15 +648,15 @@ static void *pool_alloc(size_t len)\n \t\t\tbreak;\n \n \tif (!p) {\n-\t\tif (len >= (mem_pool_alloc/2)) {\n+\t\tif (len >= (fi_mem_pool_alloc/2)) {\n \t\t\ttotal_allocd += len;\n \t\t\treturn xmalloc(len);\n \t\t}\n-\t\ttotal_allocd += sizeof(struct mem_pool) + mem_pool_alloc;\n-\t\tp = xmalloc(st_add(sizeof(struct mem_pool), mem_pool_alloc));\n+\t\ttotal_allocd += sizeof(struct fi_mem_pool) + fi_mem_pool_alloc;\n+\t\tp = xmalloc(st_add(sizeof(struct fi_mem_pool), fi_mem_pool_alloc));\n \t\tp->next_pool = mem_pool;\n \t\tp->next_free = (char *) p->space;\n-\t\tp->end = p->next_free + mem_pool_alloc;\n+\t\tp->end = p->next_free + fi_mem_pool_alloc;\n \t\tmem_pool = p;\n \t}\n \n-- \n2.14.3\n\n"},{"id":"342422","messageId":"20180321164152.204869-4-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH 3/3] fast-import: use built-in mem pool","fromName":"","fromEmail":"jameson.miller81@gmail.com","sentAt":"2018-03-21T16:41:52Z","receivedAt":"2018-03-21T16:43:33Z","isPatch":true,"sender":{"key":"jameson.miller81@gmail.com","avatar":null},"body":"From: Jameson Miller \n\nSigned-off-by: Jameson Miller \n---\n fast-import.c | 50 +++++++-------------------------------------------\n 1 file changed, 7 insertions(+), 43 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 4e68acc156..126f2da118 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -168,6 +168,7 @@ Format of STDIN stream:\n #include \"dir.h\"\n #include \"run-command.h\"\n #include \"packfile.h\"\n+#include \"mem-pool.h\"\n \n #define PACK_ID_BITS 16\n #define MAX_PACK_ID ((1<next_pool)\n-\t\tif ((p->end - p->next_free >= len))\n-\t\t\tbreak;\n-\n-\tif (!p) {\n-\t\tif (len >= (fi_mem_pool_alloc/2)) {\n-\t\t\ttotal_allocd += len;\n-\t\t\treturn xmalloc(len);\n-\t\t}\n-\t\ttotal_allocd += sizeof(struct fi_mem_pool) + fi_mem_pool_alloc;\n-\t\tp = xmalloc(st_add(sizeof(struct fi_mem_pool), fi_mem_pool_alloc));\n-\t\tp->next_pool = mem_pool;\n-\t\tp->next_free = (char *) p->space;\n-\t\tp->end = p->next_free + fi_mem_pool_alloc;\n-\t\tmem_pool = p;\n-\t}\n-\n-\tr = p->next_free;\n-\tp->next_free += len;\n-\treturn r;\n+\treturn mem_pool_alloc(&mem_pool, len);\n }\n \n static void *pool_calloc(size_t count, size_t size)\n {\n-\tsize_t len = count * size;\n-\tvoid *r = pool_alloc(len);\n-\tmemset(r, 0, len);\n-\treturn r;\n+\treturn mem_pool_calloc(&mem_pool, count, size);\n }\n \n static char *pool_strdup(const char *s)\n@@ -3541,8 +3505,8 @@ int cmd_main(int argc, const char **argv)\n \t\tfprintf(stderr, \"Total branches: %10lu (%10lu loads )\\n\", branch_count, branch_load_count);\n \t\tfprintf(stderr, \" marks: %10\" PRIuMAX \" (%10\" PRIuMAX \" unique )\\n\", (((uintmax_t)1) << marks->shift) * 1024, marks_set_count);\n \t\tfprintf(stderr, \" atoms: %10u\\n\", atom_cnt);\n-\t\tfprintf(stderr, \"Memory total: %10\" PRIuMAX \" KiB\\n\", (total_allocd + alloc_count*sizeof(struct object_entry))/1024);\n-\t\tfprintf(stderr, \" pools: %10lu KiB\\n\", (unsigned long)(total_allocd/1024));\n+\t\tfprintf(stderr, \"Memory total: %10\" PRIuMAX \" KiB\\n\", (total_allocd + mem_pool.pool_alloc + alloc_count*sizeof(struct object_entry))/1024);\n+\t\tfprintf(stderr, \" pools: %10lu KiB\\n\", (unsigned long)((total_allocd + mem_pool.pool_alloc) /1024));\n \t\tfprintf(stderr, \" objects: %10\" PRIuMAX \" KiB\\n\", (alloc_count*sizeof(struct object_entry))/1024);\n \t\tfprintf(stderr, \"---------------------------------------------------------------------\\n\");\n \t\tpack_report();\n-- \n2.14.3\n\n"},{"id":"342423","messageId":"20180321164152.204869-3-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH 2/3] Introduce a reusable memory pool type","fromName":"","fromEmail":"jameson.miller81@gmail.com","sentAt":"2018-03-21T16:41:51Z","receivedAt":"2018-03-21T16:43:35Z","isPatch":true,"sender":{"key":"jameson.miller81@gmail.com","avatar":null},"body":"From: Jameson Miller \n\nExtract the existing memory pool logic used by fast-import into a\ngeneralized component. This memory pool component can then be used by\nother components that need this functionality.\n\nSigned-off-by: Jameson Miller \n---\n Makefile | 1 +\n mem-pool.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n mem-pool.h | 48 +++++++++++++++++++++++\n 3 files changed, 179 insertions(+)\n create mode 100644 mem-pool.c\n create mode 100644 mem-pool.h\n\ndiff --git a/Makefile b/Makefile\nindex a1d8775adb..1e142b1dd9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -832,6 +832,7 @@ LIB_OBJS += lockfile.o\n LIB_OBJS += log-tree.o\n LIB_OBJS += mailinfo.o\n LIB_OBJS += mailmap.o\n+LIB_OBJS += mem-pool.o\n LIB_OBJS += match-trees.o\n LIB_OBJS += merge.o\n LIB_OBJS += merge-blobs.o\ndiff --git a/mem-pool.c b/mem-pool.c\nnew file mode 100644\nindex 0000000000..3028bc3c67\n--- /dev/null\n+++ b/mem-pool.c\n@@ -0,0 +1,130 @@\n+/*\n+ * Memory Pool implementation logic.\n+ */\n+\n+#include \"cache.h\"\n+#include \"mem-pool.h\"\n+\n+#define MIN_ALLOC_GROWTH_SIZE 1024 * 1024\n+\n+struct mp_block {\n+\tstruct mp_block *next_block;\n+\tchar *next_free;\n+\tchar *end;\n+\tuintmax_t space[FLEX_ARRAY];\n+};\n+\n+static struct mp_block *mem_pool_alloc_block(struct mem_pool *mem_pool, size_t block_alloc)\n+{\n+\tstruct mp_block *p;\n+\n+\t/* Round up to a 'uintmax_t' alignment */\n+\tif (block_alloc & (sizeof(uintmax_t) - 1))\n+\t\tblock_alloc += sizeof(uintmax_t) - (block_alloc & (sizeof(uintmax_t) - 1));\n+\n+\tmem_pool->pool_alloc += block_alloc;\n+\n+\tp = xmalloc(st_add(sizeof(struct mp_block), block_alloc));\n+\n+\tp->next_block = mem_pool->mp_block;\n+\tp->next_free = (char *)p->space;\n+\tp->end = p->next_free + block_alloc;\n+\tmem_pool->mp_block = p;\n+\n+\treturn p;\n+}\n+\n+void mem_pool_init(struct mem_pool **mem_pool, size_t block_alloc, size_t initial_size)\n+{\n+\tif (!(*mem_pool))\n+\t{\n+\t\tif (block_alloc < MIN_ALLOC_GROWTH_SIZE)\n+\t\t\tblock_alloc = MIN_ALLOC_GROWTH_SIZE;\n+\n+\t\t*mem_pool = xmalloc(sizeof(struct mem_pool));\n+\t\t(*mem_pool)->pool_alloc = 0;\n+\t\t(*mem_pool)->mp_block = 0;\n+\t\t(*mem_pool)->block_alloc = block_alloc;\n+\n+\t\tif (initial_size > 0)\n+\t\t\tmem_pool_alloc_block((*mem_pool), initial_size);\n+\t}\n+}\n+\n+void mem_pool_discard(struct mem_pool *mem_pool)\n+{\n+\tstruct mp_block *block, *block_to_free;\n+\tfor (block = mem_pool->mp_block; block;)\n+\t{\n+\t\tblock_to_free = block;\n+\t\tblock = block->next_block;\n+\t\tfree(block_to_free);\n+\t}\n+\n+\tfree(mem_pool);\n+}\n+\n+void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)\n+{\n+\tstruct mp_block *p;\n+\tvoid *r;\n+\n+\t/* Round up to a 'uintmax_t' alignment */\n+\tif (len & (sizeof(uintmax_t) - 1))\n+\t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n+\n+\tp = mem_pool->mp_block;\n+\n+\tif (p &&\n+\t (p->end - p->next_free < len)) {\n+\t\tfor (p = p->next_block; p; p = p->next_block)\n+\t\t\tif (p->end - p->next_free >= len)\n+\t\t\t\tbreak;\n+\t}\n+\n+\tif (!p) {\n+\t\tif (len >= ((mem_pool->block_alloc - sizeof(struct mp_block)) / 2)) {\n+\t\t\tp = mem_pool_alloc_block(mem_pool, len);\n+\t\t}\n+\t\telse\n+\t\t\tp = mem_pool_alloc_block(mem_pool, mem_pool->block_alloc);\n+\t}\n+\n+\tr = p->next_free;\n+\tp->next_free += len;\n+\treturn r;\n+}\n+\n+void *mem_pool_calloc(struct mem_pool *mem_pool, size_t count, size_t size)\n+{\n+\tsize_t len = st_mult(count, size);\n+\tvoid *r = mem_pool_alloc(mem_pool, len);\n+\tmemset(r, 0, len);\n+\treturn r;\n+}\n+\n+int mem_pool_contains(struct mem_pool *mem_pool, void *mem)\n+{\n+\tstruct mp_block *p;\n+\tfor (p = mem_pool->mp_block; p; p = p->next_block)\n+\t\tif ((mem >= ((void *)p->space)) &&\n+\t\t (mem < ((void *)p->end)))\n+\t\t\treturn 1;\n+\n+\treturn 0;\n+}\n+\n+void mem_pool_combine(struct mem_pool *dst, struct mem_pool *src)\n+{\n+\tstruct mp_block **tail = &dst->mp_block;\n+\t/* find pointer of dst's last block (if any) */\n+\twhile (*tail)\n+\t\ttail = &(*tail)->next_block;\n+\n+\t/* append the blocks from src to dst */\n+\t*tail = src->mp_block;\n+\n+\tdst->pool_alloc += src->pool_alloc;\n+\tsrc->pool_alloc = 0;\n+\tsrc->mp_block = NULL;\n+}\ndiff --git a/mem-pool.h b/mem-pool.h\nnew file mode 100644\nindex 0000000000..902ef8caf2\n--- /dev/null\n+++ b/mem-pool.h\n@@ -0,0 +1,48 @@\n+#ifndef MEM_POOL_H\n+#define MEM_POOL_H\n+\n+struct mem_pool {\n+\tstruct mp_block *mp_block;\n+\n+\t/* The size of new blocks to grow the pool by. */\n+\tsize_t block_alloc;\n+\n+\t/* The total amount of memory allocated by the pool. */\n+\tsize_t pool_alloc;\n+};\n+\n+/*\n+ * Initialize mem_pool with specified parameters for initial size and\n+ * how much to grow when a larger memory block is required.\n+ */\n+void mem_pool_init(struct mem_pool **mem_pool, size_t alloc_growth_size, size_t initial_size);\n+\n+/*\n+ * Move the memory associated with the 'src' pool to the 'dst' pool. The 'src'\n+ * pool will be empty and not contain any memory. It still needs to be free'd\n+ * with a call to `mem_pool_discard`.\n+ */\n+void mem_pool_combine(struct mem_pool *dst, struct mem_pool *src);\n+\n+/*\n+ * Discard a memory pool and free all the memory it is responsible for.\n+ */\n+void mem_pool_discard(struct mem_pool *mem_pool);\n+\n+/*\n+ * Alloc memory from the mem_pool.\n+ */\n+void *mem_pool_alloc(struct mem_pool *pool, size_t len);\n+\n+/*\n+ * Allocate and zero memory from the memory pool.\n+ */\n+void *mem_pool_calloc(struct mem_pool *pool, size_t count, size_t size);\n+\n+/*\n+ * Check if a memory pointed at by 'mem' is part of the range of\n+ * memory managed by the specified mem_pool.\n+ */\n+int mem_pool_contains(struct mem_pool *mem_pool, void *mem);\n+\n+#endif\n-- \n2.14.3\n\n"},{"id":"342444","messageId":"xmqq8talp6u9.fsf@gitster-ct.c.googlers.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"Re: [PATCH 0/3] Extract memory pool logic into reusable component","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-21T19:27:26Z","receivedAt":"2018-03-21T19:27:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"jameson.miller81@gmail.com writes:\n\n> This patch series extracts the memory pool implementation, currently\n> used by fast-import, into a generalized component. This memory pool\n> can then be generally used by any component that needs a pool of\n> memory.\n\nThe way this series is organized is quite reviewer hostile. \n\nStep #2 that adds bunch of new code without introducing any caller\nnor adjusting existing code means the readers will have no idea how\nthe new code relates to the existing code in fast-import.c (e.g. \"Is\nit just a rename of some type, structure fields, function names\netc., and moving the lines without no other change?\" \"Is the new\ncode more capable than fast-import's one and does step #3 just makes\nthe existing fast-import callers use just a subset of the feature?\").\n\nEither case, if I were doing this (by the way, I am not in\nparticular an expert in this area), I probably would make existing\nmemtool implementation more generic (e.g. renaming, feature\ngeneralization, etc.) and adjust existing callers while the code is\nstill in its original place in one step, and then in a separate step\nmove the resulting reusable bits to a new file wilt a new header.\n\n\n\n"},{"id":"342583","messageId":"20180323144408.213145-1-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v2 0/3] Extract memory pool logic into reusable component","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-03-23T14:44:03Z","receivedAt":"2018-03-23T14:44:55Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"Changes from v1:\n - Rework the structure of commits to be more reviewer friendly.\n\nThis patch series extracts the memory pool implementation, currently\nused by fast-import, into a generalized component. This memory pool\ncan then be generally used by any component that needs a pool of\nmemory.\n\nThis patch is in preparation for a change to speed up the loading of\nindexes with a large number of cache entries by reducing the number of\nmalloc() calls. For a rough example of how this component will be used\nin this capacity, please see [1].\n\nThe last commit in this series includes changes to fill out the\nmem_pool type into a more complete implementation. These functions are\nnot used in this change, but we will be used in an upcoming patch\nseries. I can remove this commit if it is desired.\n\n[1] https://github.com/jamill/git/compare/block_allocation_base...jamill:block_allocation\n\nJameson Miller (5):\n fast-import: rename mem_pool type to mp_block\n fast-import: introduce mem_pool type\n fast-import: update pool_* functions to work on local pool\n Move the reusable parts of memory pool into its own file\n Expand implementation of mem-pool type\n\n Makefile | 1 +\n fast-import.c | 74 +++++----------------------\n mem-pool.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n mem-pool.h | 58 +++++++++++++++++++++\n 4 files changed, 234 insertions(+), 60 deletions(-)\n create mode 100644 mem-pool.c\n create mode 100644 mem-pool.h\n\n-- \n2.14.3\n\n"},{"id":"342584","messageId":"20180323144408.213145-2-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v2 1/5] fast-import: rename mem_pool type to mp_block","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-03-23T14:44:04Z","receivedAt":"2018-03-23T14:45:09Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"This is part of a patch series to extract the memory pool logic in\nfast-import into a more generalized version. The existing mem_pool type\nmaps more closely to a \"block of memory\" (mp_block) in the more\ngeneralized memory pool. This commit renames the mem_pool to mp_block to\nreduce churn in future patches.\n\nSigned-off-by: Jameson Miller \n---\n fast-import.c | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 58ef360da4..6c3215d7c3 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -209,8 +209,8 @@ struct last_object {\n \tunsigned no_swap : 1;\n };\n \n-struct mem_pool {\n-\tstruct mem_pool *next_pool;\n+struct mp_block {\n+\tstruct mp_block *next_block;\n \tchar *next_free;\n \tchar *end;\n \tuintmax_t space[FLEX_ARRAY]; /* more */\n@@ -304,9 +304,9 @@ static int global_argc;\n static const char **global_argv;\n \n /* Memory pools */\n-static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);\n+static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mp_block);\n static size_t total_allocd;\n-static struct mem_pool *mem_pool;\n+static struct mp_block *mp_block_head;\n \n /* Atom management */\n static unsigned int atom_table_sz = 4451;\n@@ -636,14 +636,14 @@ static unsigned int hc_str(const char *s, size_t len)\n \n static void *pool_alloc(size_t len)\n {\n-\tstruct mem_pool *p;\n+\tstruct mp_block *p;\n \tvoid *r;\n \n \t/* round up to a 'uintmax_t' alignment */\n \tif (len & (sizeof(uintmax_t) - 1))\n \t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n \n-\tfor (p = mem_pool; p; p = p->next_pool)\n+\tfor (p = mp_block_head; p; p = p->next_block)\n \t\tif ((p->end - p->next_free >= len))\n \t\t\tbreak;\n \n@@ -652,12 +652,12 @@ static void *pool_alloc(size_t len)\n \t\t\ttotal_allocd += len;\n \t\t\treturn xmalloc(len);\n \t\t}\n-\t\ttotal_allocd += sizeof(struct mem_pool) + mem_pool_alloc;\n-\t\tp = xmalloc(st_add(sizeof(struct mem_pool), mem_pool_alloc));\n-\t\tp->next_pool = mem_pool;\n+\t\ttotal_allocd += sizeof(struct mp_block) + mem_pool_alloc;\n+\t\tp = xmalloc(st_add(sizeof(struct mp_block), mem_pool_alloc));\n+\t\tp->next_block = mp_block_head;\n \t\tp->next_free = (char *) p->space;\n \t\tp->end = p->next_free + mem_pool_alloc;\n-\t\tmem_pool = p;\n+\t\tmp_block_head = p;\n \t}\n \n \tr = p->next_free;\n-- \n2.14.3\n\n"},{"id":"342585","messageId":"20180323144408.213145-3-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v2 2/5] fast-import: introduce mem_pool type","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-03-23T14:44:05Z","receivedAt":"2018-03-23T14:45:10Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"Introduce the mem_pool type and wrap the existing mp_block in this new\ntype. The new mem_pool type will allow for the memory pool logic to be\nreused outside of fast-import. This type will be moved into its own file\nin a future commit.\n\nSigned-off-by: Jameson Miller \n---\n fast-import.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++-----------\n 1 file changed, 89 insertions(+), 19 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 6c3215d7c3..1262d9e6be 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -216,6 +216,19 @@ struct mp_block {\n \tuintmax_t space[FLEX_ARRAY]; /* more */\n };\n \n+struct mem_pool {\n+\tstruct mp_block *mp_block;\n+\n+\t/*\n+\t * The amount of available memory to grow the pool by.\n+\t * This size does not include the overhead for the mp_block.\n+\t */\n+\tsize_t block_alloc;\n+\n+\t/* The total amount of memory allocated by the pool. */\n+\tsize_t pool_alloc;\n+};\n+\n struct atom_str {\n \tstruct atom_str *next_atom;\n \tunsigned short str_len;\n@@ -304,9 +317,7 @@ static int global_argc;\n static const char **global_argv;\n \n /* Memory pools */\n-static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mp_block);\n-static size_t total_allocd;\n-static struct mp_block *mp_block_head;\n+static struct mem_pool fi_mem_pool = {0, 2*1024*1024 - sizeof(struct mp_block), 0 };\n \n /* Atom management */\n static unsigned int atom_table_sz = 4451;\n@@ -324,6 +335,7 @@ static off_t pack_size;\n /* Table of objects we've written. */\n static unsigned int object_entry_alloc = 5000;\n static struct object_entry_pool *blocks;\n+static size_t total_allocd = 0;\n static struct object_entry *object_table[1 << 16];\n static struct mark_set *marks;\n static const char *export_marks_file;\n@@ -634,6 +646,60 @@ static unsigned int hc_str(const char *s, size_t len)\n \treturn r;\n }\n \n+static struct mp_block *pool_alloc_block()\n+{\n+\tstruct mp_block *p;\n+\n+\tfi_mem_pool.pool_alloc += sizeof(struct mp_block) + fi_mem_pool.block_alloc;\n+\tp = xmalloc(st_add(sizeof(struct mp_block), fi_mem_pool.block_alloc));\n+\tp->next_block = fi_mem_pool.mp_block;\n+\tp->next_free = (char *)p->space;\n+\tp->end = p->next_free + fi_mem_pool.block_alloc;\n+\tfi_mem_pool.mp_block = p;\n+\n+\treturn p;\n+}\n+\n+/*\n+ * Allocates a block of memory with a specific size and\n+ * appends it to the memory pool's list of blocks.\n+ *\n+ * This function is used to allocate blocks with sizes\n+ * different than the default \"block_alloc\" size for the mem_pool.\n+ *\n+ * There are two use cases:\n+ * 1) The initial block allocation for a memory pool.\n+ *\n+ * 2) large\" blocks of a specific size, where the entire memory block\n+ * is going to be used. This means the block will not have any\n+ * available memory for future allocations. The block is placed at\n+ * the end of the list so that it will be the last block searched\n+ * for available space.\n+ */\n+static struct mp_block *pool_alloc_block_with_size(size_t block_alloc)\n+{\n+\tstruct mp_block *p, *block;\n+\n+\tfi_mem_pool.pool_alloc += sizeof(struct mp_block) + block_alloc;\n+\tp = xmalloc(st_add(sizeof(struct mp_block), block_alloc));\n+\n+\tblock = fi_mem_pool.mp_block;\n+\tif (block) {\n+\t\twhile (block->next_block)\n+\t\t\tblock = block->next_block;\n+\n+\t\tblock->next_block = p;\n+\t} else {\n+\t\tfi_mem_pool.mp_block = p;\n+\t}\n+\n+\tp->next_block = NULL;\n+\tp->next_free = (char *)p->space;\n+\tp->end = p->next_free + block_alloc;\n+\n+\treturn p;\n+}\n+\n static void *pool_alloc(size_t len)\n {\n \tstruct mp_block *p;\n@@ -643,21 +709,25 @@ static void *pool_alloc(size_t len)\n \tif (len & (sizeof(uintmax_t) - 1))\n \t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n \n-\tfor (p = mp_block_head; p; p = p->next_block)\n-\t\tif ((p->end - p->next_free >= len))\n-\t\t\tbreak;\n+\tp = fi_mem_pool.mp_block;\n+\n+\t/*\n+\t * In performance profiling, there was a minor perf benefit to\n+\t * check for available memory in the head block via the if\n+\t * statement, and only going through the loop when needed.\n+\t */\n+\tif (p &&\n+\t (p->end - p->next_free < len)) {\n+\t\tfor (p = p->next_block; p; p = p->next_block)\n+\t\t\tif (p->end - p->next_free >= len)\n+\t\t\t\tbreak;\n+\t}\n \n \tif (!p) {\n-\t\tif (len >= (mem_pool_alloc/2)) {\n-\t\t\ttotal_allocd += len;\n-\t\t\treturn xmalloc(len);\n-\t\t}\n-\t\ttotal_allocd += sizeof(struct mp_block) + mem_pool_alloc;\n-\t\tp = xmalloc(st_add(sizeof(struct mp_block), mem_pool_alloc));\n-\t\tp->next_block = mp_block_head;\n-\t\tp->next_free = (char *) p->space;\n-\t\tp->end = p->next_free + mem_pool_alloc;\n-\t\tmp_block_head = p;\n+\t\tif (len >= (fi_mem_pool.block_alloc / 2))\n+\t\t\tp = pool_alloc_block_with_size(len);\n+\t\telse\n+\t\t\tp = pool_alloc_block();\n \t}\n \n \tr = p->next_free;\n@@ -667,7 +737,7 @@ static void *pool_alloc(size_t len)\n \n static void *pool_calloc(size_t count, size_t size)\n {\n-\tsize_t len = count * size;\n+\tsize_t len = st_mult(count, size);\n \tvoid *r = pool_alloc(len);\n \tmemset(r, 0, len);\n \treturn r;\n@@ -3541,8 +3611,8 @@ int cmd_main(int argc, const char **argv)\n \t\tfprintf(stderr, \"Total branches: %10lu (%10lu loads )\\n\", branch_count, branch_load_count);\n \t\tfprintf(stderr, \" marks: %10\" PRIuMAX \" (%10\" PRIuMAX \" unique )\\n\", (((uintmax_t)1) << marks->shift) * 1024, marks_set_count);\n \t\tfprintf(stderr, \" atoms: %10u\\n\", atom_cnt);\n-\t\tfprintf(stderr, \"Memory total: %10\" PRIuMAX \" KiB\\n\", (total_allocd + alloc_count*sizeof(struct object_entry))/1024);\n-\t\tfprintf(stderr, \" pools: %10lu KiB\\n\", (unsigned long)(total_allocd/1024));\n+\t\tfprintf(stderr, \"Memory total: %10\" PRIuMAX \" KiB\\n\", (total_allocd + fi_mem_pool.pool_alloc + alloc_count*sizeof(struct object_entry))/1024);\n+\t\tfprintf(stderr, \" pools: %10lu KiB\\n\", (unsigned long)((total_allocd + fi_mem_pool.pool_alloc) /1024));\n \t\tfprintf(stderr, \" objects: %10\" PRIuMAX \" KiB\\n\", (alloc_count*sizeof(struct object_entry))/1024);\n \t\tfprintf(stderr, \"---------------------------------------------------------------------\\n\");\n \t\tpack_report();\n-- \n2.14.3\n\n"},{"id":"342586","messageId":"20180323144408.213145-4-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v2 3/5] fast-import: update pool_* functions to work on local pool","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-03-23T14:44:06Z","receivedAt":"2018-03-23T14:45:13Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"Update memory pool functions to work on a passed in mem_pool instead of\nthe global mem_pool type. This is in preparation for making the memory\npool logic reusable.\n\nSigned-off-by: Jameson Miller \n---\n fast-import.c | 52 ++++++++++++++++++++++++++--------------------------\n 1 file changed, 26 insertions(+), 26 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 1262d9e6be..519e1cbd7f 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -646,16 +646,16 @@ static unsigned int hc_str(const char *s, size_t len)\n \treturn r;\n }\n \n-static struct mp_block *pool_alloc_block()\n+static struct mp_block *mem_pool_alloc_block(struct mem_pool *mem_pool)\n {\n \tstruct mp_block *p;\n \n-\tfi_mem_pool.pool_alloc += sizeof(struct mp_block) + fi_mem_pool.block_alloc;\n-\tp = xmalloc(st_add(sizeof(struct mp_block), fi_mem_pool.block_alloc));\n-\tp->next_block = fi_mem_pool.mp_block;\n+\tmem_pool->pool_alloc += sizeof(struct mp_block) + mem_pool->block_alloc;\n+\tp = xmalloc(st_add(sizeof(struct mp_block), mem_pool->block_alloc));\n+\tp->next_block = mem_pool->mp_block;\n \tp->next_free = (char *)p->space;\n-\tp->end = p->next_free + fi_mem_pool.block_alloc;\n-\tfi_mem_pool.mp_block = p;\n+\tp->end = p->next_free + mem_pool->block_alloc;\n+\tmem_pool->mp_block = p;\n \n \treturn p;\n }\n@@ -676,21 +676,21 @@ static struct mp_block *pool_alloc_block()\n * the end of the list so that it will be the last block searched\n * for available space.\n */\n-static struct mp_block *pool_alloc_block_with_size(size_t block_alloc)\n+static struct mp_block *mem_pool_alloc_block_with_size(struct mem_pool *mem_pool, size_t block_alloc)\n {\n \tstruct mp_block *p, *block;\n \n-\tfi_mem_pool.pool_alloc += sizeof(struct mp_block) + block_alloc;\n+\tmem_pool->pool_alloc += sizeof(struct mp_block) + block_alloc;\n \tp = xmalloc(st_add(sizeof(struct mp_block), block_alloc));\n \n-\tblock = fi_mem_pool.mp_block;\n+\tblock = mem_pool->mp_block;\n \tif (block) {\n \t\twhile (block->next_block)\n \t\t\tblock = block->next_block;\n \n \t\tblock->next_block = p;\n \t} else {\n-\t\tfi_mem_pool.mp_block = p;\n+\t\tmem_pool->mp_block = p;\n \t}\n \n \tp->next_block = NULL;\n@@ -700,7 +700,7 @@ static struct mp_block *pool_alloc_block_with_size(size_t block_alloc)\n \treturn p;\n }\n \n-static void *pool_alloc(size_t len)\n+static void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)\n {\n \tstruct mp_block *p;\n \tvoid *r;\n@@ -709,7 +709,7 @@ static void *pool_alloc(size_t len)\n \tif (len & (sizeof(uintmax_t) - 1))\n \t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n \n-\tp = fi_mem_pool.mp_block;\n+\tp = mem_pool->mp_block;\n \n \t/*\n \t * In performance profiling, there was a minor perf benefit to\n@@ -724,10 +724,10 @@ static void *pool_alloc(size_t len)\n \t}\n \n \tif (!p) {\n-\t\tif (len >= (fi_mem_pool.block_alloc / 2))\n-\t\t\tp = pool_alloc_block_with_size(len);\n+\t\tif (len >= (mem_pool->block_alloc / 2))\n+\t\t\tp = mem_pool_alloc_block_with_size(mem_pool, len);\n \t\telse\n-\t\t\tp = pool_alloc_block();\n+\t\t\tp = mem_pool_alloc_block(mem_pool);\n \t}\n \n \tr = p->next_free;\n@@ -735,10 +735,10 @@ static void *pool_alloc(size_t len)\n \treturn r;\n }\n \n-static void *pool_calloc(size_t count, size_t size)\n+static void *mem_pool_calloc(struct mem_pool *mem_pool, size_t count, size_t size)\n {\n \tsize_t len = st_mult(count, size);\n-\tvoid *r = pool_alloc(len);\n+\tvoid *r = mem_pool_alloc(mem_pool, len);\n \tmemset(r, 0, len);\n \treturn r;\n }\n@@ -746,7 +746,7 @@ static void *pool_calloc(size_t count, size_t size)\n static char *pool_strdup(const char *s)\n {\n \tsize_t len = strlen(s) + 1;\n-\tchar *r = pool_alloc(len);\n+\tchar *r = mem_pool_alloc(&fi_mem_pool, len);\n \tmemcpy(r, s, len);\n \treturn r;\n }\n@@ -755,7 +755,7 @@ static void insert_mark(uintmax_t idnum, struct object_entry *oe)\n {\n \tstruct mark_set *s = marks;\n \twhile ((idnum >> s->shift) >= 1024) {\n-\t\ts = pool_calloc(1, sizeof(struct mark_set));\n+\t\ts = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n \t\ts->shift = marks->shift + 10;\n \t\ts->data.sets[0] = marks;\n \t\tmarks = s;\n@@ -764,7 +764,7 @@ static void insert_mark(uintmax_t idnum, struct object_entry *oe)\n \t\tuintmax_t i = idnum >> s->shift;\n \t\tidnum -= i << s->shift;\n \t\tif (!s->data.sets[i]) {\n-\t\t\ts->data.sets[i] = pool_calloc(1, sizeof(struct mark_set));\n+\t\t\ts->data.sets[i] = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n \t\t\ts->data.sets[i]->shift = s->shift - 10;\n \t\t}\n \t\ts = s->data.sets[i];\n@@ -802,7 +802,7 @@ static struct atom_str *to_atom(const char *s, unsigned short len)\n \t\tif (c->str_len == len && !strncmp(s, c->str_dat, len))\n \t\t\treturn c;\n \n-\tc = pool_alloc(sizeof(struct atom_str) + len + 1);\n+\tc = mem_pool_alloc(&fi_mem_pool, sizeof(struct atom_str) + len + 1);\n \tc->str_len = len;\n \tmemcpy(c->str_dat, s, len);\n \tc->str_dat[len] = 0;\n@@ -833,7 +833,7 @@ static struct branch *new_branch(const char *name)\n \tif (check_refname_format(name, REFNAME_ALLOW_ONELEVEL))\n \t\tdie(\"Branch name doesn't conform to GIT standards: %s\", name);\n \n-\tb = pool_calloc(1, sizeof(struct branch));\n+\tb = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct branch));\n \tb->name = pool_strdup(name);\n \tb->table_next_branch = branch_table[hc];\n \tb->branch_tree.versions[0].mode = S_IFDIR;\n@@ -869,7 +869,7 @@ static struct tree_content *new_tree_content(unsigned int cnt)\n \t\t\tavail_tree_table[hc] = f->next_avail;\n \t} else {\n \t\tcnt = cnt & 7 ? ((cnt / 8) + 1) * 8 : cnt;\n-\t\tf = pool_alloc(sizeof(*t) + sizeof(t->entries[0]) * cnt);\n+\t\tf = mem_pool_alloc(&fi_mem_pool, sizeof(*t) + sizeof(t->entries[0]) * cnt);\n \t\tf->entry_capacity = cnt;\n \t}\n \n@@ -2932,7 +2932,7 @@ static void parse_new_tag(const char *arg)\n \tenum object_type type;\n \tconst char *v;\n \n-\tt = pool_alloc(sizeof(struct tag));\n+\tt = mem_pool_alloc(&fi_mem_pool, sizeof(struct tag));\n \tmemset(t, 0, sizeof(struct tag));\n \tt->name = pool_strdup(arg);\n \tif (last_tag)\n@@ -3531,12 +3531,12 @@ int cmd_main(int argc, const char **argv)\n \tatom_table = xcalloc(atom_table_sz, sizeof(struct atom_str*));\n \tbranch_table = xcalloc(branch_table_sz, sizeof(struct branch*));\n \tavail_tree_table = xcalloc(avail_tree_table_sz, sizeof(struct avail_tree_content*));\n-\tmarks = pool_calloc(1, sizeof(struct mark_set));\n+\tmarks = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n \n \tglobal_argc = argc;\n \tglobal_argv = argv;\n \n-\trc_free = pool_alloc(cmd_save * sizeof(*rc_free));\n+\trc_free = mem_pool_alloc(&fi_mem_pool, cmd_save * sizeof(*rc_free));\n \tfor (i = 0; i < (cmd_save - 1); i++)\n \t\trc_free[i].next = &rc_free[i + 1];\n \trc_free[cmd_save - 1].next = NULL;\n-- \n2.14.3\n\n"},{"id":"342587","messageId":"20180323144408.213145-6-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v2 5/5] Expand implementation of mem-pool type","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-03-23T14:44:08Z","receivedAt":"2018-03-23T14:45:14Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"This commit adds functionality to the mem-pool type that can be\ngenerally useful. This functionality will be used in a future commit.\n\nSigned-off-by: Jameson Miller \n---\n mem-pool.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n mem-pool.h | 24 ++++++++++++++++++++++++\n 2 files changed, 82 insertions(+)\n\ndiff --git a/mem-pool.c b/mem-pool.c\nindex 992e354e12..7d21a7e035 100644\n--- a/mem-pool.c\n+++ b/mem-pool.c\n@@ -5,6 +5,8 @@\n #include \"cache.h\"\n #include \"mem-pool.h\"\n \n+#define MIN_ALLOC_GROWTH_SIZE 1024 * 1024\n+\n static struct mp_block *mem_pool_alloc_block(struct mem_pool *mem_pool)\n {\n \tstruct mp_block *p;\n@@ -59,6 +61,36 @@ static struct mp_block *mem_pool_alloc_block_with_size(struct mem_pool *mem_pool\n \treturn p;\n }\n \n+void mem_pool_init(struct mem_pool **mem_pool, size_t block_alloc, size_t initial_size)\n+{\n+\tif (!(*mem_pool))\n+\t{\n+\t\tif (block_alloc < (MIN_ALLOC_GROWTH_SIZE - sizeof(struct mp_block)))\n+\t\t\tblock_alloc = (MIN_ALLOC_GROWTH_SIZE - sizeof(struct mp_block));\n+\n+\t\t*mem_pool = xmalloc(sizeof(struct mem_pool));\n+\t\t(*mem_pool)->pool_alloc = 0;\n+\t\t(*mem_pool)->mp_block = 0;\n+\t\t(*mem_pool)->block_alloc = block_alloc;\n+\n+\t\tif (initial_size > 0)\n+\t\t\tmem_pool_alloc_block_with_size(*mem_pool, initial_size);\n+\t}\n+}\n+\n+void mem_pool_discard(struct mem_pool *mem_pool)\n+{\n+\tstruct mp_block *block, *block_to_free;\n+\tfor (block = mem_pool->mp_block; block;)\n+\t{\n+\t\tblock_to_free = block;\n+\t\tblock = block->next_block;\n+\t\tfree(block_to_free);\n+\t}\n+\n+\tfree(mem_pool);\n+}\n+\n void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)\n {\n \tstruct mp_block *p;\n@@ -101,3 +133,29 @@ void *mem_pool_calloc(struct mem_pool *mem_pool, size_t count, size_t size)\n \tmemset(r, 0, len);\n \treturn r;\n }\n+\n+int mem_pool_contains(struct mem_pool *mem_pool, void *mem)\n+{\n+\tstruct mp_block *p;\n+\tfor (p = mem_pool->mp_block; p; p = p->next_block)\n+\t\tif ((mem >= ((void *)p->space)) &&\n+\t\t (mem < ((void *)p->end)))\n+\t\t\treturn 1;\n+\n+\treturn 0;\n+}\n+\n+void mem_pool_combine(struct mem_pool *dst, struct mem_pool *src)\n+{\n+\tstruct mp_block **tail = &dst->mp_block;\n+\t/* find pointer of dst's last block (if any) */\n+\twhile (*tail)\n+\t\ttail = &(*tail)->next_block;\n+\n+\t/* append the blocks from src to dst */\n+\t*tail = src->mp_block;\n+\n+\tdst->pool_alloc += src->pool_alloc;\n+\tsrc->pool_alloc = 0;\n+\tsrc->mp_block = NULL;\n+}\ndiff --git a/mem-pool.h b/mem-pool.h\nindex 829ad58ecf..d9e7f21541 100644\n--- a/mem-pool.h\n+++ b/mem-pool.h\n@@ -21,6 +21,17 @@ struct mem_pool {\n \tsize_t pool_alloc;\n };\n \n+/*\n+ * Initialize mem_pool with specified parameters for initial size and\n+ * how much to grow when a larger memory block is required.\n+ */\n+void mem_pool_init(struct mem_pool **mem_pool, size_t alloc_growth_size, size_t initial_size);\n+\n+/*\n+ * Discard a memory pool and free all the memory it is responsible for.\n+ */\n+void mem_pool_discard(struct mem_pool *mem_pool);\n+\n /*\n * Alloc memory from the mem_pool.\n */\n@@ -31,4 +42,17 @@ void *mem_pool_alloc(struct mem_pool *pool, size_t len);\n */\n void *mem_pool_calloc(struct mem_pool *pool, size_t count, size_t size);\n \n+/*\n+ * Move the memory associated with the 'src' pool to the 'dst' pool. The 'src'\n+ * pool will be empty and not contain any memory. It still needs to be free'd\n+ * with a call to `mem_pool_discard`.\n+ */\n+void mem_pool_combine(struct mem_pool *dst, struct mem_pool *src);\n+\n+/*\n+ * Check if a memory pointed at by 'mem' is part of the range of\n+ * memory managed by the specified mem_pool.\n+ */\n+int mem_pool_contains(struct mem_pool *mem_pool, void *mem);\n+\n #endif\n-- \n2.14.3\n\n"},{"id":"342588","messageId":"20180323144408.213145-5-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v2 4/5] Move the reusable parts of memory pool into its own file","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-03-23T14:44:07Z","receivedAt":"2018-03-23T14:45:18Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"This moves the reusable parts of the memory pool logic used by\nfast-import.c into its own file for use by other components.\n\nSigned-off-by: Jameson Miller \n---\n Makefile | 1 +\n fast-import.c | 118 +---------------------------------------------------------\n mem-pool.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++\n mem-pool.h | 34 +++++++++++++++++\n 4 files changed, 139 insertions(+), 117 deletions(-)\n create mode 100644 mem-pool.c\n create mode 100644 mem-pool.h\n\ndiff --git a/Makefile b/Makefile\nindex a1d8775adb..1e142b1dd9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -832,6 +832,7 @@ LIB_OBJS += lockfile.o\n LIB_OBJS += log-tree.o\n LIB_OBJS += mailinfo.o\n LIB_OBJS += mailmap.o\n+LIB_OBJS += mem-pool.o\n LIB_OBJS += match-trees.o\n LIB_OBJS += merge.o\n LIB_OBJS += merge-blobs.o\ndiff --git a/fast-import.c b/fast-import.c\nindex 519e1cbd7f..36ac5760d6 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -168,6 +168,7 @@ Format of STDIN stream:\n #include \"dir.h\"\n #include \"run-command.h\"\n #include \"packfile.h\"\n+#include \"mem-pool.h\"\n \n #define PACK_ID_BITS 16\n #define MAX_PACK_ID ((1<pool_alloc += sizeof(struct mp_block) + mem_pool->block_alloc;\n-\tp = xmalloc(st_add(sizeof(struct mp_block), mem_pool->block_alloc));\n-\tp->next_block = mem_pool->mp_block;\n-\tp->next_free = (char *)p->space;\n-\tp->end = p->next_free + mem_pool->block_alloc;\n-\tmem_pool->mp_block = p;\n-\n-\treturn p;\n-}\n-\n-/*\n- * Allocates a block of memory with a specific size and\n- * appends it to the memory pool's list of blocks.\n- *\n- * This function is used to allocate blocks with sizes\n- * different than the default \"block_alloc\" size for the mem_pool.\n- *\n- * There are two use cases:\n- * 1) The initial block allocation for a memory pool.\n- *\n- * 2) large\" blocks of a specific size, where the entire memory block\n- * is going to be used. This means the block will not have any\n- * available memory for future allocations. The block is placed at\n- * the end of the list so that it will be the last block searched\n- * for available space.\n- */\n-static struct mp_block *mem_pool_alloc_block_with_size(struct mem_pool *mem_pool, size_t block_alloc)\n-{\n-\tstruct mp_block *p, *block;\n-\n-\tmem_pool->pool_alloc += sizeof(struct mp_block) + block_alloc;\n-\tp = xmalloc(st_add(sizeof(struct mp_block), block_alloc));\n-\n-\tblock = mem_pool->mp_block;\n-\tif (block) {\n-\t\twhile (block->next_block)\n-\t\t\tblock = block->next_block;\n-\n-\t\tblock->next_block = p;\n-\t} else {\n-\t\tmem_pool->mp_block = p;\n-\t}\n-\n-\tp->next_block = NULL;\n-\tp->next_free = (char *)p->space;\n-\tp->end = p->next_free + block_alloc;\n-\n-\treturn p;\n-}\n-\n-static void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)\n-{\n-\tstruct mp_block *p;\n-\tvoid *r;\n-\n-\t/* round up to a 'uintmax_t' alignment */\n-\tif (len & (sizeof(uintmax_t) - 1))\n-\t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n-\n-\tp = mem_pool->mp_block;\n-\n-\t/*\n-\t * In performance profiling, there was a minor perf benefit to\n-\t * check for available memory in the head block via the if\n-\t * statement, and only going through the loop when needed.\n-\t */\n-\tif (p &&\n-\t (p->end - p->next_free < len)) {\n-\t\tfor (p = p->next_block; p; p = p->next_block)\n-\t\t\tif (p->end - p->next_free >= len)\n-\t\t\t\tbreak;\n-\t}\n-\n-\tif (!p) {\n-\t\tif (len >= (mem_pool->block_alloc / 2))\n-\t\t\tp = mem_pool_alloc_block_with_size(mem_pool, len);\n-\t\telse\n-\t\t\tp = mem_pool_alloc_block(mem_pool);\n-\t}\n-\n-\tr = p->next_free;\n-\tp->next_free += len;\n-\treturn r;\n-}\n-\n-static void *mem_pool_calloc(struct mem_pool *mem_pool, size_t count, size_t size)\n-{\n-\tsize_t len = st_mult(count, size);\n-\tvoid *r = mem_pool_alloc(mem_pool, len);\n-\tmemset(r, 0, len);\n-\treturn r;\n-}\n-\n static char *pool_strdup(const char *s)\n {\n \tsize_t len = strlen(s) + 1;\ndiff --git a/mem-pool.c b/mem-pool.c\nnew file mode 100644\nindex 0000000000..992e354e12\n--- /dev/null\n+++ b/mem-pool.c\n@@ -0,0 +1,103 @@\n+/*\n+ * Memory Pool implementation logic.\n+ */\n+\n+#include \"cache.h\"\n+#include \"mem-pool.h\"\n+\n+static struct mp_block *mem_pool_alloc_block(struct mem_pool *mem_pool)\n+{\n+\tstruct mp_block *p;\n+\n+\tmem_pool->pool_alloc += sizeof(struct mp_block) + mem_pool->block_alloc;\n+\tp = xmalloc(st_add(sizeof(struct mp_block), mem_pool->block_alloc));\n+\tp->next_block = mem_pool->mp_block;\n+\tp->next_free = (char *)p->space;\n+\tp->end = p->next_free + mem_pool->block_alloc;\n+\tmem_pool->mp_block = p;\n+\n+\treturn p;\n+}\n+\n+/*\n+ * Allocates a block of memory with a specific size and\n+ * appends it to the memory pool's list of blocks.\n+ *\n+ * This function is used to allocate blocks with sizes\n+ * different than the default \"block_alloc\" size for the mem_pool.\n+ *\n+ * There are two use cases:\n+ * 1) The initial block allocation for a memory pool.\n+ *\n+ * 2) large\" blocks of a specific size, where the entire memory block\n+ * is going to be used. This means the block will not have any\n+ * available memory for future allocations. The block is placed at\n+ * the end of the list so that it will be the last block searched\n+ * for available space.\n+ */\n+static struct mp_block *mem_pool_alloc_block_with_size(struct mem_pool *mem_pool, size_t block_alloc)\n+{\n+\tstruct mp_block *p, *block;\n+\n+\tmem_pool->pool_alloc += sizeof(struct mp_block) + block_alloc;\n+\tp = xmalloc(st_add(sizeof(struct mp_block), block_alloc));\n+\n+\tblock = mem_pool->mp_block;\n+\tif (block) {\n+\t\twhile (block->next_block)\n+\t\t\tblock = block->next_block;\n+\n+\t\tblock->next_block = p;\n+\t} else {\n+\t\tmem_pool->mp_block = p;\n+\t}\n+\n+\tp->next_block = NULL;\n+\tp->next_free = (char *)p->space;\n+\tp->end = p->next_free + block_alloc;\n+\n+\treturn p;\n+}\n+\n+void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)\n+{\n+\tstruct mp_block *p;\n+\tvoid *r;\n+\n+\t/* round up to a 'uintmax_t' alignment */\n+\tif (len & (sizeof(uintmax_t) - 1))\n+\t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n+\n+\tp = mem_pool->mp_block;\n+\n+\t/*\n+\t * In performance profiling, there was a minor perf benefit to\n+\t * check for available memory in the head block via the if\n+\t * statement, and only going through the loop when needed.\n+\t */\n+\tif (p &&\n+\t (p->end - p->next_free < len)) {\n+\t\tfor (p = p->next_block; p; p = p->next_block)\n+\t\t\tif (p->end - p->next_free >= len)\n+\t\t\t\tbreak;\n+\t}\n+\n+\tif (!p) {\n+\t\tif (len >= (mem_pool->block_alloc / 2))\n+\t\t\tp = mem_pool_alloc_block_with_size(mem_pool, len);\n+\t\telse\n+\t\t\tp = mem_pool_alloc_block(mem_pool);\n+\t}\n+\n+\tr = p->next_free;\n+\tp->next_free += len;\n+\treturn r;\n+}\n+\n+void *mem_pool_calloc(struct mem_pool *mem_pool, size_t count, size_t size)\n+{\n+\tsize_t len = st_mult(count, size);\n+\tvoid *r = mem_pool_alloc(mem_pool, len);\n+\tmemset(r, 0, len);\n+\treturn r;\n+}\ndiff --git a/mem-pool.h b/mem-pool.h\nnew file mode 100644\nindex 0000000000..829ad58ecf\n--- /dev/null\n+++ b/mem-pool.h\n@@ -0,0 +1,34 @@\n+#ifndef MEM_POOL_H\n+#define MEM_POOL_H\n+\n+struct mp_block {\n+\tstruct mp_block *next_block;\n+\tchar *next_free;\n+\tchar *end;\n+\tuintmax_t space[FLEX_ARRAY]; /* more */\n+};\n+\n+struct mem_pool {\n+\tstruct mp_block *mp_block;\n+\n+\t/*\n+\t * The amount of available memory to grow the pool by.\n+\t * This size does not include the overhead for the mp_block.\n+\t */\n+\tsize_t block_alloc;\n+\n+\t/* The total amount of memory allocated by the pool. */\n+\tsize_t pool_alloc;\n+};\n+\n+/*\n+ * Alloc memory from the mem_pool.\n+ */\n+void *mem_pool_alloc(struct mem_pool *pool, size_t len);\n+\n+/*\n+ * Allocate and zero memory from the memory pool.\n+ */\n+void *mem_pool_calloc(struct mem_pool *pool, size_t count, size_t size);\n+\n+#endif\n-- \n2.14.3\n\n"},{"id":"342597","messageId":"xmqqvadmkak4.fsf@gitster-ct.c.googlers.com","threadId":"48107","inReplyTo":"20180323144408.213145-2-jamill@microsoft.com","subject":"Re: [PATCH v2 1/5] fast-import: rename mem_pool type to mp_block","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-23T16:42:51Z","receivedAt":"2018-03-23T16:43:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jameson Miller writes:\n\n> This is part of a patch series to extract the memory pool logic in\n> fast-import into a more generalized version. The existing mem_pool type\n> maps more closely to a \"block of memory\" (mp_block) in the more\n> generalized memory pool.\n\nSounds good.\n\n"},{"id":"342605","messageId":"xmqqk1u2k91l.fsf@gitster-ct.c.googlers.com","threadId":"48107","inReplyTo":"20180323144408.213145-3-jamill@microsoft.com","subject":"Re: [PATCH v2 2/5] fast-import: introduce mem_pool type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-23T17:15:34Z","receivedAt":"2018-03-23T17:15:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jameson Miller writes:\n\n> Introduce the mem_pool type and wrap the existing mp_block in this new\n> type. The new mem_pool type will allow for the memory pool logic to be\n> reused outside of fast-import. This type will be moved into its own file\n> in a future commit.\n>\n> Signed-off-by: Jameson Miller \n> ---\n> fast-import.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++-----------\n> 1 file changed, 89 insertions(+), 19 deletions(-)\n>\n> diff --git a/fast-import.c b/fast-import.c\n> index 6c3215d7c3..1262d9e6be 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -216,6 +216,19 @@ struct mp_block {\n> \tuintmax_t space[FLEX_ARRAY]; /* more */\n> };\n> \n> +struct mem_pool {\n> +\tstruct mp_block *mp_block;\n> +\n> +\t/*\n> +\t * The amount of available memory to grow the pool by.\n> +\t * This size does not include the overhead for the mp_block.\n> +\t */\n> +\tsize_t block_alloc;\n> +\n> +\t/* The total amount of memory allocated by the pool. */\n> +\tsize_t pool_alloc;\n> +};\n\nOK, so the existing fast-import one knows that it only needs a\nsingle instance, but by introducing this structure, we made it\npossible to have more than one instance of it, and more importantly,\nall the necessary pieces of info to manage it are available to those\nwho have a pointer to this instance.\n\n> struct atom_str {\n> \tstruct atom_str *next_atom;\n> \tunsigned short str_len;\n> @@ -304,9 +317,7 @@ static int global_argc;\n> static const char **global_argv;\n> \n> /* Memory pools */\n> -static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mp_block);\n> -static size_t total_allocd;\n> -static struct mp_block *mp_block_head;\n> +static struct mem_pool fi_mem_pool = {0, 2*1024*1024 - sizeof(struct mp_block), 0 };\n\nAnd this struct instance represents the for fast-import's use, which used to be separate\nglobal variables that are file-scope static here.\n\nGood.\n\n> /* Atom management */\n> static unsigned int atom_table_sz = 4451;\n> @@ -324,6 +335,7 @@ static off_t pack_size;\n> /* Table of objects we've written. */\n> static unsigned int object_entry_alloc = 5000;\n> static struct object_entry_pool *blocks;\n> +static size_t total_allocd = 0;\n\nThis one is now used as a total across all the mempool instances and\nis kept outside the struct, which is OK.\n\nDo not initialize a var in BSS with an explicit \"= 0\".\n\n> @@ -634,6 +646,60 @@ static unsigned int hc_str(const char *s, size_t len)\n> \treturn r;\n> }\n> \n> +static struct mp_block *pool_alloc_block()\n\nShould spell this as:\n\n\tstatic struct mp_block *pool_alloc_block(void)\n\nBut it is somewhat curious; if we are moving to make this reusable,\nshouldn't this be accepting \"struct mem_pool *pool\" from the caller\nand using pool->pool_alloc, pool->block_alloc, etc. instead of\nalways using the global instance, which fi_mem_pool is?\n\n> +{\n> +\tstruct mp_block *p;\n> +\n> +\tfi_mem_pool.pool_alloc += sizeof(struct mp_block) + fi_mem_pool.block_alloc;\n\nThis addition, just like the addition on the next line, potentially\noverflows and wraps-around, so it is probably a candidate for\nst_add(), but IIUC, pool_alloc is only for usage stats, so it may\nnot be too bad an offence. In any case, the above is done as close\nto the original and is good for a refactoring patch.\n\n> +\tp = xmalloc(st_add(sizeof(struct mp_block), fi_mem_pool.block_alloc));\n> +\tp->next_block = fi_mem_pool.mp_block;\n> +\tp->next_free = (char *)p->space;\n> +\tp->end = p->next_free + fi_mem_pool.block_alloc;\n> +\tfi_mem_pool.mp_block = p;\n> +\n> +\treturn p;\n> +}\n\nSo this is one half of the original pool_alloc(), that allocated a\nnew block and queued it to the pool when a not-too-large allocation\nis requested (the other half that dealt with large allocations\nbypassed the pool mechanism altogether).\n\n> +/*\n> + * Allocates a block of memory with a specific size and\n> + * appends it to the memory pool's list of blocks.\n> + *\n> + * This function is used to allocate blocks with sizes\n> + * different than the default \"block_alloc\" size for the mem_pool.\n> + *\n> + * There are two use cases:\n> + * 1) The initial block allocation for a memory pool.\n> + *\n> + * 2) large\" blocks of a specific size, where the entire memory block\n> + * is going to be used. This means the block will not have any\n> + * available memory for future allocations. The block is placed at\n> + * the end of the list so that it will be the last block searched\n> + * for available space.\n\nIf we know the block won't have any leftover space, what's the point\nof \"searching for available space\" in it in the first place, even if\nyou make it the last?\n\n> + */\n> +static struct mp_block *pool_alloc_block_with_size(size_t block_alloc)\n> +{\n\nSame comment on passing a \"pool\" instance as parameter applies to\nthis function.\n\n> +\tstruct mp_block *p, *block;\n> +\n> +\tfi_mem_pool.pool_alloc += sizeof(struct mp_block) + block_alloc;\n> +\tp = xmalloc(st_add(sizeof(struct mp_block), block_alloc));\n> +\n> +\tblock = fi_mem_pool.mp_block;\n> +\tif (block) {\n> +\t\twhile (block->next_block)\n> +\t\t\tblock = block->next_block;\n> +\n> +\t\tblock->next_block = p;\n\nIf there were a need to append to the end (and also a need to\niterate from the beginning), keep an extra pointer in the \"pool\"\ninstance to make it an O(1) operation. Or perhaps this list can be\nmanaged with list.h?\n\nIt is unclear why this and pool_alloc_block() need to exist as\nseparate functions. 80% of the code are shared between them.\n\n> +\t} else {\n> +\t\tfi_mem_pool.mp_block = p;\n> +\t}\n> +\n> +\tp->next_block = NULL;\n> +\tp->next_free = (char *)p->space;\n> +\tp->end = p->next_free + block_alloc;\n> +\n> +\treturn p;\n> +}\n> +\n> static void *pool_alloc(size_t len)\n> {\n> \tstruct mp_block *p;\n> @@ -643,21 +709,25 @@ static void *pool_alloc(size_t len)\n> \tif (len & (sizeof(uintmax_t) - 1))\n> \t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n> \n> -\tfor (p = mp_block_head; p; p = p->next_block)\n> -\t\tif ((p->end - p->next_free >= len))\n> -\t\t\tbreak;\n> +\tp = fi_mem_pool.mp_block;\n> +\n> +\t/*\n> +\t * In performance profiling, there was a minor perf benefit to\n> +\t * check for available memory in the head block via the if\n> +\t * statement, and only going through the loop when needed.\n> +\t */\n\nDon't mix premature optimization in a refactoring patch, please.\n\n> +\tif (p &&\n> +\t (p->end - p->next_free < len)) {\n> +\t\tfor (p = p->next_block; p; p = p->next_block)\n> +\t\t\tif (p->end - p->next_free >= len)\n> +\t\t\t\tbreak;\n> +\t}\n> \n> \tif (!p) {\n> -\t\tif (len >= (mem_pool_alloc/2)) {\n> -\t\t\ttotal_allocd += len;\n> -\t\t\treturn xmalloc(len);\n> -\t\t}\n> -\t\ttotal_allocd += sizeof(struct mp_block) + mem_pool_alloc;\n> -\t\tp = xmalloc(st_add(sizeof(struct mp_block), mem_pool_alloc));\n> -\t\tp->next_block = mp_block_head;\n> -\t\tp->next_free = (char *) p->space;\n> -\t\tp->end = p->next_free + mem_pool_alloc;\n> -\t\tmp_block_head = p;\n> +\t\tif (len >= (fi_mem_pool.block_alloc / 2))\n> +\t\t\tp = pool_alloc_block_with_size(len);\n> +\t\telse\n> +\t\t\tp = pool_alloc_block();\n\nSo we used to fulfill large request directly from xmalloc() but now\nwe create a new block, which is large enough to fulfill the request\nbut otherwise unusable for helping later requests, and queue that to\nthe mem_pool structure.\n\nIt _might_ be a justifiable change in behaviour, but is entirely\nunexpected from a refactoring patch that wants to \"wrap the existing\nmp_block in this new type\", especially without justifying why in the\nproposed log message.\n"},{"id":"342607","messageId":"xmqqfu4qk8uy.fsf@gitster-ct.c.googlers.com","threadId":"48107","inReplyTo":"20180323144408.213145-4-jamill@microsoft.com","subject":"Re: [PATCH v2 3/5] fast-import: update pool_* functions to work on local pool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-23T17:19:33Z","receivedAt":"2018-03-23T17:19:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jameson Miller writes:\n\n> Update memory pool functions to work on a passed in mem_pool instead of\n> the global mem_pool type. This is in preparation for making the memory\n> pool logic reusable.\n\nThis should be part of 2/5; the same comments to 2/5 I just sent out\nwould still apply to a patch that is a combined result between 2/5\nand 3/5.\n\nAlso, if mem_pool_alloc_block() etc. are the desired final names,\nusing them from the beginning would make the result easier to read\nthrough (both during review and later \"log -p\").\n\nThanks. This round, especially 2/5 (with anticipation of 3/5\nalready in reader's head), was easier to grok than the previous.\n\n"},{"id":"342677","messageId":"xmqqr2oaillr.fsf@gitster-ct.c.googlers.com","threadId":"48107","inReplyTo":"20180323144408.213145-5-jamill@microsoft.com","subject":"Re: [PATCH v2 4/5] Move the reusable parts of memory pool into its own file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-23T20:27:12Z","receivedAt":"2018-03-23T20:27:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jameson Miller writes:\n\n> This moves the reusable parts of the memory pool logic used by\n> fast-import.c into its own file for use by other components.\n>\n> Signed-off-by: Jameson Miller \n> ---\n> Makefile | 1 +\n> fast-import.c | 118 +---------------------------------------------------------\n> mem-pool.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++\n> mem-pool.h | 34 +++++++++++++++++\n> 4 files changed, 139 insertions(+), 117 deletions(-)\n> create mode 100644 mem-pool.c\n> create mode 100644 mem-pool.h\n\nThis step looks totally uncontroversial, provided that the result\nafter [1-3/5] is in a reasonable shape ;-)\n\nHint for other reviewers. This\n\n $ git blame -s -b -C HEAD^..HEAD mem-pool.c\n\ncan be used to see that most of the lines in this new file are\nimported verbatim from fast-import.c at the receiving end.\n\n\n\n"},{"id":"342679","messageId":"xmqqk1u2ikxt.fsf@gitster-ct.c.googlers.com","threadId":"48107","inReplyTo":"20180323144408.213145-6-jamill@microsoft.com","subject":"Re: [PATCH v2 5/5] Expand implementation of mem-pool type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-23T20:41:34Z","receivedAt":"2018-03-23T20:41:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jameson Miller writes:\n\n> +void mem_pool_discard(struct mem_pool *mem_pool)\n> +{\n> +\tstruct mp_block *block, *block_to_free;\n> +\tfor (block = mem_pool->mp_block; block;)\n> +\t{\n> +\t\tblock_to_free = block;\n> +\t\tblock = block->next_block;\n> +\t\tfree(block_to_free);\n> +\t}\n> +\n> +\tfree(mem_pool);\n> +}\n\nThis can be used as a (half-)valid justification to the unadvertised\nbehaviour change made in [2/5] where a large allocation is still\ngiven its own block and connected to a pool (as opposed to bypassing\nthe pool subsystem altogether). If an instance of pool does not\nkeep track of all the pieces of memory mem_pool_alloc() hands out,\nmem_pool_discard() cannot be used to reclaim all of the resources.\n\nIt however still does not justify how pool_alloc_block_with_size()\nappends the new block at the end of the \"next\" list, which is\nscanned for unused spaces when a new request comes in. \"We need to\nkeep track of them so that disard() can release\" would justify a\npointer in the pool struct that chains together allocation blocks,\neach of which hosts the memory for a single large allocation request\nwithout extra room to carve out memory for subsequent small\nrequests. That pointer does not have to be the usual \"mp_block\"\npointer.\n\n> diff --git a/mem-pool.h b/mem-pool.h\n> index 829ad58ecf..d9e7f21541 100644\n> --- a/mem-pool.h\n> +++ b/mem-pool.h\n> @@ -21,6 +21,17 @@ struct mem_pool {\n> \tsize_t pool_alloc;\n> };\n> \n> +/*\n> + * Initialize mem_pool with specified parameters for initial size and\n> + * how much to grow when a larger memory block is required.\n> + */\n> +void mem_pool_init(struct mem_pool **mem_pool, size_t alloc_growth_size, size_t initial_size);\n> +\n> +/*\n> + * Discard a memory pool and free all the memory it is responsible for.\n> + */\n> +void mem_pool_discard(struct mem_pool *mem_pool);\n> +\n> /*\n> * Alloc memory from the mem_pool.\n> */\n> @@ -31,4 +42,17 @@ void *mem_pool_alloc(struct mem_pool *pool, size_t len);\n> */\n> void *mem_pool_calloc(struct mem_pool *pool, size_t count, size_t size);\n> \n> +/*\n> + * Move the memory associated with the 'src' pool to the 'dst' pool. The 'src'\n> + * pool will be empty and not contain any memory. It still needs to be free'd\n> + * with a call to `mem_pool_discard`.\n> + */\n> +void mem_pool_combine(struct mem_pool *dst, struct mem_pool *src);\n> +\n> +/*\n> + * Check if a memory pointed at by 'mem' is part of the range of\n> + * memory managed by the specified mem_pool.\n> + */\n> +int mem_pool_contains(struct mem_pool *mem_pool, void *mem);\n> +\n> #endif\n\nWhen adding a new \"API\", we prefer to see its usage demonstrated\nwith its first user, either in the same patch or in later steps in\nthe same series. That would make it easier to evaluate its worth\nfairly.\n\nThanks.\n\n\n"},{"id":"343025","messageId":"20180326170351.197793-3-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v3 2/3] fast-import: introduce mem_pool type","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-03-26T17:03:50Z","receivedAt":"2018-03-26T17:04:38Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"Introduce the mem_pool type which encapsulates all the information\nnecessary to manage a pool of memory.This change moves the existing\nvariables in fast-import used to support the global memory pool to use\nthis structure.\n\nThese changes allow for the multiple instances of a memory pool to\nexist and be reused outside of fast-import. In a future commit the\nmem_pool type will be moved to its own file.\n\nSigned-off-by: Jameson Miller \n---\n fast-import.c | 80 +++++++++++++++++++++++++++++++++++++----------------------\n 1 file changed, 51 insertions(+), 29 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 6c3215d7c3..e85512191c 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -216,6 +216,19 @@ struct mp_block {\n \tuintmax_t space[FLEX_ARRAY]; /* more */\n };\n \n+struct mem_pool {\n+\tstruct mp_block *mp_block;\n+\n+\t/*\n+\t * The amount of available memory to grow the pool by.\n+\t * This size does not include the overhead for the mp_block.\n+\t */\n+\tsize_t block_alloc;\n+\n+\t/* The total amount of memory allocated by the pool. */\n+\tsize_t pool_alloc;\n+};\n+\n struct atom_str {\n \tstruct atom_str *next_atom;\n \tunsigned short str_len;\n@@ -304,9 +317,7 @@ static int global_argc;\n static const char **global_argv;\n \n /* Memory pools */\n-static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mp_block);\n-static size_t total_allocd;\n-static struct mp_block *mp_block_head;\n+static struct mem_pool fi_mem_pool = {0, 2*1024*1024 - sizeof(struct mp_block), 0 };\n \n /* Atom management */\n static unsigned int atom_table_sz = 4451;\n@@ -324,6 +335,7 @@ static off_t pack_size;\n /* Table of objects we've written. */\n static unsigned int object_entry_alloc = 5000;\n static struct object_entry_pool *blocks;\n+static size_t total_allocd;\n static struct object_entry *object_table[1 << 16];\n static struct mark_set *marks;\n static const char *export_marks_file;\n@@ -634,7 +646,21 @@ static unsigned int hc_str(const char *s, size_t len)\n \treturn r;\n }\n \n-static void *pool_alloc(size_t len)\n+static struct mp_block *mem_pool_alloc_block(struct mem_pool *mem_pool, size_t block_alloc)\n+{\n+\tstruct mp_block *p;\n+\n+\tmem_pool->pool_alloc += sizeof(struct mp_block) + block_alloc;\n+\tp = xmalloc(st_add(sizeof(struct mp_block), block_alloc));\n+\tp->next_block = mem_pool->mp_block;\n+\tp->next_free = (char *)p->space;\n+\tp->end = p->next_free + block_alloc;\n+\tmem_pool->mp_block = p;\n+\n+\treturn p;\n+}\n+\n+static void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)\n {\n \tstruct mp_block *p;\n \tvoid *r;\n@@ -643,21 +669,17 @@ static void *pool_alloc(size_t len)\n \tif (len & (sizeof(uintmax_t) - 1))\n \t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n \n-\tfor (p = mp_block_head; p; p = p->next_block)\n-\t\tif ((p->end - p->next_free >= len))\n-\t\t\tbreak;\n+\tfor (p = mem_pool->mp_block; p; p = p->next_block)\n+\t\tif (p->end - p->next_free >= len)\n+\t \t\tbreak;\n \n \tif (!p) {\n-\t\tif (len >= (mem_pool_alloc/2)) {\n-\t\t\ttotal_allocd += len;\n+\t\tif (len >= (mem_pool->block_alloc / 2)) {\n+\t\t\tmem_pool->pool_alloc += len;\n \t\t\treturn xmalloc(len);\n \t\t}\n-\t\ttotal_allocd += sizeof(struct mp_block) + mem_pool_alloc;\n-\t\tp = xmalloc(st_add(sizeof(struct mp_block), mem_pool_alloc));\n-\t\tp->next_block = mp_block_head;\n-\t\tp->next_free = (char *) p->space;\n-\t\tp->end = p->next_free + mem_pool_alloc;\n-\t\tmp_block_head = p;\n+\n+\t\tp = mem_pool_alloc_block(mem_pool, mem_pool->block_alloc);\n \t}\n \n \tr = p->next_free;\n@@ -665,10 +687,10 @@ static void *pool_alloc(size_t len)\n \treturn r;\n }\n \n-static void *pool_calloc(size_t count, size_t size)\n+static void *mem_pool_calloc(struct mem_pool *mem_pool, size_t count, size_t size)\n {\n-\tsize_t len = count * size;\n-\tvoid *r = pool_alloc(len);\n+\tsize_t len = st_mult(count, size);\n+\tvoid *r = mem_pool_alloc(mem_pool, len);\n \tmemset(r, 0, len);\n \treturn r;\n }\n@@ -676,7 +698,7 @@ static void *pool_calloc(size_t count, size_t size)\n static char *pool_strdup(const char *s)\n {\n \tsize_t len = strlen(s) + 1;\n-\tchar *r = pool_alloc(len);\n+\tchar *r = mem_pool_alloc(&fi_mem_pool, len);\n \tmemcpy(r, s, len);\n \treturn r;\n }\n@@ -685,7 +707,7 @@ static void insert_mark(uintmax_t idnum, struct object_entry *oe)\n {\n \tstruct mark_set *s = marks;\n \twhile ((idnum >> s->shift) >= 1024) {\n-\t\ts = pool_calloc(1, sizeof(struct mark_set));\n+\t\ts = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n \t\ts->shift = marks->shift + 10;\n \t\ts->data.sets[0] = marks;\n \t\tmarks = s;\n@@ -694,7 +716,7 @@ static void insert_mark(uintmax_t idnum, struct object_entry *oe)\n \t\tuintmax_t i = idnum >> s->shift;\n \t\tidnum -= i << s->shift;\n \t\tif (!s->data.sets[i]) {\n-\t\t\ts->data.sets[i] = pool_calloc(1, sizeof(struct mark_set));\n+\t\t\ts->data.sets[i] = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n \t\t\ts->data.sets[i]->shift = s->shift - 10;\n \t\t}\n \t\ts = s->data.sets[i];\n@@ -732,7 +754,7 @@ static struct atom_str *to_atom(const char *s, unsigned short len)\n \t\tif (c->str_len == len && !strncmp(s, c->str_dat, len))\n \t\t\treturn c;\n \n-\tc = pool_alloc(sizeof(struct atom_str) + len + 1);\n+\tc = mem_pool_alloc(&fi_mem_pool, sizeof(struct atom_str) + len + 1);\n \tc->str_len = len;\n \tmemcpy(c->str_dat, s, len);\n \tc->str_dat[len] = 0;\n@@ -763,7 +785,7 @@ static struct branch *new_branch(const char *name)\n \tif (check_refname_format(name, REFNAME_ALLOW_ONELEVEL))\n \t\tdie(\"Branch name doesn't conform to GIT standards: %s\", name);\n \n-\tb = pool_calloc(1, sizeof(struct branch));\n+\tb = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct branch));\n \tb->name = pool_strdup(name);\n \tb->table_next_branch = branch_table[hc];\n \tb->branch_tree.versions[0].mode = S_IFDIR;\n@@ -799,7 +821,7 @@ static struct tree_content *new_tree_content(unsigned int cnt)\n \t\t\tavail_tree_table[hc] = f->next_avail;\n \t} else {\n \t\tcnt = cnt & 7 ? ((cnt / 8) + 1) * 8 : cnt;\n-\t\tf = pool_alloc(sizeof(*t) + sizeof(t->entries[0]) * cnt);\n+\t\tf = mem_pool_alloc(&fi_mem_pool, sizeof(*t) + sizeof(t->entries[0]) * cnt);\n \t\tf->entry_capacity = cnt;\n \t}\n \n@@ -2862,7 +2884,7 @@ static void parse_new_tag(const char *arg)\n \tenum object_type type;\n \tconst char *v;\n \n-\tt = pool_alloc(sizeof(struct tag));\n+\tt = mem_pool_alloc(&fi_mem_pool, sizeof(struct tag));\n \tmemset(t, 0, sizeof(struct tag));\n \tt->name = pool_strdup(arg);\n \tif (last_tag)\n@@ -3461,12 +3483,12 @@ int cmd_main(int argc, const char **argv)\n \tatom_table = xcalloc(atom_table_sz, sizeof(struct atom_str*));\n \tbranch_table = xcalloc(branch_table_sz, sizeof(struct branch*));\n \tavail_tree_table = xcalloc(avail_tree_table_sz, sizeof(struct avail_tree_content*));\n-\tmarks = pool_calloc(1, sizeof(struct mark_set));\n+\tmarks = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n \n \tglobal_argc = argc;\n \tglobal_argv = argv;\n \n-\trc_free = pool_alloc(cmd_save * sizeof(*rc_free));\n+\trc_free = mem_pool_alloc(&fi_mem_pool, cmd_save * sizeof(*rc_free));\n \tfor (i = 0; i < (cmd_save - 1); i++)\n \t\trc_free[i].next = &rc_free[i + 1];\n \trc_free[cmd_save - 1].next = NULL;\n@@ -3541,8 +3563,8 @@ int cmd_main(int argc, const char **argv)\n \t\tfprintf(stderr, \"Total branches: %10lu (%10lu loads )\\n\", branch_count, branch_load_count);\n \t\tfprintf(stderr, \" marks: %10\" PRIuMAX \" (%10\" PRIuMAX \" unique )\\n\", (((uintmax_t)1) << marks->shift) * 1024, marks_set_count);\n \t\tfprintf(stderr, \" atoms: %10u\\n\", atom_cnt);\n-\t\tfprintf(stderr, \"Memory total: %10\" PRIuMAX \" KiB\\n\", (total_allocd + alloc_count*sizeof(struct object_entry))/1024);\n-\t\tfprintf(stderr, \" pools: %10lu KiB\\n\", (unsigned long)(total_allocd/1024));\n+\t\tfprintf(stderr, \"Memory total: %10\" PRIuMAX \" KiB\\n\", (total_allocd + fi_mem_pool.pool_alloc + alloc_count*sizeof(struct object_entry))/1024);\n+\t\tfprintf(stderr, \" pools: %10lu KiB\\n\", (unsigned long)((total_allocd + fi_mem_pool.pool_alloc) /1024));\n \t\tfprintf(stderr, \" objects: %10\" PRIuMAX \" KiB\\n\", (alloc_count*sizeof(struct object_entry))/1024);\n \t\tfprintf(stderr, \"---------------------------------------------------------------------\\n\");\n \t\tpack_report();\n-- \n2.14.3\n\n"},{"id":"343026","messageId":"20180326170351.197793-2-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v3 1/3] fast-import: rename mem_pool type to mp_block","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-03-26T17:03:49Z","receivedAt":"2018-03-26T17:04:40Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"This is part of a patch series to extract the memory pool logic in\nfast-import into a more generalized version. The existing mem_pool type\nmaps more closely to a \"block of memory\" (mp_block) in the more\ngeneralized memory pool. This commit renames the mem_pool to mp_block to\nreduce churn in future patches.\n\nSigned-off-by: Jameson Miller \n---\n fast-import.c | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 58ef360da4..6c3215d7c3 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -209,8 +209,8 @@ struct last_object {\n \tunsigned no_swap : 1;\n };\n \n-struct mem_pool {\n-\tstruct mem_pool *next_pool;\n+struct mp_block {\n+\tstruct mp_block *next_block;\n \tchar *next_free;\n \tchar *end;\n \tuintmax_t space[FLEX_ARRAY]; /* more */\n@@ -304,9 +304,9 @@ static int global_argc;\n static const char **global_argv;\n \n /* Memory pools */\n-static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);\n+static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mp_block);\n static size_t total_allocd;\n-static struct mem_pool *mem_pool;\n+static struct mp_block *mp_block_head;\n \n /* Atom management */\n static unsigned int atom_table_sz = 4451;\n@@ -636,14 +636,14 @@ static unsigned int hc_str(const char *s, size_t len)\n \n static void *pool_alloc(size_t len)\n {\n-\tstruct mem_pool *p;\n+\tstruct mp_block *p;\n \tvoid *r;\n \n \t/* round up to a 'uintmax_t' alignment */\n \tif (len & (sizeof(uintmax_t) - 1))\n \t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n \n-\tfor (p = mem_pool; p; p = p->next_pool)\n+\tfor (p = mp_block_head; p; p = p->next_block)\n \t\tif ((p->end - p->next_free >= len))\n \t\t\tbreak;\n \n@@ -652,12 +652,12 @@ static void *pool_alloc(size_t len)\n \t\t\ttotal_allocd += len;\n \t\t\treturn xmalloc(len);\n \t\t}\n-\t\ttotal_allocd += sizeof(struct mem_pool) + mem_pool_alloc;\n-\t\tp = xmalloc(st_add(sizeof(struct mem_pool), mem_pool_alloc));\n-\t\tp->next_pool = mem_pool;\n+\t\ttotal_allocd += sizeof(struct mp_block) + mem_pool_alloc;\n+\t\tp = xmalloc(st_add(sizeof(struct mp_block), mem_pool_alloc));\n+\t\tp->next_block = mp_block_head;\n \t\tp->next_free = (char *) p->space;\n \t\tp->end = p->next_free + mem_pool_alloc;\n-\t\tmem_pool = p;\n+\t\tmp_block_head = p;\n \t}\n \n \tr = p->next_free;\n-- \n2.14.3\n\n"},{"id":"343027","messageId":"20180326170351.197793-4-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v3 3/3] Move reusable parts of memory pool into its own file","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-03-26T17:03:51Z","receivedAt":"2018-03-26T17:04:46Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"This moves the reusable parts of the memory pool logic used by\nfast-import.c into its own file for use by other components.\n\nSigned-off-by: Jameson Miller \n---\n Makefile | 1 +\n fast-import.c | 70 +----------------------------------------------------------\n mem-pool.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++\n mem-pool.h | 34 +++++++++++++++++++++++++++++\n 4 files changed, 91 insertions(+), 69 deletions(-)\n create mode 100644 mem-pool.c\n create mode 100644 mem-pool.h\n\ndiff --git a/Makefile b/Makefile\nindex a1d8775adb..1e142b1dd9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -832,6 +832,7 @@ LIB_OBJS += lockfile.o\n LIB_OBJS += log-tree.o\n LIB_OBJS += mailinfo.o\n LIB_OBJS += mailmap.o\n+LIB_OBJS += mem-pool.o\n LIB_OBJS += match-trees.o\n LIB_OBJS += merge.o\n LIB_OBJS += merge-blobs.o\ndiff --git a/fast-import.c b/fast-import.c\nindex e85512191c..12c0058c06 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -168,6 +168,7 @@ Format of STDIN stream:\n #include \"dir.h\"\n #include \"run-command.h\"\n #include \"packfile.h\"\n+#include \"mem-pool.h\"\n \n #define PACK_ID_BITS 16\n #define MAX_PACK_ID ((1<pool_alloc += sizeof(struct mp_block) + block_alloc;\n-\tp = xmalloc(st_add(sizeof(struct mp_block), block_alloc));\n-\tp->next_block = mem_pool->mp_block;\n-\tp->next_free = (char *)p->space;\n-\tp->end = p->next_free + block_alloc;\n-\tmem_pool->mp_block = p;\n-\n-\treturn p;\n-}\n-\n-static void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)\n-{\n-\tstruct mp_block *p;\n-\tvoid *r;\n-\n-\t/* round up to a 'uintmax_t' alignment */\n-\tif (len & (sizeof(uintmax_t) - 1))\n-\t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n-\n-\tfor (p = mem_pool->mp_block; p; p = p->next_block)\n-\t\tif (p->end - p->next_free >= len)\n-\t \t\tbreak;\n-\n-\tif (!p) {\n-\t\tif (len >= (mem_pool->block_alloc / 2)) {\n-\t\t\tmem_pool->pool_alloc += len;\n-\t\t\treturn xmalloc(len);\n-\t\t}\n-\n-\t\tp = mem_pool_alloc_block(mem_pool, mem_pool->block_alloc);\n-\t}\n-\n-\tr = p->next_free;\n-\tp->next_free += len;\n-\treturn r;\n-}\n-\n-static void *mem_pool_calloc(struct mem_pool *mem_pool, size_t count, size_t size)\n-{\n-\tsize_t len = st_mult(count, size);\n-\tvoid *r = mem_pool_alloc(mem_pool, len);\n-\tmemset(r, 0, len);\n-\treturn r;\n-}\n-\n static char *pool_strdup(const char *s)\n {\n \tsize_t len = strlen(s) + 1;\ndiff --git a/mem-pool.c b/mem-pool.c\nnew file mode 100644\nindex 0000000000..389d7af447\n--- /dev/null\n+++ b/mem-pool.c\n@@ -0,0 +1,55 @@\n+/*\n+ * Memory Pool implementation logic.\n+ */\n+\n+#include \"cache.h\"\n+#include \"mem-pool.h\"\n+\n+static struct mp_block *mem_pool_alloc_block(struct mem_pool *mem_pool, size_t block_alloc)\n+{\n+\tstruct mp_block *p;\n+\n+\tmem_pool->pool_alloc += sizeof(struct mp_block) + block_alloc;\n+\tp = xmalloc(st_add(sizeof(struct mp_block), block_alloc));\n+\tp->next_block = mem_pool->mp_block;\n+\tp->next_free = (char *)p->space;\n+\tp->end = p->next_free + block_alloc;\n+\tmem_pool->mp_block = p;\n+\n+\treturn p;\n+}\n+\n+void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)\n+{\n+\tstruct mp_block *p;\n+\tvoid *r;\n+\n+\t/* round up to a 'uintmax_t' alignment */\n+\tif (len & (sizeof(uintmax_t) - 1))\n+\t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n+\n+\tfor (p = mem_pool->mp_block; p; p = p->next_block)\n+\t\tif (p->end - p->next_free >= len)\n+\t\t\tbreak;\n+\n+\tif (!p) {\n+\t\tif (len >= (mem_pool->block_alloc / 2)) {\n+\t\t\tmem_pool->pool_alloc += len;\n+\t\t\treturn xmalloc(len);\n+\t\t}\n+\n+\t\tp = mem_pool_alloc_block(mem_pool, mem_pool->block_alloc);\n+\t}\n+\n+\tr = p->next_free;\n+\tp->next_free += len;\n+\treturn r;\n+}\n+\n+void *mem_pool_calloc(struct mem_pool *mem_pool, size_t count, size_t size)\n+{\n+\tsize_t len = st_mult(count, size);\n+\tvoid *r = mem_pool_alloc(mem_pool, len);\n+\tmemset(r, 0, len);\n+\treturn r;\n+}\ndiff --git a/mem-pool.h b/mem-pool.h\nnew file mode 100644\nindex 0000000000..829ad58ecf\n--- /dev/null\n+++ b/mem-pool.h\n@@ -0,0 +1,34 @@\n+#ifndef MEM_POOL_H\n+#define MEM_POOL_H\n+\n+struct mp_block {\n+\tstruct mp_block *next_block;\n+\tchar *next_free;\n+\tchar *end;\n+\tuintmax_t space[FLEX_ARRAY]; /* more */\n+};\n+\n+struct mem_pool {\n+\tstruct mp_block *mp_block;\n+\n+\t/*\n+\t * The amount of available memory to grow the pool by.\n+\t * This size does not include the overhead for the mp_block.\n+\t */\n+\tsize_t block_alloc;\n+\n+\t/* The total amount of memory allocated by the pool. */\n+\tsize_t pool_alloc;\n+};\n+\n+/*\n+ * Alloc memory from the mem_pool.\n+ */\n+void *mem_pool_alloc(struct mem_pool *pool, size_t len);\n+\n+/*\n+ * Allocate and zero memory from the memory pool.\n+ */\n+void *mem_pool_calloc(struct mem_pool *pool, size_t count, size_t size);\n+\n+#endif\n-- \n2.14.3\n\n"},{"id":"343028","messageId":"20180326170351.197793-1-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v3 0/3] Extract memory pool logic into reusable component","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-03-26T17:03:48Z","receivedAt":"2018-03-26T17:04:54Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"Changes from v2:\n\n - Code Review Reactions\n \n - Lifetime management functions for mem_pool will be included in\n future patch series\n\nThis patch series extracts the memory pool implementation, currently\nused by fast-import, into a generalized component. This memory pool\ncan then be generally used by any component that needs a pool of\nmemory.\n\nThis patch is in preparation for a change to speed up the loading of\nindexes with a large number of cache entries by reducing the number of\nmalloc() calls. For a rough example of how this component will be used\nin this capacity, please see [1].\n\n[1] https://github.com/jamill/git/compare/block_allocation_base...jamill:block_allocation\n\nJameson Miller (3):\n fast-import: rename mem_pool type to mp_block\n fast-import: introduce mem_pool type\n Move reusable parts of memory pool into its own file\n\n Makefile | 1 +\n fast-import.c | 74 +++++++++++------------------------------------------------\n mem-pool.c | 55 ++++++++++++++++++++++++++++++++++++++++++++\n mem-pool.h | 34 +++++++++++++++++++++++++++\n 4 files changed, 104 insertions(+), 60 deletions(-)\n create mode 100644 mem-pool.c\n create mode 100644 mem-pool.h\n\n-- \n2.14.3\n\n"},{"id":"343033","messageId":"CAPig+cS83ECwXp3PuV2ex2nDOV0tZHmEqAdtUNjV_LmtEaxBhw@mail.gmail.com","threadId":"48107","inReplyTo":"20180326170351.197793-3-jamill@microsoft.com","subject":"Re: [PATCH v3 2/3] fast-import: introduce mem_pool type","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-03-26T17:34:21Z","receivedAt":"2018-03-26T17:34:27Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 26, 2018 at 1:03 PM, Jameson Miller wrote:\n> Introduce the mem_pool type which encapsulates all the information\n> necessary to manage a pool of memory.This change moves the existing\n\ns/memory\\.This/memory. This/\n\n> variables in fast-import used to support the global memory pool to use\n> this structure.\n>\n> These changes allow for the multiple instances of a memory pool to\n> exist and be reused outside of fast-import. In a future commit the\n> mem_pool type will be moved to its own file.\n>\n> Signed-off-by: Jameson Miller \n"},{"id":"343142","messageId":"xmqqpo3pa4ag.fsf@gitster-ct.c.googlers.com","threadId":"48107","inReplyTo":"20180326170351.197793-3-jamill@microsoft.com","subject":"Re: [PATCH v3 2/3] fast-import: introduce mem_pool type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-27T16:09:43Z","receivedAt":"2018-03-27T16:10:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jameson Miller writes:\n\n> Introduce the mem_pool type which encapsulates all the information\n> necessary to manage a pool of memory.This change moves the existing\n> variables in fast-import used to support the global memory pool to use\n> this structure.\n>\n> These changes allow for the multiple instances of a memory pool to\n> exist and be reused outside of fast-import. In a future commit the\n> mem_pool type will be moved to its own file.\n>\n> Signed-off-by: Jameson Miller \n> ---\n> fast-import.c | 80 +++++++++++++++++++++++++++++++++++++----------------------\n> 1 file changed, 51 insertions(+), 29 deletions(-)\n\nThanks, this got much easier to read and reason about.\n\n> @@ -304,9 +317,7 @@ static int global_argc;\n> static const char **global_argv;\n> \n> /* Memory pools */\n> -static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mp_block);\n> -static size_t total_allocd;\n> -static struct mp_block *mp_block_head;\n> +static struct mem_pool fi_mem_pool = {0, 2*1024*1024 - sizeof(struct mp_block), 0 };\n> \n> /* Atom management */\n> static unsigned int atom_table_sz = 4451;\n> @@ -324,6 +335,7 @@ static off_t pack_size;\n> /* Table of objects we've written. */\n> static unsigned int object_entry_alloc = 5000;\n> static struct object_entry_pool *blocks;\n> +static size_t total_allocd;\n\nSo the design decision made at this step is that pool_alloc field\nkeeps track of the per-pool allocation, while total_allocd is a sum\nacross instances of pools. That sounds appropriate for stats.\n\n> @@ -634,7 +646,21 @@ static unsigned int hc_str(const char *s, size_t len)\n> \treturn r;\n> }\n> \n> -static void *pool_alloc(size_t len)\n> +static struct mp_block *mem_pool_alloc_block(struct mem_pool *mem_pool, size_t block_alloc)\n> +{\n> +\tstruct mp_block *p;\n> +\n> +\tmem_pool->pool_alloc += sizeof(struct mp_block) + block_alloc;\n> +\tp = xmalloc(st_add(sizeof(struct mp_block), block_alloc));\n> +\tp->next_block = mem_pool->mp_block;\n> +\tp->next_free = (char *)p->space;\n> +\tp->end = p->next_free + block_alloc;\n> +\tmem_pool->mp_block = p;\n\nThis, compared to what used to happen in mem_pool_alloc()'s original\ncode, ignores total_allocd. I cannot tell if that is an intentional\nchange or a mistake.\n\n> +\n> +\treturn p;\n> +}\n> +\n> +static void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)\n> {\n> \tstruct mp_block *p;\n> \tvoid *r;\n> @@ -643,21 +669,17 @@ static void *pool_alloc(size_t len)\n> \tif (len & (sizeof(uintmax_t) - 1))\n> \t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n> \n> -\tfor (p = mp_block_head; p; p = p->next_block)\n> -\t\tif ((p->end - p->next_free >= len))\n> -\t\t\tbreak;\n> +\tfor (p = mem_pool->mp_block; p; p = p->next_block)\n> +\t\tif (p->end - p->next_free >= len)\n> +\t \t\tbreak;\n> \n> \tif (!p) {\n> -\t\tif (len >= (mem_pool_alloc/2)) {\n> -\t\t\ttotal_allocd += len;\n> +\t\tif (len >= (mem_pool->block_alloc / 2)) {\n> +\t\t\tmem_pool->pool_alloc += len;\n> \t\t\treturn xmalloc(len);\n\nIt is unfair to account this piece of memory to the mem_pool, as we\nended up not carving it out from here. Did you mean to increment\ntotal_allocd by len instead, perhaps?\n\nAnd I do agree with the idea in the previous round to make these\noversized pieces of memory allocated here to be freeable via the\nmem_pool instance (I only found it questionable to use the main\n\"here are the list of blocks that we could carve small pieces out\"\nlist), and anticipate that a later step in the series would change\nthis part to do so. With that anticipation, I very much appreciate\nthat this step did not mix that and stayed as close to the original\n(except for the possible mis-accounting). It makes it very clear\nwhat is going on in each separate step in the series.\n\n> \t\t}\n> -\t\ttotal_allocd += sizeof(struct mp_block) + mem_pool_alloc;\n\nThis is what I noticed got lost in the pool-alloc-block helper above.\n\n> -\t\tp = xmalloc(st_add(sizeof(struct mp_block), mem_pool_alloc));\n> -\t\tp->next_block = mp_block_head;\n> -\t\tp->next_free = (char *) p->space;\n> -\t\tp->end = p->next_free + mem_pool_alloc;\n> -\t\tmp_block_head = p;\n> +\n> +\t\tp = mem_pool_alloc_block(mem_pool, mem_pool->block_alloc);\n> \t}\n> \n> \tr = p->next_free;\n> @@ -665,10 +687,10 @@ static void *pool_alloc(size_t len)\n> \treturn r;\n> }\n> \n> -static void *pool_calloc(size_t count, size_t size)\n> +static void *mem_pool_calloc(struct mem_pool *mem_pool, size_t count, size_t size)\n> {\n> -\tsize_t len = count * size;\n> -\tvoid *r = pool_alloc(len);\n> +\tsize_t len = st_mult(count, size);\n\nNice ;-)\n\n> +\tvoid *r = mem_pool_alloc(mem_pool, len);\n> \tmemset(r, 0, len);\n> \treturn r;\n> }\n"},{"id":"343147","messageId":"xmqqlgeda2pd.fsf@gitster-ct.c.googlers.com","threadId":"48107","inReplyTo":"20180326170351.197793-4-jamill@microsoft.com","subject":"Re: [PATCH v3 3/3] Move reusable parts of memory pool into its own file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-27T16:43:58Z","receivedAt":"2018-03-27T16:44:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jameson Miller writes:\n\n> This moves the reusable parts of the memory pool logic used by\n> fast-import.c into its own file for use by other components.\n>\n> Signed-off-by: Jameson Miller \n> ---\n> Makefile | 1 +\n> fast-import.c | 70 +----------------------------------------------------------\n> mem-pool.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++\n> mem-pool.h | 34 +++++++++++++++++++++++++++++\n> 4 files changed, 91 insertions(+), 69 deletions(-)\n> create mode 100644 mem-pool.c\n> create mode 100644 mem-pool.h\n\nOK. This is indeed straight-forward line movements and nothing else,\nother than obviously a few static helpers are now extern.\n\nI said I'd anticipate that the allocation that bypasses the pool\nsubsystem would want to become traceable by the pool subsystem,\nwhich would allow us to free() the pieces of memory allocated\ndirectly with xmalloc() in mem_pool_alloc() instead of leaking. I\nam OK if the structure of this series is to make that change after\nthese three steps we see here. When that happens, it will start to\nmake sense to bill the \"this is too big so do not attempt to carve\nit out from block, and do not overallocate and make the remainder\navailable for later requests\" to the pool instance mem_pool_alloc()\nis working on, as that piece of memory is also known to a specific\npool instance.\n\nAfter I wrote review for 2/3, I found out that you changed the\nmeaning of total_allocd (which should probably be described in its\nlog message). Unlike the original that counted \"total\", it now is\nused only for memory that is allocated directly by fast-import.c and\ndoes not account for memory obtained by calling mem-pool.\n\nThe output routine is changed in 2/3 to add fi_mem_pool's pool_alloc\nto it, so billing oversized allocation that does *not* belong to any\nspecific pool to _some_ pool and ignoring total_allocd would cancel\nthings out. It still feels a bit fishy, but I think it is OK.\n\nSo all in all, I think we are in no worse shape than the original\n(we call it \"bug-to-bug compatible\" ;-)), and successfully extracted\na reusable piece in a separate file in a clean way so that we can\nrefine and extend it further. Nicely done.\n\nWill queue; the proposed log for step 2/3 may want to be a bit\npolished, though.\n\nThanks.\n\n"},{"id":"343324","messageId":"BL0PR2101MB11061477A78E978A65C0E4D4CEA20@BL0PR2101MB1106.namprd21.prod.outlook.com","threadId":"48107","inReplyTo":"xmqqlgeda2pd.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 3/3] Move reusable parts of memory pool into its own file","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-03-29T14:12:10Z","receivedAt":"2018-03-29T14:12:17Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"Thank you for the thorough and detailed review - I very much appreciate it.\n\n> I\n> am OK if the structure of this series is to make that change after\n> these three steps we see here. \n\nI will add (back) the accounting of large memory blocks in a future patch\nseries, taking into account your existing feedback. As this patch series did not have\nany consumers that \"discarded\" a memory pool, I decided to wait until I had \na consumer that used this functionality.\n\n> Will queue; the proposed log for step 2/3 may want to be a bit\n> polished, though.\n\nI am away the rest of this week and next week, but will update this patch\nseries when I get back (Week of April 9th).\n\nThank you\nJameson\n\n________________________________________\nFrom: Junio C Hamano on behalf of Junio C Hamano \nSent: Tuesday, March 27, 2018 12:43 PM\nTo: Jameson Miller\nCc: git@vger.kernel.org; peff@peff.net\nSubject: Re: [PATCH v3 3/3] Move reusable parts of memory pool into its own file \n \nJameson Miller writes:\n\n> This moves the reusable parts of the memory pool logic used by\n> fast-import.c into its own file for use by other components.\n>\n> Signed-off-by: Jameson Miller \n> ---\n>  Makefile      |  1 +\n>  fast-import.c | 70 +----------------------------------------------------------\n>  mem-pool.c    | 55 ++++++++++++++++++++++++++++++++++++++++++++++\n>  mem-pool.h    | 34 +++++++++++++++++++++++++++++\n>  4 files changed, 91 insertions(+), 69 deletions(-)\n>  create mode 100644 mem-pool.c\n>  create mode 100644 mem-pool.h\n\nOK.  This is indeed straight-forward line movements and nothing else,\nother than obviously a few static helpers are now extern.\n\nI said I'd anticipate that the allocation that bypasses the pool\nsubsystem would want to become traceable by the pool subsystem,\nwhich would allow us to free() the pieces of memory allocated\ndirectly with xmalloc() in mem_pool_alloc() instead of leaking.  I\nam OK if the structure of this series is to make that change after\nthese three steps we see here.  When that happens, it will start to\nmake sense to bill the \"this is too big so do not attempt to carve\nit out from block, and do not overallocate and make the remainder\navailable for later requests\" to the pool instance mem_pool_alloc()\nis working on, as that piece of memory is also known to a specific\npool instance.\n\nAfter I wrote review for 2/3, I found out that you changed the\nmeaning of total_allocd (which should probably be described in its\nlog message).  Unlike the original that counted \"total\", it now is\nused only for memory that is allocated directly by fast-import.c and\ndoes not account for memory obtained by calling mem-pool.\n\nThe output routine is changed in 2/3 to add fi_mem_pool's pool_alloc\nto it, so billing oversized allocation that does *not* belong to any\nspecific pool to _some_ pool and ignoring total_allocd would cancel\nthings out.  It still feels a bit fishy, but I think it is OK.\n\nSo all in all, I think we are in no worse shape than the original\n(we call it \"bug-to-bug compatible\" ;-)), and successfully extracted\na reusable piece in a separate file in a clean way so that we can\nrefine and extend it further.  Nicely done.\n\nWill queue; the proposed log for step 2/3 may want to be a bit\npolished, though.\n\nThanks.\n"},{"id":"344469","messageId":"20180411183738.113809-1-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v4 0/3] Extract memory pool logic into reusable component","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-04-11T18:37:53Z","receivedAt":"2018-04-11T18:38:03Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"Thank you everyone for taking the time review and provide feedback on\nthis patch series.\n\nChanges from v3:\n\n - Based patch off of new commit, to resolve merge conflict.\n\n - Updated log message in 2/3 based on feedback.\n\n - Squashed patch from Ramsay Jones into 2/3 to fix warning from\n sparse.\n\n - Updated variable names in 2/3 to reflect updated usage of\n variable.\n\nJameson Miller (3):\n fast-import: rename mem_pool type to mp_block\n fast-import: introduce mem_pool type\n Move reusable parts of memory pool into its own file\n\n Makefile | 1 +\n fast-import.c | 77 +++++++++++++----------------------------------------------\n mem-pool.c | 55 ++++++++++++++++++++++++++++++++++++++++++\n mem-pool.h | 34 ++++++++++++++++++++++++++\n 4 files changed, 106 insertions(+), 61 deletions(-)\n create mode 100644 mem-pool.c\n create mode 100644 mem-pool.h\n\n-- \n2.14.3\n\n"},{"id":"344470","messageId":"20180411183738.113809-4-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v4 3/3] Move reusable parts of memory pool into its own file","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-04-11T18:37:55Z","receivedAt":"2018-04-11T18:38:04Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"This moves the reusable parts of the memory pool logic used by\nfast-import.c into its own file for use by other components.\n\nSigned-off-by: Jameson Miller \n---\n Makefile | 1 +\n fast-import.c | 70 +----------------------------------------------------------\n mem-pool.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++\n mem-pool.h | 34 +++++++++++++++++++++++++++++\n 4 files changed, 91 insertions(+), 69 deletions(-)\n create mode 100644 mem-pool.c\n create mode 100644 mem-pool.h\n\ndiff --git a/Makefile b/Makefile\nindex f181687250..5aa4f8064f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -844,6 +844,7 @@ LIB_OBJS += log-tree.o\n LIB_OBJS += mailinfo.o\n LIB_OBJS += mailmap.o\n LIB_OBJS += match-trees.o\n+LIB_OBJS += mem-pool.o\n LIB_OBJS += merge.o\n LIB_OBJS += merge-blobs.o\n LIB_OBJS += merge-recursive.o\ndiff --git a/fast-import.c b/fast-import.c\nindex 48d4797ab5..05d1079d23 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -170,6 +170,7 @@ Format of STDIN stream:\n #include \"run-command.h\"\n #include \"packfile.h\"\n #include \"object-store.h\"\n+#include \"mem-pool.h\"\n \n #define PACK_ID_BITS 16\n #define MAX_PACK_ID ((1<pool_alloc += sizeof(struct mp_block) + block_alloc;\n-\tp = xmalloc(st_add(sizeof(struct mp_block), block_alloc));\n-\tp->next_block = mem_pool->mp_block;\n-\tp->next_free = (char *)p->space;\n-\tp->end = p->next_free + block_alloc;\n-\tmem_pool->mp_block = p;\n-\n-\treturn p;\n-}\n-\n-static void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)\n-{\n-\tstruct mp_block *p;\n-\tvoid *r;\n-\n-\t/* round up to a 'uintmax_t' alignment */\n-\tif (len & (sizeof(uintmax_t) - 1))\n-\t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n-\n-\tfor (p = mem_pool->mp_block; p; p = p->next_block)\n-\t\tif (p->end - p->next_free >= len)\n-\t\t\tbreak;\n-\n-\tif (!p) {\n-\t\tif (len >= (mem_pool->block_alloc / 2)) {\n-\t\t\tmem_pool->pool_alloc += len;\n-\t\t\treturn xmalloc(len);\n-\t\t}\n-\n-\t\tp = mem_pool_alloc_block(mem_pool, mem_pool->block_alloc);\n-\t}\n-\n-\tr = p->next_free;\n-\tp->next_free += len;\n-\treturn r;\n-}\n-\n-static void *mem_pool_calloc(struct mem_pool *mem_pool, size_t count, size_t size)\n-{\n-\tsize_t len = st_mult(count, size);\n-\tvoid *r = mem_pool_alloc(mem_pool, len);\n-\tmemset(r, 0, len);\n-\treturn r;\n-}\n-\n static char *pool_strdup(const char *s)\n {\n \tsize_t len = strlen(s) + 1;\ndiff --git a/mem-pool.c b/mem-pool.c\nnew file mode 100644\nindex 0000000000..389d7af447\n--- /dev/null\n+++ b/mem-pool.c\n@@ -0,0 +1,55 @@\n+/*\n+ * Memory Pool implementation logic.\n+ */\n+\n+#include \"cache.h\"\n+#include \"mem-pool.h\"\n+\n+static struct mp_block *mem_pool_alloc_block(struct mem_pool *mem_pool, size_t block_alloc)\n+{\n+\tstruct mp_block *p;\n+\n+\tmem_pool->pool_alloc += sizeof(struct mp_block) + block_alloc;\n+\tp = xmalloc(st_add(sizeof(struct mp_block), block_alloc));\n+\tp->next_block = mem_pool->mp_block;\n+\tp->next_free = (char *)p->space;\n+\tp->end = p->next_free + block_alloc;\n+\tmem_pool->mp_block = p;\n+\n+\treturn p;\n+}\n+\n+void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)\n+{\n+\tstruct mp_block *p;\n+\tvoid *r;\n+\n+\t/* round up to a 'uintmax_t' alignment */\n+\tif (len & (sizeof(uintmax_t) - 1))\n+\t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n+\n+\tfor (p = mem_pool->mp_block; p; p = p->next_block)\n+\t\tif (p->end - p->next_free >= len)\n+\t\t\tbreak;\n+\n+\tif (!p) {\n+\t\tif (len >= (mem_pool->block_alloc / 2)) {\n+\t\t\tmem_pool->pool_alloc += len;\n+\t\t\treturn xmalloc(len);\n+\t\t}\n+\n+\t\tp = mem_pool_alloc_block(mem_pool, mem_pool->block_alloc);\n+\t}\n+\n+\tr = p->next_free;\n+\tp->next_free += len;\n+\treturn r;\n+}\n+\n+void *mem_pool_calloc(struct mem_pool *mem_pool, size_t count, size_t size)\n+{\n+\tsize_t len = st_mult(count, size);\n+\tvoid *r = mem_pool_alloc(mem_pool, len);\n+\tmemset(r, 0, len);\n+\treturn r;\n+}\ndiff --git a/mem-pool.h b/mem-pool.h\nnew file mode 100644\nindex 0000000000..829ad58ecf\n--- /dev/null\n+++ b/mem-pool.h\n@@ -0,0 +1,34 @@\n+#ifndef MEM_POOL_H\n+#define MEM_POOL_H\n+\n+struct mp_block {\n+\tstruct mp_block *next_block;\n+\tchar *next_free;\n+\tchar *end;\n+\tuintmax_t space[FLEX_ARRAY]; /* more */\n+};\n+\n+struct mem_pool {\n+\tstruct mp_block *mp_block;\n+\n+\t/*\n+\t * The amount of available memory to grow the pool by.\n+\t * This size does not include the overhead for the mp_block.\n+\t */\n+\tsize_t block_alloc;\n+\n+\t/* The total amount of memory allocated by the pool. */\n+\tsize_t pool_alloc;\n+};\n+\n+/*\n+ * Alloc memory from the mem_pool.\n+ */\n+void *mem_pool_alloc(struct mem_pool *pool, size_t len);\n+\n+/*\n+ * Allocate and zero memory from the memory pool.\n+ */\n+void *mem_pool_calloc(struct mem_pool *pool, size_t count, size_t size);\n+\n+#endif\n-- \n2.14.3\n\n"},{"id":"344471","messageId":"20180411183738.113809-3-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v4 2/3] fast-import: introduce mem_pool type","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-04-11T18:37:54Z","receivedAt":"2018-04-11T18:38:08Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"Introduce the mem_pool type which encapsulates all the information necessary to\nmanage a pool of memory. This change moves the existing variables in\nfast-import used to support the global memory pool to use this structure. It\nalso renames variables that are no longer used by memory pools to reflect their\nmore scoped usage.\n\nThese changes allow for the multiple instances of a memory pool to\nexist and be reused outside of fast-import. In a future commit the\nmem_pool type will be moved to its own file.\n\nSigned-off-by: Jameson Miller \n---\n fast-import.c | 81 ++++++++++++++++++++++++++++++++++++++---------------------\n 1 file changed, 52 insertions(+), 29 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 38af0a294b..48d4797ab5 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -218,6 +218,19 @@ struct mp_block {\n \tuintmax_t space[FLEX_ARRAY]; /* more */\n };\n \n+struct mem_pool {\n+\tstruct mp_block *mp_block;\n+\n+\t/*\n+\t * The amount of available memory to grow the pool by.\n+\t * This size does not include the overhead for the mp_block.\n+\t */\n+\tsize_t block_alloc;\n+\n+\t/* The total amount of memory allocated by the pool. */\n+\tsize_t pool_alloc;\n+};\n+\n struct atom_str {\n \tstruct atom_str *next_atom;\n \tunsigned short str_len;\n@@ -306,9 +319,8 @@ static int global_argc;\n static const char **global_argv;\n \n /* Memory pools */\n-static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mp_block);\n-static size_t total_allocd;\n-static struct mp_block *mp_block_head;\n+static struct mem_pool fi_mem_pool = {NULL, 2*1024*1024 -\n+\t\t\t\t sizeof(struct mp_block), 0 };\n \n /* Atom management */\n static unsigned int atom_table_sz = 4451;\n@@ -343,6 +355,7 @@ static unsigned int tree_entry_alloc = 1000;\n static void *avail_tree_entry;\n static unsigned int avail_tree_table_sz = 100;\n static struct avail_tree_content **avail_tree_table;\n+static size_t tree_entry_allocd;\n static struct strbuf old_tree = STRBUF_INIT;\n static struct strbuf new_tree = STRBUF_INIT;\n \n@@ -636,7 +649,21 @@ static unsigned int hc_str(const char *s, size_t len)\n \treturn r;\n }\n \n-static void *pool_alloc(size_t len)\n+static struct mp_block *mem_pool_alloc_block(struct mem_pool *mem_pool, size_t block_alloc)\n+{\n+\tstruct mp_block *p;\n+\n+\tmem_pool->pool_alloc += sizeof(struct mp_block) + block_alloc;\n+\tp = xmalloc(st_add(sizeof(struct mp_block), block_alloc));\n+\tp->next_block = mem_pool->mp_block;\n+\tp->next_free = (char *)p->space;\n+\tp->end = p->next_free + block_alloc;\n+\tmem_pool->mp_block = p;\n+\n+\treturn p;\n+}\n+\n+static void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)\n {\n \tstruct mp_block *p;\n \tvoid *r;\n@@ -645,21 +672,17 @@ static void *pool_alloc(size_t len)\n \tif (len & (sizeof(uintmax_t) - 1))\n \t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n \n-\tfor (p = mp_block_head; p; p = p->next_block)\n-\t\tif ((p->end - p->next_free >= len))\n+\tfor (p = mem_pool->mp_block; p; p = p->next_block)\n+\t\tif (p->end - p->next_free >= len)\n \t\t\tbreak;\n \n \tif (!p) {\n-\t\tif (len >= (mem_pool_alloc/2)) {\n-\t\t\ttotal_allocd += len;\n+\t\tif (len >= (mem_pool->block_alloc / 2)) {\n+\t\t\tmem_pool->pool_alloc += len;\n \t\t\treturn xmalloc(len);\n \t\t}\n-\t\ttotal_allocd += sizeof(struct mp_block) + mem_pool_alloc;\n-\t\tp = xmalloc(st_add(sizeof(struct mp_block), mem_pool_alloc));\n-\t\tp->next_block = mp_block_head;\n-\t\tp->next_free = (char *) p->space;\n-\t\tp->end = p->next_free + mem_pool_alloc;\n-\t\tmp_block_head = p;\n+\n+\t\tp = mem_pool_alloc_block(mem_pool, mem_pool->block_alloc);\n \t}\n \n \tr = p->next_free;\n@@ -667,10 +690,10 @@ static void *pool_alloc(size_t len)\n \treturn r;\n }\n \n-static void *pool_calloc(size_t count, size_t size)\n+static void *mem_pool_calloc(struct mem_pool *mem_pool, size_t count, size_t size)\n {\n-\tsize_t len = count * size;\n-\tvoid *r = pool_alloc(len);\n+\tsize_t len = st_mult(count, size);\n+\tvoid *r = mem_pool_alloc(mem_pool, len);\n \tmemset(r, 0, len);\n \treturn r;\n }\n@@ -678,7 +701,7 @@ static void *pool_calloc(size_t count, size_t size)\n static char *pool_strdup(const char *s)\n {\n \tsize_t len = strlen(s) + 1;\n-\tchar *r = pool_alloc(len);\n+\tchar *r = mem_pool_alloc(&fi_mem_pool, len);\n \tmemcpy(r, s, len);\n \treturn r;\n }\n@@ -687,7 +710,7 @@ static void insert_mark(uintmax_t idnum, struct object_entry *oe)\n {\n \tstruct mark_set *s = marks;\n \twhile ((idnum >> s->shift) >= 1024) {\n-\t\ts = pool_calloc(1, sizeof(struct mark_set));\n+\t\ts = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n \t\ts->shift = marks->shift + 10;\n \t\ts->data.sets[0] = marks;\n \t\tmarks = s;\n@@ -696,7 +719,7 @@ static void insert_mark(uintmax_t idnum, struct object_entry *oe)\n \t\tuintmax_t i = idnum >> s->shift;\n \t\tidnum -= i << s->shift;\n \t\tif (!s->data.sets[i]) {\n-\t\t\ts->data.sets[i] = pool_calloc(1, sizeof(struct mark_set));\n+\t\t\ts->data.sets[i] = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n \t\t\ts->data.sets[i]->shift = s->shift - 10;\n \t\t}\n \t\ts = s->data.sets[i];\n@@ -734,7 +757,7 @@ static struct atom_str *to_atom(const char *s, unsigned short len)\n \t\tif (c->str_len == len && !strncmp(s, c->str_dat, len))\n \t\t\treturn c;\n \n-\tc = pool_alloc(sizeof(struct atom_str) + len + 1);\n+\tc = mem_pool_alloc(&fi_mem_pool, sizeof(struct atom_str) + len + 1);\n \tc->str_len = len;\n \tmemcpy(c->str_dat, s, len);\n \tc->str_dat[len] = 0;\n@@ -765,7 +788,7 @@ static struct branch *new_branch(const char *name)\n \tif (check_refname_format(name, REFNAME_ALLOW_ONELEVEL))\n \t\tdie(\"Branch name doesn't conform to GIT standards: %s\", name);\n \n-\tb = pool_calloc(1, sizeof(struct branch));\n+\tb = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct branch));\n \tb->name = pool_strdup(name);\n \tb->table_next_branch = branch_table[hc];\n \tb->branch_tree.versions[0].mode = S_IFDIR;\n@@ -801,7 +824,7 @@ static struct tree_content *new_tree_content(unsigned int cnt)\n \t\t\tavail_tree_table[hc] = f->next_avail;\n \t} else {\n \t\tcnt = cnt & 7 ? ((cnt / 8) + 1) * 8 : cnt;\n-\t\tf = pool_alloc(sizeof(*t) + sizeof(t->entries[0]) * cnt);\n+\t\tf = mem_pool_alloc(&fi_mem_pool, sizeof(*t) + sizeof(t->entries[0]) * cnt);\n \t\tf->entry_capacity = cnt;\n \t}\n \n@@ -846,7 +869,7 @@ static struct tree_entry *new_tree_entry(void)\n \n \tif (!avail_tree_entry) {\n \t\tunsigned int n = tree_entry_alloc;\n-\t\ttotal_allocd += n * sizeof(struct tree_entry);\n+\t\ttree_entry_allocd += n * sizeof(struct tree_entry);\n \t\tALLOC_ARRAY(e, n);\n \t\tavail_tree_entry = e;\n \t\twhile (n-- > 1) {\n@@ -2867,7 +2890,7 @@ static void parse_new_tag(const char *arg)\n \tenum object_type type;\n \tconst char *v;\n \n-\tt = pool_alloc(sizeof(struct tag));\n+\tt = mem_pool_alloc(&fi_mem_pool, sizeof(struct tag));\n \tmemset(t, 0, sizeof(struct tag));\n \tt->name = pool_strdup(arg);\n \tif (last_tag)\n@@ -3466,12 +3489,12 @@ int cmd_main(int argc, const char **argv)\n \tatom_table = xcalloc(atom_table_sz, sizeof(struct atom_str*));\n \tbranch_table = xcalloc(branch_table_sz, sizeof(struct branch*));\n \tavail_tree_table = xcalloc(avail_tree_table_sz, sizeof(struct avail_tree_content*));\n-\tmarks = pool_calloc(1, sizeof(struct mark_set));\n+\tmarks = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set));\n \n \tglobal_argc = argc;\n \tglobal_argv = argv;\n \n-\trc_free = pool_alloc(cmd_save * sizeof(*rc_free));\n+\trc_free = mem_pool_alloc(&fi_mem_pool, cmd_save * sizeof(*rc_free));\n \tfor (i = 0; i < (cmd_save - 1); i++)\n \t\trc_free[i].next = &rc_free[i + 1];\n \trc_free[cmd_save - 1].next = NULL;\n@@ -3545,8 +3568,8 @@ int cmd_main(int argc, const char **argv)\n \t\tfprintf(stderr, \"Total branches: %10lu (%10lu loads )\\n\", branch_count, branch_load_count);\n \t\tfprintf(stderr, \" marks: %10\" PRIuMAX \" (%10\" PRIuMAX \" unique )\\n\", (((uintmax_t)1) << marks->shift) * 1024, marks_set_count);\n \t\tfprintf(stderr, \" atoms: %10u\\n\", atom_cnt);\n-\t\tfprintf(stderr, \"Memory total: %10\" PRIuMAX \" KiB\\n\", (total_allocd + alloc_count*sizeof(struct object_entry))/1024);\n-\t\tfprintf(stderr, \" pools: %10lu KiB\\n\", (unsigned long)(total_allocd/1024));\n+\t\tfprintf(stderr, \"Memory total: %10\" PRIuMAX \" KiB\\n\", (tree_entry_allocd + fi_mem_pool.pool_alloc + alloc_count*sizeof(struct object_entry))/1024);\n+\t\tfprintf(stderr, \" pools: %10lu KiB\\n\", (unsigned long)((tree_entry_allocd + fi_mem_pool.pool_alloc) /1024));\n \t\tfprintf(stderr, \" objects: %10\" PRIuMAX \" KiB\\n\", (alloc_count*sizeof(struct object_entry))/1024);\n \t\tfprintf(stderr, \"---------------------------------------------------------------------\\n\");\n \t\tpack_report();\n-- \n2.14.3\n\n"},{"id":"344501","messageId":"20180411183738.113809-2-jamill@microsoft.com","threadId":"48107","inReplyTo":"20180321164152.204869-1-jamill@microsoft.com","subject":"[PATCH v4 1/3] fast-import: rename mem_pool type to mp_block","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-04-11T18:37:53Z","receivedAt":"2018-04-11T21:27:10Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"This is part of a patch series to extract the memory pool logic in\nfast-import into a more generalized version. The existing mem_pool type\nmaps more closely to a \"block of memory\" (mp_block) in the more\ngeneralized memory pool. This commit renames the mem_pool to mp_block to\nreduce churn in future patches.\n\nSigned-off-by: Jameson Miller \n---\n fast-import.c | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 99f8f56e8c..38af0a294b 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -211,8 +211,8 @@ struct last_object {\n \tunsigned no_swap : 1;\n };\n \n-struct mem_pool {\n-\tstruct mem_pool *next_pool;\n+struct mp_block {\n+\tstruct mp_block *next_block;\n \tchar *next_free;\n \tchar *end;\n \tuintmax_t space[FLEX_ARRAY]; /* more */\n@@ -306,9 +306,9 @@ static int global_argc;\n static const char **global_argv;\n \n /* Memory pools */\n-static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);\n+static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mp_block);\n static size_t total_allocd;\n-static struct mem_pool *mem_pool;\n+static struct mp_block *mp_block_head;\n \n /* Atom management */\n static unsigned int atom_table_sz = 4451;\n@@ -638,14 +638,14 @@ static unsigned int hc_str(const char *s, size_t len)\n \n static void *pool_alloc(size_t len)\n {\n-\tstruct mem_pool *p;\n+\tstruct mp_block *p;\n \tvoid *r;\n \n \t/* round up to a 'uintmax_t' alignment */\n \tif (len & (sizeof(uintmax_t) - 1))\n \t\tlen += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));\n \n-\tfor (p = mem_pool; p; p = p->next_pool)\n+\tfor (p = mp_block_head; p; p = p->next_block)\n \t\tif ((p->end - p->next_free >= len))\n \t\t\tbreak;\n \n@@ -654,12 +654,12 @@ static void *pool_alloc(size_t len)\n \t\t\ttotal_allocd += len;\n \t\t\treturn xmalloc(len);\n \t\t}\n-\t\ttotal_allocd += sizeof(struct mem_pool) + mem_pool_alloc;\n-\t\tp = xmalloc(st_add(sizeof(struct mem_pool), mem_pool_alloc));\n-\t\tp->next_pool = mem_pool;\n+\t\ttotal_allocd += sizeof(struct mp_block) + mem_pool_alloc;\n+\t\tp = xmalloc(st_add(sizeof(struct mp_block), mem_pool_alloc));\n+\t\tp->next_block = mp_block_head;\n \t\tp->next_free = (char *) p->space;\n \t\tp->end = p->next_free + mem_pool_alloc;\n-\t\tmem_pool = p;\n+\t\tmp_block_head = p;\n \t}\n \n \tr = p->next_free;\n-- \n2.14.3\n\n"},{"id":"344881","messageId":"BL0PR2101MB1106528350D2169FE22F3E4FCEB70@BL0PR2101MB1106.namprd21.prod.outlook.com","threadId":"48107","inReplyTo":"20180411183738.113809-1-jamill@microsoft.com","subject":"RE: [PATCH v4 0/3] Extract memory pool logic into reusable component","fromName":"Jameson Miller","fromEmail":"jamill@microsoft.com","sentAt":"2018-04-17T16:43:00Z","receivedAt":"2018-04-17T16:43:09Z","isPatch":true,"sender":{"key":"jamill@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/2045976?v=4"},"body":"I think this version (V4) should reflect the latest round of feedback. Please let me know if there are any other questions or outstanding work to finish here.\n\nI have submitted a patch series to have a second component use this memory pool [1].\n\nThank you\n\n[1] https://public-inbox.org/git/20180417163400.3875-2-jamill@microsoft.com/\n\n-----Original Message-----\nFrom: Jameson Miller \nSent: Wednesday, April 11, 2018 2:38 PM\nTo: git@vger.kernel.org\nCc: gitster@pobox.com; peff@peff.net; sunshine@sunshineco.com; ramsay@ramsayjones.plus.com; Jameson Miller \nSubject: [PATCH v4 0/3] Extract memory pool logic into reusable component\n\nThank you everyone for taking the time review and provide feedback on this patch series.\n\nChanges from v3:\n\n - Based patch off of new commit, to resolve merge conflict.\n\n - Updated log message in 2/3 based on feedback.\n\n - Squashed patch from Ramsay Jones into 2/3 to fix warning from\n sparse.\n\n - Updated variable names in 2/3 to reflect updated usage of\n variable.\n\nJameson Miller (3):\n fast-import: rename mem_pool type to mp_block\n fast-import: introduce mem_pool type\n Move reusable parts of memory pool into its own file\n\n Makefile | 1 +\n fast-import.c | 77 +++++++++++++----------------------------------------------\n mem-pool.c | 55 ++++++++++++++++++++++++++++++++++++++++++\n mem-pool.h | 34 ++++++++++++++++++++++++++\n 4 files changed, 106 insertions(+), 61 deletions(-) create mode 100644 mem-pool.c create mode 100644 mem-pool.h\n\n--\n2.14.3\n\n"}]}