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

Hello up there,

Today I've tried to upgrade to linux-3.0 on my netbook with 1GB of RAM,
with today's Git master (v1.7.6-233-gd79bcd6) and Git was killed by an OOM:

    $ git new-workdir linux linux-3.0 linux-3.0.y
    Checking out files: ~80% (XXXXX/36783)
    Aborted
    (with OOM messages in dmesg).

git new-workdir boils down to (uninteresting to the issue) symlinks
setup + final `git checkout -f <branch>` and that final git checkout is
failing.


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

    commit b6691092d707860019bbab80eaaf9173ada10586
    Author: Junio C Hamano [off-list ref]
    Date:   Fri May 20 14:33:31 2011 -0700

        Add streaming filter API

        This introduces an API to plug custom filters to an input stream.

        The caller gets get_stream_filter("path") to obtain an appropriate
        filter for the path, and then uses it when opening an input stream
        via open_istream().  After that, the caller can read from the stream
        with read_istream(), and close it with close_istream(), just like an
        unfiltered stream.

        This only adds a "null" filter that is a pass-thru filter, but later
        changes can add LF-to-CRLF and other filters, and the callers of the
        streaming API do not have to change.

        Signed-off-by: Junio C Hamano [off-list ref]



Hope Git stays usable for us, with not enormous amount of RAM onboard,
and thanks beforehand for fixing,

Kirill

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)