Re: git rev-list | git cherry-pick --stdin is leaky

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