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 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