mem_pool: add GIT_TRACE_MEMPOOL support

PATCH In review Ben Peart 2 participants 4 messages Nov 27, 2018 – Nov 29, 2018

lore

Share & tools

lore

Ben Peart

author

2018-11-27 19:53 UTC #0 lore raw

PATCH 1/1 mem_pool: add GIT_TRACE_MEMPOOL support

From: Ben Peart <[email protected]>

Add tracing around initializing and discarding mempools. In discard report on the amount of memory unused in the current block to help tune setting the initial_size.

Signed-off-by Ben Peart <[email protected]>

---

Notes:
Base Ref: * git-trace-mempool
Web-Diff: https://github.com/benpeart/git/commit/9ac84bbca2
Checkout: git fetch https://github.com/benpeart/git git-trace-mempool-v1 && git checkout 9ac84bbca2

 mem-pool.c | 5 +++++
 1 file changed, 5 insertions(+)

mem-pool.c +5 -0

index a2841a4a9a..065389aaec 100644
@@ -5,6 +5,7 @@
 #include "cache.h"
 #include "mem-pool.h"
 
+static struct trace_key trace_mem_pool = TRACE_KEY_INIT(MEMPOOL);
 #define BLOCK_GROWTH_SIZE 1024*1024 - sizeof(struct mp_block);
 
 /*

@@ -48,12 +49,16 @@ void mem_pool_init(struct mem_pool **mem_pool, size_t initial_size)
 		mem_pool_alloc_block(pool, initial_size, NULL);
 
 	*mem_pool = pool;
+	trace_printf_key(&trace_mem_pool, "mem_pool (%p): init (%"PRIuMAX") initial size\n",
+		pool, (uintmax_t)initial_size);
 }
 
 void mem_pool_discard(struct mem_pool *mem_pool, int invalidate_memory)
 {
 	struct mp_block *block, *block_to_free;
 
+	trace_printf_key(&trace_mem_pool, "mem_pool (%p): discard (%"PRIuMAX") unused\n",
+		mem_pool, (uintmax_t)(mem_pool->mp_block->end - mem_pool->mp_block->next_free));
 	block = mem_pool->mp_block;
 	while (block)
 	{

base-commit: bb75be6cb916297f271c846f2f9caa3daaaec718

Signature · 1 line

2.18.0.windows.1

Johannes Schindelin

2018-11-28 09:37 UTC #1 lore raw

Hi Ben,

Quoted 8 lines · Ben Peart

From: Ben Peart <[email protected]>
Add tracing around initializing and discarding mempools. In discard report on the amount of memory unused in the current block to help tune setting the initial_size.
Signed-off-by Ben Peart <[email protected]>
---

Looks good.

My only question: should we also trace calls to _alloc(), _calloc() and _combine()?

Ciao,
Johannes

Quoted 44 lines

Notes:
Base Ref: * git-trace-mempool
Web-Diff: https://github.com/benpeart/git/commit/9ac84bbca2
Checkout: git fetch https://github.com/benpeart/git git-trace-mempool-v1 && git checkout 9ac84bbca2

 mem-pool.c | 5 +++++
 1 file changed, 5 insertions(+)

mem-pool.c +5 -0

index a2841a4a9a..065389aaec 100644
@@ -5,6 +5,7 @@
 #include "cache.h"
 #include "mem-pool.h"
 
+static struct trace_key trace_mem_pool = TRACE_KEY_INIT(MEMPOOL);
 #define BLOCK_GROWTH_SIZE 1024*1024 - sizeof(struct mp_block);
 
 /*

@@ -48,12 +49,16 @@ void mem_pool_init(struct mem_pool **mem_pool, size_t initial_size)
 		mem_pool_alloc_block(pool, initial_size, NULL);
 
 	*mem_pool = pool;
+	trace_printf_key(&trace_mem_pool, "mem_pool (%p): init (%"PRIuMAX") initial size\n",
+		pool, (uintmax_t)initial_size);
 }
 
 void mem_pool_discard(struct mem_pool *mem_pool, int invalidate_memory)
 {
 	struct mp_block *block, *block_to_free;
 
+	trace_printf_key(&trace_mem_pool, "mem_pool (%p): discard (%"PRIuMAX") unused\n",
+		mem_pool, (uintmax_t)(mem_pool->mp_block->end - mem_pool->mp_block->next_free));
 	block = mem_pool->mp_block;
 	while (block)
 	{

base-commit: bb75be6cb916297f271c846f2f9caa3daaaec718

Signature · 1 line

2.18.0.windows.1

Ben Peart

author

2018-11-29 14:09 UTC #2 lore raw

Quoted 10 lines · Johannes Schindelin

Hi Ben,
Quoted 6 lines · Ben Peart
From: Ben Peart <[email protected]>
Add tracing around initializing and discarding mempools. In discard report on the amount of memory unused in the current block to help tune setting the initial_size.

Quoted 6 lines

Quoted 2 lines
Signed-off-by Ben Peart <[email protected]>
---
Looks good.
My only question: should we also trace calls to _alloc(), _calloc() and _combine()?

I was trying to tune the initial size in my use of the mem_pool and so found this tracing useful to see how much memory was actually being used. I'm inclined to only add tracing as it is needed rather that proactively because we think it _might_ be needed. I suspect _alloc() and _calloc() would get very noisy and not add much value.

Quoted 47 lines

Ciao,
Johannes

Quoted 43 lines

Notes:
Base Ref: * git-trace-mempool
Web-Diff: https://github.com/benpeart/git/commit/9ac84bbca2
Checkout: git fetch https://github.com/benpeart/git git-trace-mempool-v1 && git checkout 9ac84bbca2

  mem-pool.c | 5 +++++
  1 file changed, 5 insertions(+)

mem-pool.c +5 -0

index a2841a4a9a..065389aaec 100644
@@ -5,6 +5,7 @@
  #include "cache.h"
  #include "mem-pool.h"
 
+static struct trace_key trace_mem_pool = TRACE_KEY_INIT(MEMPOOL);
  #define BLOCK_GROWTH_SIZE 1024*1024 - sizeof(struct mp_block);
 
  /*

@@ -48,12 +49,16 @@ void mem_pool_init(struct mem_pool **mem_pool, size_t initial_size)
  		mem_pool_alloc_block(pool, initial_size, NULL);
 
  	*mem_pool = pool;
+	trace_printf_key(&trace_mem_pool, "mem_pool (%p): init (%"PRIuMAX") initial size\n",
+		pool, (uintmax_t)initial_size);
  }
 
  void mem_pool_discard(struct mem_pool *mem_pool, int invalidate_memory)
  {
  	struct mp_block *block, *block_to_free;
 
+	trace_printf_key(&trace_mem_pool, "mem_pool (%p): discard (%"PRIuMAX") unused\n",
+		mem_pool, (uintmax_t)(mem_pool->mp_block->end - mem_pool->mp_block->next_free));
  	block = mem_pool->mp_block;
  	while (block)
  	{

base-commit: bb75be6cb916297f271c846f2f9caa3daaaec718

Signature · 1 line

2.18.0.windows.1

Johannes Schindelin

2018-11-29 15:37 UTC #3 lore raw

Hi Ben,

Quoted 15 lines · Ben Peart

Quoted 14 lines · Johannes Schindelin
Hi Ben,
Quoted 8 lines · Ben Peart
From: Ben Peart <[email protected]>
Add tracing around initializing and discarding mempools. In discard report on the amount of memory unused in the current block to help tune setting the initial_size.
Signed-off-by Ben Peart <[email protected]>
---
Looks good.

Quoted 8 lines

Quoted 2 lines
My only question: should we also trace calls to _alloc(), _calloc() and _combine()?
I was trying to tune the initial size in my use of the mem_pool and so found this tracing useful to see how much memory was actually being used. I'm inclined to only add tracing as it is needed rather that proactively because we think it _might_ be needed. I suspect _alloc() and _calloc() would get very noisy and not add much value.

In other words, YAGNI. Makes sense.

Thanks,
Johannes

Quoted 54 lines

Quoted 52 lines

Ciao,
Johannes

Quoted 47 lines

Notes:
Base Ref: * git-trace-mempool
Web-Diff: https://github.com/benpeart/git/commit/9ac84bbca2
Checkout: git fetch https://github.com/benpeart/git
git-trace-mempool-v1 && git checkout 9ac84bbca2

  mem-pool.c | 5 +++++
  1 file changed, 5 insertions(+)

mem-pool.c +4 -0

index a2841a4a9a..065389aaec 100644
@@ -5,6 +5,7 @@
  #include "cache.h"
  #include "mem-pool.h"
 
+static struct trace_key trace_mem_pool = TRACE_KEY_INIT(MEMPOOL);
  #define BLOCK_GROWTH_SIZE 1024*1024 - sizeof(struct mp_block);
 
  /*

@@ -48,12 +49,16 @@ void mem_pool_init(struct mem_pool **mem_pool, size_t
 initial_size)
    mem_pool_alloc_block(pool, initial_size, NULL);
 
  	*mem_pool = pool;
+	trace_printf_key(&trace_mem_pool, "mem_pool (%p): init (%"PRIuMAX")
 initial size\n",
+		pool, (uintmax_t)initial_size);
  }
 
  void mem_pool_discard(struct mem_pool *mem_pool, int invalidate_memory)
  {
   struct mp_block *block, *block_to_free;
  +	trace_printf_key(&trace_mem_pool, "mem_pool (%p): discard (%"PRIuMAX")
 unused\n",
+		mem_pool, (uintmax_t)(mem_pool->mp_block->end -

mem_pool->mp_block->next_free));
   block = mem_pool->mp_block;
   while (block)
   {

base-commit: bb75be6cb916297f271c846f2f9caa3daaaec718

Signature · 1 line

2.18.0.windows.1