Re: [REGRESSION, BISECTED] `git checkout <branch>` started to be memory hog

On Fri, Jul 22, 2011 at 05:05:18PM +0400, Kirill Smelkov wrote:

It turned out that with Git v1.7.6 memory usage for git-checkout
linux-3.0.y as seen in top is

    VIRTmax     RESmax

    ~338M       ~247M

and for master

    VIRTmax     RESmax
    (both till not killed)
   ~2200M       ~1000M


i.e. it looks like when residential memory usage approaches the amount of
physical RAM, the OOM killer comes into play.


And I've bisected this to b6691092 ("Add streaming filter API"; Junio C
Hamano, May 20 2011; merged to next on Jun 30 2011):

Hmm, that series was supposed to _reduce_ memory usage. :)

According to valgrind, we are leaking gigabytes of memory allocated in
git_istream buffers:

  $ cd linux-2.6
  $ rm -vrf *
  $ valgrind --leak-check=full git checkout -f
  [...]
  2,418,940,448 (1,163,660,832 direct, 1,255,279,616 indirect) bytes in 35,271
        blocks are definitely lost in loss record 81 of 81
     at 0x4C2780D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
     by 0x517C62: xmalloc (wrapper.c:35)
     by 0x503112: attach_stream_filter (streaming.c:255)
     by 0x502D61: open_istream (streaming.c:152)
     by 0x4B1B7A: streaming_write_entry (entry.c:130)
     by 0x4B1E0B: write_entry (entry.c:193)
     by 0x4B23F4: checkout_entry (entry.c:318)
     by 0x511DA4: check_updates (unpack-trees.c:223)
     by 0x513D72: unpack_trees (unpack-trees.c:1125)
     by 0x41CCB4: reset_tree (checkout.c:333)
     by 0x41CE32: merge_working_tree (checkout.c:378)
     by 0x41DE6A: switch_branches (checkout.c:737)

This malloc is for the actual git_istream struct. It seems that we never
actually free it when calling close_istream(). And these structs are
quite big; they contain 32K of filter buffers inside a union.

From my quick look, I came up with the fix below. It removes the leak

and doesn't trigger any memory errors according to valgrind. So it
_must_ be right. :)

-Peff

---

diff --git a/streaming.c b/streaming.c
index 565f000..f3acc5d 100644
--- a/streaming.c
+++ b/streaming.c

@@ -93,7 +93,9 @@ struct git_istream {
 
 int close_istream(struct git_istream *st)
 {
-	return st->vtbl->close(st);
+	int r = st->vtbl->close(st);
+	free(st);
+	return r;
 }
 
 ssize_t read_istream(struct git_istream *st, char *buf, size_t sz)