{"thread":{"id":"4317","subject":"Fix memory leak in \"git rev-list --objects\"","startedAt":"2006-05-28T18:37:23Z","lastAt":"2006-05-28T18:37:23Z","messageCount":1,"participants":["Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"20830","messageId":"Pine.LNX.4.64.0605281114340.5623@g5.osdl.org","threadId":"4317","inReplyTo":null,"subject":"Fix memory leak in \"git rev-list --objects\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-28T18:37:23Z","receivedAt":"2006-05-28T18:37:23Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nMartin Langhoff points out that \"git repack -a\" ends up using up a lot of \nmemory for big archives, and that git cvsimport probably should do only \nincremental repacks in order to avoid having repacking flush all the \ncaches.\n\nThe big majority of the memory usage of repacking is from git rev-list \ntracking all objects, and this patch should go a long way in avoiding the \nexcessive memory usage: the bulk of it was due to the object names being \nleaked from the tree parser.\n\nFor the historic Linux kernel archive, this simple patch does:\n\nBefore:\n\t/usr/bin/time git-rev-list --all --objects > /dev/null \n\n\t72.45user 0.82system 1:13.55elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k\n\t0inputs+0outputs (0major+125376minor)pagefaults 0swaps\n\nAfter:\n\t/usr/bin/time git-rev-list --all --objects > /dev/null \n\n\t75.22user 0.48system 1:16.34elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k\n\t0inputs+0outputs (0major+43921minor)pagefaults 0swaps\n\nwhere we do end up wasting a bit of time on some extra strdup()s (which \ncould be avoided, but that would require tracking where the pathnames came \nfrom), but we avoid a lot of memory usage.\n\nMinor page faults track maximum RSS very closely (each page fault maps in \none page into memory), so the reduction from 125376 page faults to 43921 \nmeans a rough reduction of VM footprint from almost half a gigabyte to \nabout a third of that. Those numbers were also double-checked by looking \nat \"top\" while the process was running.\n\n(Side note: at least part of the remaining VM footprint is the mapping of \nthe 177MB pack-file, so the remaining memory use is at least partly \"well \nbehaved\" from a project caching perspective).\n\nFor the current git archive itself, the memory usage for a \"--all \n--objects\" rev-list invocation dropped from 7128 pages to 2318 (27MB to \n9MB), so the reduction seems to hold for much smaller projects too.\n\nFor regular \"git-rev-list\" usage (ie without the \"--objects\" flag) this \npatch has no impact.\n\nSigned-off-by: Linus Torvalds <torvalds@osdl.org>\n---\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex f11dbd6..5277d3c 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -103,6 +103,7 @@ static struct object_list **process_blob\n \tif (obj->flags & (UNINTERESTING | SEEN))\n \t\treturn p;\n \tobj->flags |= SEEN;\n+\tname = strdup(name);\n \treturn add_object(obj, p, path, name);\n }\n \n@@ -122,6 +123,7 @@ static struct object_list **process_tree\n \tif (parse_tree(tree) < 0)\n \t\tdie(\"bad tree object %s\", sha1_to_hex(obj->sha1));\n \tobj->flags |= SEEN;\n+\tname = strdup(name);\n \tp = add_object(obj, p, path, name);\n \tme.up = path;\n \tme.elem = name;\n@@ -134,6 +136,7 @@ static struct object_list **process_tree\n \t\t\tp = process_tree(entry->item.tree, p, &me, entry->name);\n \t\telse\n \t\t\tp = process_blob(entry->item.blob, p, &me, entry->name);\n+\t\tfree(entry->name);\n \t\tfree(entry);\n \t\tentry = next;\n \t}\n"}]}