threads / patch / 49903

patchmem_pool: add GIT_TRACE_MEMPOOL support

Subject: [PATCH v1] mem_pool: add GIT_TRACE_MEMPOOL support

## tl;dr

4 messages between Nov 27, 2018 and Nov 29, 2018. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Ben Peart· Nov 27, 2018, 19:53 UTC · lore
From: Ben Peart <benpeart@microsoft.com>

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 <benpeart@microsoft.com>
---
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(+)
Show changes to mem-pool.c +5 −0
diff --git a/mem-pool.c b/mem-pool.c
index a2841a4a9a..065389aaec 100644
--- a/mem-pool.c
+++ b/mem-pool.c
@@ -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

-- 
2.18.0.windows.1
Johannes Schindelin· Nov 28, 2018, 09:37 UTC · re: Ben Peart · lore

Re: [PATCH v1] mem_pool: add GIT_TRACE_MEMPOOL support

Hi Ben,
On Tue, 27 Nov 2018, Ben Peart wrote:
Show 8 quoted lines
> From: Ben Peart <benpeart@microsoft.com>
> 
> 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 <benpeart@microsoft.com>
> ---
Looks good.

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

Ciao, Johannes

Show 44 quoted 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(+)
> 
> diff --git a/mem-pool.c b/mem-pool.c
> index a2841a4a9a..065389aaec 100644
> --- a/mem-pool.c
> +++ b/mem-pool.c
> @@ -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
> -- 
> 2.18.0.windows.1
> 
>
Ben Peart· Nov 29, 2018, 14:09 UTC · re: Johannes Schindelin · lore

Re: [PATCH v1] mem_pool: add GIT_TRACE_MEMPOOL support

On 11/28/2018 4:37 AM, Johannes Schindelin wrote:
Show 16 quoted lines
> Hi Ben,
>
> On Tue, 27 Nov 2018, Ben Peart wrote:
>
>> From: Ben Peart <benpeart@microsoft.com>
>>
>> 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 <benpeart@microsoft.com>
>> ---
> 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.

Show 47 quoted lines
>
> Ciao,
> Johannes
>
>> 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(+)
>>
>> diff --git a/mem-pool.c b/mem-pool.c
>> index a2841a4a9a..065389aaec 100644
>> --- a/mem-pool.c
>> +++ b/mem-pool.c
>> @@ -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
>> -- 
>> 2.18.0.windows.1
>>
>>
Johannes Schindelin· Nov 29, 2018, 15:37 UTC · re: Ben Peart · lore

Re: [PATCH v1] mem_pool: add GIT_TRACE_MEMPOOL support

Hi Ben,
On Thu, 29 Nov 2018, Ben Peart wrote:
Show 23 quoted lines
> On 11/28/2018 4:37 AM, Johannes Schindelin wrote:
> > Hi Ben,
> >
> > On Tue, 27 Nov 2018, Ben Peart wrote:
> >
> > > From: Ben Peart <benpeart@microsoft.com>
> > >
> > > 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 <benpeart@microsoft.com>
> > > ---
> > 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.
In other words, YAGNI. Makes sense.

Thanks, Johannes

Show 54 quoted lines
> 
> >
> > Ciao,
> > Johannes
> >
> > > 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(+)
> > >
> > > diff --git a/mem-pool.c b/mem-pool.c
> > > index a2841a4a9a..065389aaec 100644
> > > --- a/mem-pool.c
> > > +++ b/mem-pool.c
> > > @@ -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
> > > -- 
> > > 2.18.0.windows.1
> > >
> > >
> 
>

← back to recent threads