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

[Stephen Boyd <sboyd@codeaurora.org>]

On 04/30/13 15:47, René Scharfe wrote:
> 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?

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