Huge possible memory leak while cherry-picking.
Hi,
I have tried to cherry-pick a range of ~200 commits from one branch to another. And you can't imagine how I was surprised when the git process ate 8 Gb of RAM and died - before cherry-picking was complete.
I downloaded git sources from master and built it with gperftools support (-ltcmalloc). After running git cherry-pick <some hash> with a heap-leak checker enabled I got this:
Quoted 2 lines
Have memory regions w/o callers: might report false leaks Leak check _main_ detected leaks of 42838782 bytes in 257986 objects
These objects are allocated at
Quoted 1 line
read-cache.c:1340: struct
cache_entry*ce = xmalloc(cache_entry_size(len));
After looking in the code, I found a comment in the function static void remove_dir_entry(...):
/*
* Release reference to the directory entry (and parents if 0). *
* Note: we do not remove / free the entry because there's no
* hash.[ch]::remove_hash and dir->next may point to other entries
* that are still valid, so we must not free the memory. */
So, this objects are never freed - by design?
Is it a real issue, or do I just misunderstand something?
Quoted 3 lines · Лежанкин Иван
I have tried to cherry-pick a range of ~200 commits from one branch to another. And you can't imagine how I was surprised when the git process ate 8 Gb of RAM and died - before cherry-picking was complete.
Try this:
http://article.gmane.org/gmane.comp.version-control.git/226757
Signature · 1 line
Felipe Contreras
Thank you, it works very well!
Will this patch go to upstream?
Also, there is still some unexpected memory consumption - about 2Gb per ~200 commits, but it's bearable.
I will do a futher investigation.
Felipe, should I exclude you from my futher reports on possible memory leaks?
Quoted 10 lines · Felipe Contreras
Quoted 3 lines · Лежанкин Иван
I have tried to cherry-pick a range of ~200 commits from one branch to another. And you can't imagine how I was surprised when the git process ate 8 Gb of RAM and died - before cherry-picking was complete.
Try this:
http://article.gmane.org/gmane.comp.version-control.git/226757--
Felipe Contreras
Quoted 2 lines · Лежанкин Иван
Thank you, it works very well!
Will this patch go to upstream?
Ask Junio.
Quoted 3 lines
Also, there is still some unexpected memory consumption - about 2Gb per ~200 commits, but it's bearable.
I will do a futher investigation.
Can you post some valgrind log? Or even better, a way to reproduce?
Quoted 1 line
Felipe, should I exclude you from my futher reports on possible memory leaks?
Exclude me?
Signature · 1 line
Felipe Contreras
PATCH 1/1 unpack-trees: plug a memory leak
From: Felipe Contreras <[email protected]>
Before overwriting the destination index, first let's discard its contents.
---
Felipe sent this patch as part of multiple series in June, but it can stand on its own. This version is trivially rebased against master. The leak seems to have been introduced by 34110cd4 (2008-03-06, "Make 'unpack_trees()' have a separate source and destination index").
unpack-trees.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
unpack-trees.c +3 -1
@@ -1154,8 +1154,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options o->src_index =NULL ; ret =check_updates (o) ? (-2 ) :0 ; -if (o->dst_index) +if (o->dst_index) { +discard_index (o->dst_index); *o->dst_index = o->result; + } done:clear_exclude_list (&el);
Signature · 1 line
1.8.3.3
Re: [PATCH] unpack-trees: plug a memory leak
Quoted 5 lines · René Scharfe
From: Felipe Contreras <[email protected]>
Before overwriting the destination index, first let's discard its contents.
Quoted 7 lines
Signed-off-by Felipe Contreras <[email protected]>Tested-by Лежанкин Иван <[email protected]> wrote:---
Felipe sent this patch as part of multiple series in June, but it can stand on its own. This version is trivially rebased against master. The leak seems to have been introduced by34110cd4(2008-03-06, "Make 'unpack_trees()' have a separate source and destination index").
It was lost in the follow-up discussion and I missed it.
I assume that this is signed-off by you as a forwarder? I'd prefer to even mark it Reviewed-by: you.
Thanks.
Quoted 19 lines
unpack-trees.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)unpack-trees.c +3
-1@@ -1154,8 +1154,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options o->src_index =NULL ; ret =check_updates (o) ? (-2 ) :0 ; -if (o->dst_index) +if (o->dst_index) { +discard_index (o->dst_index); *o->dst_index = o->result; + } done:clear_exclude_list (&el);
Re: [PATCH] unpack-trees: plug a memory leak
Am 13.08.2013 23:12, schrieb Junio C Hamano:
Quoted 15 lines
Quoted 5 lines · René Scharfe
From: Felipe Contreras <[email protected]>
Before overwriting the destination index, first let's discard its contents.
Quoted 7 lines
Signed-off-by Felipe Contreras <[email protected]>Tested-by Лежанкин Иван <[email protected]> wrote:---
Felipe sent this patch as part of multiple series in June, but it can stand on its own. This version is trivially rebased against master. The leak seems to have been introduced by34110cd4(2008-03-06, "Make 'unpack_trees()' have a separate source and destination index").
Quoted 1 line
It was lost in the follow-up discussion and I missed it.
I had forgotten about it as well, until Felipe mentioned it again.
Quoted 2 lines
I assume that this is signed-off by you as a forwarder? I'd prefer to even mark it Reviewed-by: you.
Right, I did review the patch and you can tag it as such.
Thanks,
René
Re: [PATCH] unpack-trees: plug a memory leak
Quoted 8 lines · René Scharfe
Quoted 1 line
It was lost in the follow-up discussion and I missed it.
I had forgotten about it as well, until Felipe mentioned it again.
Quoted 2 lines
I assume that this is signed-off by you as a forwarder? I'd prefer to even mark it Reviewed-by: you.
Right, I did review the patch and you can tag it as such.
OK, will queue and soon merge to and cook in 'next'.
Thanks, everybody.
Quoted 2 lines · Лежанкин Иван
Thank you, it works very well!
Will this patch go to upstream?
Ask Junio.
Quoted 3 lines
Also, there is still some unexpected memory consumption - about 2Gb per ~200 commits, but it's bearable.
I will do a futher investigation.
Can you post some valgrind log? Or even better, a way to reproduce?
Quoted 1 line
Felipe, should I exclude you from my futher reports on possible memory leaks?
Exclude me?
Signature · 1 line
Felipe Contreras