From: Stephen Boyd <hidden> Date: 2016-06-15 22:57:04
(resending since the attachment seems to make vger sad)
Hi,
I'm running git rev-list | git cherry-pick --stdin on a range of about
300 commits. Eventually the chery-pick dies with:
error: cannot fork() for commit: Cannot allocate memory
Running valgrind shows me that the tree traversal code is leaking
gigabytes of memory (particularly unpack_callback). Since cherry-pick is
a very long running process all these allocations are never freed and
eventually I run out of memory. The worst offender and summary is:
==7986== 938,956,692 (929,961,582 direct, 8,995,110 indirect) bytes in
7,765,439 blocks are definitely lost in loss record 257 of 257
==7986== at 0x4C267CC: calloc (vg_replace_malloc.c:467)
==7986== by 0x4FAF57: xcalloc (wrapper.c:119)
==7986== by 0x4F5281: unpack_callback (unpack-trees.c:539)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986==
==7986== LEAK SUMMARY:
==7986== definitely lost: 2,514,117,692 bytes in 21,210,861 blocks
==7986== indirectly lost: 885,481,947 bytes in 10,165,801 blocks
==7986== possibly lost: 650,712,395 bytes in 6,014,309 blocks
==7986== still reachable: 7,734,870 bytes in 47,794 blocks
==7986== suppressed: 0 bytes in 0 blocks
This is against recent git.git
(89740333e8d398f1da701e9023675321bbb9a85b). A workaround is to limit the
amount of commits per cherry-pick invocation, but can we somehow fix the
leaks?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
From: René Scharfe <hidden> Date: 2016-06-15 22:57:05
Am 30.04.2013 02:11, schrieb Stephen Boyd:
(resending since the attachment seems to make vger sad)
Hi,
I'm running git rev-list | git cherry-pick --stdin on a range of about
300 commits. Eventually the chery-pick dies with:
error: cannot fork() for commit: Cannot allocate memory
Running valgrind shows me that the tree traversal code is leaking
gigabytes of memory (particularly unpack_callback). Since cherry-pick is
a very long running process all these allocations are never freed and
eventually I run out of memory. The worst offender and summary is:
==7986== 938,956,692 (929,961,582 direct, 8,995,110 indirect) bytes in
7,765,439 blocks are definitely lost in loss record 257 of 257
==7986== at 0x4C267CC: calloc (vg_replace_malloc.c:467)
==7986== by 0x4FAF57: xcalloc (wrapper.c:119)
==7986== by 0x4F5281: unpack_callback (unpack-trees.c:539)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986==
==7986== LEAK SUMMARY:
==7986== definitely lost: 2,514,117,692 bytes in 21,210,861 blocks
==7986== indirectly lost: 885,481,947 bytes in 10,165,801 blocks
==7986== possibly lost: 650,712,395 bytes in 6,014,309 blocks
==7986== still reachable: 7,734,870 bytes in 47,794 blocks
==7986== suppressed: 0 bytes in 0 blocks
I looked at that particular leak a year ago but couldn't convince myself
to submit the patch below. If the callback function we call through
call_unpack_fn does something strange like free()ing entries itself or
adding them to some list without duplication then the added free() can
cause trouble.
Looking at it again today I don't understand that concern any more. The
current callback functions don't do something like that, in any case.
Maybe I'm missing something.
Anyway, could you please check if the patch helps with your use case?
Thanks,
René
@@ -590,8 +590,13 @@ static int unpack_nondirectories(int n, unsigned long mask,src[i+o->merge]=create_ce_entry(info,names+i,stage);}-if(o->merge)-returncall_unpack_fn(src,o);+if(o->merge){+intret=call_unpack_fn(src,o);+for(i=1;i<n+1;i++)+if(src[i]&&src[i]!=o->df_conflict_entry)+free(src[i]);+returnret;+}for(i=0;i<n;i++)if(src[i]&&src[i]!=o->df_conflict_entry)
From: Stephen Boyd <hidden> Date: 2016-06-15 22:57:06
On 04/30/13 15:47, René Scharfe wrote:
Am 30.04.2013 02:11, schrieb Stephen Boyd:
quoted
(resending since the attachment seems to make vger sad)
Hi,
I'm running git rev-list | git cherry-pick --stdin on a range of about
300 commits. Eventually the chery-pick dies with:
error: cannot fork() for commit: Cannot allocate memory
Running valgrind shows me that the tree traversal code is leaking
gigabytes of memory (particularly unpack_callback). Since cherry-pick is
a very long running process all these allocations are never freed and
eventually I run out of memory. The worst offender and summary is:
==7986== 938,956,692 (929,961,582 direct, 8,995,110 indirect) bytes in
7,765,439 blocks are definitely lost in loss record 257 of 257
==7986== at 0x4C267CC: calloc (vg_replace_malloc.c:467)
==7986== by 0x4FAF57: xcalloc (wrapper.c:119)
==7986== by 0x4F5281: unpack_callback (unpack-trees.c:539)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986==
==7986== LEAK SUMMARY:
==7986== definitely lost: 2,514,117,692 bytes in 21,210,861 blocks
==7986== indirectly lost: 885,481,947 bytes in 10,165,801 blocks
==7986== possibly lost: 650,712,395 bytes in 6,014,309 blocks
==7986== still reachable: 7,734,870 bytes in 47,794 blocks
==7986== suppressed: 0 bytes in 0 blocks
I looked at that particular leak a year ago but couldn't convince myself
to submit the patch below. If the callback function we call through
call_unpack_fn does something strange like free()ing entries itself or
adding them to some list without duplication then the added free() can
cause trouble.
Looking at it again today I don't understand that concern any more. The
current callback functions don't do something like that, in any case.
Maybe I'm missing something.
Anyway, could you please check if the patch helps with your use case?
Ok I think I will make a copy of my .git first before I try out your
patch. In case you're curious here are the next big leaks.
==7986== 433,116,790 (432,950,308 direct, 166,482 indirect) bytes in 4,146,402 blocks are definitely lost in loss record 253 of 257
==7986== at 0x4C267CC: calloc (vg_replace_malloc.c:467)
==7986== by 0x4FAF57: xcalloc (wrapper.c:119)
==7986== by 0x4F5281: unpack_callback (unpack-trees.c:539)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F6FAE: unpack_trees (unpack-trees.c:1074)
==7986== by 0x4AD095: git_merge_trees (merge-recursive.c:241)
==7986== by 0x4AF3D5: merge_trees (merge-recursive.c:1811)
==7986== by 0x4D9A7A: do_pick_commit (sequencer.c:311)
==7986==
==7986== 482,083,201 (46,465,928 direct, 435,617,273 indirect) bytes in 93 blocks are definitely lost in loss record 255 of 257
==7986== at 0x4C267CC: calloc (vg_replace_malloc.c:467)
==7986== by 0x4FAF57: xcalloc (wrapper.c:119)
==7986== by 0x4C4AE4: read_index_from (read-cache.c:1452)
==7986== by 0x4D99BC: do_pick_commit (sequencer.c:297)
==7986== by 0x4DA750: pick_commits (sequencer.c:995)
==7986== by 0x4DAFD6: sequencer_pick_revisions (sequencer.c:1124)
==7986== by 0x463E7C: cmd_cherry_pick (revert.c:236)
==7986== by 0x404C86: handle_internal_command (git.c:284)
==7986== by 0x40541C: main (git.c:492)
==7986==
==7986== 557,706,880 (548,062,684 direct, 9,644,196 indirect) bytes in 4,931,819 blocks are definitely lost in loss record 256 of 257
==7986== at 0x4C267CC: calloc (vg_replace_malloc.c:467)
==7986== by 0x4FAF57: xcalloc (wrapper.c:119)
==7986== by 0x4F5281: unpack_callback (unpack-trees.c:539)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F6FAE: unpack_trees (unpack-trees.c:1074)
==7986== by 0x4AD095: git_merge_trees (merge-recursive.c:241)
==7986==
Full report can be found at https://gist.github.com/bebarino/5497181
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
From: Stephen Boyd <hidden> Date: 2016-06-15 22:57:09
On 04/30/13 15:47, René Scharfe wrote:
Am 30.04.2013 02:11, schrieb Stephen Boyd:
quoted
(resending since the attachment seems to make vger sad)
Hi,
I'm running git rev-list | git cherry-pick --stdin on a range of about
300 commits. Eventually the chery-pick dies with:
error: cannot fork() for commit: Cannot allocate memory
Running valgrind shows me that the tree traversal code is leaking
gigabytes of memory (particularly unpack_callback). Since cherry-pick is
a very long running process all these allocations are never freed and
eventually I run out of memory. The worst offender and summary is:
==7986== 938,956,692 (929,961,582 direct, 8,995,110 indirect) bytes in
7,765,439 blocks are definitely lost in loss record 257 of 257
==7986== at 0x4C267CC: calloc (vg_replace_malloc.c:467)
==7986== by 0x4FAF57: xcalloc (wrapper.c:119)
==7986== by 0x4F5281: unpack_callback (unpack-trees.c:539)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986== by 0x4F586C: unpack_callback (unpack-trees.c:467)
==7986== by 0x4F40E5: traverse_trees (tree-walk.c:407)
==7986==
==7986== LEAK SUMMARY:
==7986== definitely lost: 2,514,117,692 bytes in 21,210,861 blocks
==7986== indirectly lost: 885,481,947 bytes in 10,165,801 blocks
==7986== possibly lost: 650,712,395 bytes in 6,014,309 blocks
==7986== still reachable: 7,734,870 bytes in 47,794 blocks
==7986== suppressed: 0 bytes in 0 blocks
I looked at that particular leak a year ago but couldn't convince myself
to submit the patch below. If the callback function we call through
call_unpack_fn does something strange like free()ing entries itself or
adding them to some list without duplication then the added free() can
cause trouble.
Looking at it again today I don't understand that concern any more. The
current callback functions don't do something like that, in any case.
Maybe I'm missing something.
Anyway, could you please check if the patch helps with your use case?
Ok. I tested it and it definitely helps.
==10728== LEAK SUMMARY:
==10728== definitely lost: 316,355,458 bytes in 8,652 blocks
==10728== indirectly lost: 1,327,251,588 bytes in 16,180,628 blocks
==10728== possibly lost: 677,049,918 bytes in 7,381,801 blocks
==10728== still reachable: 9,238,039 bytes in 63,947 blocks
==10728== suppressed: 0 bytes in 0 blocks
vs.
==27614== LEAK SUMMARY:
==27614== definitely lost: 2,369,692,222 bytes in 20,005,707 blocks
==27614== indirectly lost: 829,151,786 bytes in 9,594,715 blocks
==27614== possibly lost: 658,069,373 bytes in 6,345,172 blocks
==27614== still reachable: 8,806,386 bytes in 50,199 blocks
==27614== suppressed: 0 bytes in 0 blocks
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
From: René Scharfe <hidden> Date: 2016-06-15 22:57:11
Am 06.05.2013 22:16, schrieb Stephen Boyd:
Ok. I tested it and it definitely helps.
==10728== LEAK SUMMARY:
==10728== definitely lost: 316,355,458 bytes in 8,652 blocks
==10728== indirectly lost: 1,327,251,588 bytes in 16,180,628 blocks
==10728== possibly lost: 677,049,918 bytes in 7,381,801 blocks
==10728== still reachable: 9,238,039 bytes in 63,947 blocks
==10728== suppressed: 0 bytes in 0 blocks
vs.
==27614== LEAK SUMMARY:
==27614== definitely lost: 2,369,692,222 bytes in 20,005,707 blocks
==27614== indirectly lost: 829,151,786 bytes in 9,594,715 blocks
==27614== possibly lost: 658,069,373 bytes in 6,345,172 blocks
==27614== still reachable: 8,806,386 bytes in 50,199 blocks
==27614== suppressed: 0 bytes in 0 blocks
Thanks, Stephen. I'm going to prepare a series around that patch which
will (hopefully) show that freeing these entries is safe by passing only
const pointers down to the callbacks. It's too late for 1.8.3, of
course, but it shouldn't take another year as most of that series is
done already. :)
We still have an impressive amount of leakage here. I wonder why
"indirectly lost" increased so much. Do you perhaps still have the full
output of valgrind for the run with the patch applied?
Thanks,
René
From: Stephen Boyd <hidden> Date: 2016-06-15 22:57:11
On 05/09/13 08:10, René Scharfe wrote:
Am 06.05.2013 22:16, schrieb Stephen Boyd:
quoted
Ok. I tested it and it definitely helps.
==10728== LEAK SUMMARY:
==10728== definitely lost: 316,355,458 bytes in 8,652 blocks
==10728== indirectly lost: 1,327,251,588 bytes in 16,180,628 blocks
==10728== possibly lost: 677,049,918 bytes in 7,381,801 blocks
==10728== still reachable: 9,238,039 bytes in 63,947 blocks
==10728== suppressed: 0 bytes in 0 blocks
vs.
==27614== LEAK SUMMARY:
==27614== definitely lost: 2,369,692,222 bytes in 20,005,707 blocks
==27614== indirectly lost: 829,151,786 bytes in 9,594,715 blocks
==27614== possibly lost: 658,069,373 bytes in 6,345,172 blocks
==27614== still reachable: 8,806,386 bytes in 50,199 blocks
==27614== suppressed: 0 bytes in 0 blocks
Thanks, Stephen. I'm going to prepare a series around that patch
which will (hopefully) show that freeing these entries is safe by
passing only const pointers down to the callbacks. It's too late for
1.8.3, of course, but it shouldn't take another year as most of that
series is done already. :)
Yes I started trying to throw const around everywhere but then I just
sent out the email in hopes someone had already solved this problem.
We still have an impressive amount of leakage here. I wonder why
"indirectly lost" increased so much. Do you perhaps still have the
full output of valgrind for the run with the patch applied?
I think it increased because the first one ran to completion while the
second one failed half way through so it's not an apples to apples
comparison. I can re-run with a certain range that is known not to fail
if you're interested, but I think you got the idea that the patch helps.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation