{"thread":{"id":"12025","subject":"[PATCH] pack-objects: only throw away data during memory pressure","startedAt":"2008-02-11T07:26:25Z","lastAt":"2008-02-13T01:48:06Z","messageCount":12,"participants":["Martin Koegler","Johannes Schindelin","Nicolas Pitre","Brian Downing"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"68332","messageId":"120271478556-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"12025","inReplyTo":null,"subject":"[PATCH] pack-objects: only throw away data during memory pressure","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-02-11T07:26:25Z","receivedAt":"2008-02-11T07:26:25Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"If pack-objects hit the memory limit, it deletes objects from the delta\nwindow.\n\nThis patch make it only delete the data, which is recomputed, if needed again.\n\nSigned-off-by: Martin Koegler \n---\nWhat about this not really tested patch for dealing with memory pressure in git-pack-objects?\n\nIt will slow down the repack in the case of memory pressure, but missing memory will not affect the results.\n\n builtin-pack-objects.c | 13 +++++++++++--\n 1 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 6f8f388..231d65f 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1464,7 +1464,7 @@ static unsigned int check_delta_limit(struct object_entry *me, unsigned int n)\n \treturn m;\n }\n \n-static unsigned long free_unpacked(struct unpacked *n)\n+static unsigned long free_unpacked_data(struct unpacked *n)\n {\n \tunsigned long freed_mem = sizeof_delta_index(n->index);\n \tfree_delta_index(n->index);\n@@ -1474,6 +1474,12 @@ static unsigned long free_unpacked(struct unpacked *n)\n \t\tfree(n->data);\n \t\tn->data = NULL;\n \t}\n+\treturn freed_mem;\n+}\n+\n+static unsigned long free_unpacked(struct unpacked *n)\n+{\n+\tunsigned long freed_mem = free_unpacked_data(n);\n \tn->entry = NULL;\n \tn->depth = 0;\n \treturn freed_mem;\n@@ -1514,7 +1520,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,\n \t\t mem_usage > window_memory_limit &&\n \t\t count > 1) {\n \t\t\tuint32_t tail = (idx + window - count) % window;\n-\t\t\tmem_usage -= free_unpacked(array + tail);\n+\t\t\tmem_usage -= free_unpacked_data(array + tail);\n \t\t\tcount--;\n \t\t}\n \n@@ -1547,6 +1553,9 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,\n \t\t\tif (!m->entry)\n \t\t\t\tbreak;\n \t\t\tret = try_delta(n, m, max_depth, &mem_usage);\n+\t\t\tif (window_memory_limit &&\n+\t\t\t mem_usage > window_memory_limit)\n+\t\t\t\tmem_usage -= free_unpacked_data(m);\n \t\t\tif (ret < 0)\n \t\t\t\tbreak;\n \t\t\telse if (ret > 0)\n-- \n1.5.4.g42f90\n"},{"id":"68397","messageId":"alpine.LSU.1.00.0802111519300.3870@racer.site","threadId":"12025","inReplyTo":"120271478556-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"Re: [PATCH] pack-objects: only throw away data during memory pressure","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-11T15:20:23Z","receivedAt":"2008-02-11T15:20:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 11 Feb 2008, Martin Koegler wrote:\n\n> What about this not really tested patch for dealing with memory pressure \n> in git-pack-objects?\n> \n> It will slow down the repack in the case of memory pressure, but missing \n> memory will not affect the results.\n\nIt almost helped:\n\n$ /usr/bin/time git repack -a -d -f --window=250 --depth=250\nCounting objects: 2477715, done.\nfatal: Out of memory, malloc failed411764)\nCommand exited with non-zero status 1\n10050.12user 240.63system 2:53:37elapsed 98%CPU (0avgtext+0avgdata \n0maxresident)k\n0inputs+0outputs (29555major+94032945minor)pagefaults 0swaps\n\nSo, it ran longer until it ran out of memory.\n\nCiao,\nDscho\n"},{"id":"68402","messageId":"alpine.LFD.1.00.0802111054080.2732@xanadu.home","threadId":"12025","inReplyTo":"alpine.LSU.1.00.0802111519300.3870@racer.site","subject":"Re: [PATCH] pack-objects: only throw away data during memory pressure","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-02-11T16:00:25Z","receivedAt":"2008-02-11T16:00:25Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 11 Feb 2008, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Mon, 11 Feb 2008, Martin Koegler wrote:\n> \n> > What about this not really tested patch for dealing with memory pressure \n> > in git-pack-objects?\n> > \n> > It will slow down the repack in the case of memory pressure, but missing \n> > memory will not affect the results.\n> \n> It almost helped:\n> \n> $ /usr/bin/time git repack -a -d -f --window=250 --depth=250\n> Counting objects: 2477715, done.\n> fatal: Out of memory, malloc failed411764)\n> Command exited with non-zero status 1\n> 10050.12user 240.63system 2:53:37elapsed 98%CPU (0avgtext+0avgdata \n> 0maxresident)k\n> 0inputs+0outputs (29555major+94032945minor)pagefaults 0swaps\n> \n> So, it ran longer until it ran out of memory.\n\nWhat it can do for you is to limit the window memory usage much more \nwithout affecting the end result, say to 128MB. Of course the repack is \nthen going to progress much slower if active purging of the window \nmemory is involved.\n\nIf you still run out of memory at that point then there is not much more \nto do besides using a new memory allocator that doesn't suffer as much \nfrom memory fragmentation, or find the possible memory leak that no one \nelse found so far.\n\n\nNicolas\n"},{"id":"68403","messageId":"alpine.LFD.1.00.0802110942310.2732@xanadu.home","threadId":"12025","inReplyTo":"120271478556-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"Re: [PATCH] pack-objects: only throw away data during memory pressure","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-02-11T16:00:33Z","receivedAt":"2008-02-11T16:00:33Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 11 Feb 2008, Martin Koegler wrote:\n\n> If pack-objects hit the memory limit, it deletes objects from the delta\n> window.\n> \n> This patch make it only delete the data, which is recomputed, if needed again.\n> \n> Signed-off-by: Martin Koegler \n\nLooks fine.\n\nAcked-by: Nicolas Pitre \n\n\nNicolas\n"},{"id":"68405","messageId":"alpine.LSU.1.00.0802111606440.3870@racer.site","threadId":"12025","inReplyTo":"alpine.LFD.1.00.0802111054080.2732@xanadu.home","subject":"Re: [PATCH] pack-objects: only throw away data during memory pressure","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-11T16:08:13Z","receivedAt":"2008-02-11T16:08:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 11 Feb 2008, Nicolas Pitre wrote:\n\n> On Mon, 11 Feb 2008, Johannes Schindelin wrote:\n> \n> > On Mon, 11 Feb 2008, Martin Koegler wrote:\n> > \n> > > What about this not really tested patch for dealing with memory \n> > > pressure in git-pack-objects?\n> > > \n> > > It will slow down the repack in the case of memory pressure, but \n> > > missing memory will not affect the results.\n> > \n> > It almost helped:\n> > \n> > $ /usr/bin/time git repack -a -d -f --window=250 --depth=250\n> > Counting objects: 2477715, done.\n> > fatal: Out of memory, malloc failed411764)\n> > Command exited with non-zero status 1\n> > 10050.12user 240.63system 2:53:37elapsed 98%CPU (0avgtext+0avgdata \n> > 0maxresident)k\n> > 0inputs+0outputs (29555major+94032945minor)pagefaults 0swaps\n> > \n> > So, it ran longer until it ran out of memory.\n> \n> What it can do for you is to limit the window memory usage much more \n> without affecting the end result, say to 128MB. Of course the repack is \n> then going to progress much slower if active purging of the window \n> memory is involved.\n\nOh, well. I did not think of setting a smaller windowMemory. Will try \nthat next.\n\n> If you still run out of memory at that point then there is not much more \n> to do besides using a new memory allocator that doesn't suffer as much \n> from memory fragmentation, or find the possible memory leak that no one \n> else found so far.\n\nCompletely forgot to say: I am running with ememoa (that is the Google \nallocator, right?).\n\nCiao,\nDscho\n"},{"id":"68496","messageId":"20080212082211.GE27535@lavos.net","threadId":"12025","inReplyTo":"alpine.LFD.1.00.0802110942310.2732@xanadu.home","subject":"Re: [PATCH] pack-objects: only throw away data during memory pressure","fromName":"Brian Downing","fromEmail":"bdowning@lavos.net","sentAt":"2008-02-12T08:22:12Z","receivedAt":"2008-02-12T08:22:12Z","isPatch":true,"sender":{"key":"bdowning@lavos.net","avatar":"https://avatars.githubusercontent.com/u/366426?v=4"},"body":"On Mon, Feb 11, 2008 at 11:00:33AM -0500, Nicolas Pitre wrote:\n> On Mon, 11 Feb 2008, Martin Koegler wrote:\n> > If pack-objects hit the memory limit, it deletes objects from the\n> > delta window.\n> > \n> > This patch make it only delete the data, which is recomputed, if\n> > needed again.\n> > \n> > Signed-off-by: Martin Koegler \n> \n> Looks fine.\n> \n> Acked-by: Nicolas Pitre \n\nUnfortunately this patch (if I understand what it's doing correctly)\nbasically defeats my intended use-case for which I wrote the memory\nlimiter. I have a repository with files of very mixed size. I want the\nwindow to be very large for small files, for good archival repacking,\nbut I don't want it to be very large for my 20+MB files with hundreds of\nrevisions, because I want it to finish someday.\n\nAlso, I've gotten into the habit of just doing:\n git repack --window=100000 --window-memory=256m\nfor archival repacks and just letting the memory limit automatically\nsize the window. Basically, I don't really want to specify a window\nsize, I just want it to use 512mb of RAM (and go at the speed that size\nof a window would entail.) While this is slow, it tends to be a\nrelatively constant speed, and it tends to find some very interesting\ndeltas in my trees that I wouldn't have otherwise expected.\n\nIf this patch is accepted, I'd really like a way to maintain the old\nbehavior as an option.\n\n-bcd\n"},{"id":"68520","messageId":"alpine.LFD.1.00.0802120910440.2732@xanadu.home","threadId":"12025","inReplyTo":"20080212082211.GE27535@lavos.net","subject":"Re: [PATCH] pack-objects: only throw away data during memory pressure","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-02-12T14:26:25Z","receivedAt":"2008-02-12T14:26:25Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 12 Feb 2008, Brian Downing wrote:\n\n> On Mon, Feb 11, 2008 at 11:00:33AM -0500, Nicolas Pitre wrote:\n> > On Mon, 11 Feb 2008, Martin Koegler wrote:\n> > > If pack-objects hit the memory limit, it deletes objects from the\n> > > delta window.\n> > > \n> > > This patch make it only delete the data, which is recomputed, if\n> > > needed again.\n> > > \n> > > Signed-off-by: Martin Koegler \n> > \n> > Looks fine.\n> > \n> > Acked-by: Nicolas Pitre \n> \n> Unfortunately this patch (if I understand what it's doing correctly)\n> basically defeats my intended use-case for which I wrote the memory\n> limiter. I have a repository with files of very mixed size. I want the\n> window to be very large for small files, for good archival repacking,\n> but I don't want it to be very large for my 20+MB files with hundreds of\n> revisions, because I want it to finish someday.\n> \n> Also, I've gotten into the habit of just doing:\n> git repack --window=100000 --window-memory=256m\n> for archival repacks and just letting the memory limit automatically\n> size the window. Basically, I don't really want to specify a window\n> size, I just want it to use 512mb of RAM (and go at the speed that size\n> of a window would entail.) While this is slow, it tends to be a\n> relatively constant speed, and it tends to find some very interesting\n> deltas in my trees that I wouldn't have otherwise expected.\n> \n> If this patch is accepted, I'd really like a way to maintain the old\n> behavior as an option.\n\nI think your use case has merits, but the previous behavior had \nsemantics problems. We always had constant window size with dynamic \nmemory usage, and now we have constant window size with bounded memory \nusage.\n\nIf what you want is really to have a dynamic window size using a \nconstant memory usage then it needs a different and coherent way to be \nspecified.\n\n\nNicolas\n"},{"id":"68532","messageId":"20080212171403.GG27535@lavos.net","threadId":"12025","inReplyTo":"alpine.LFD.1.00.0802120910440.2732@xanadu.home","subject":"Re: [PATCH] pack-objects: only throw away data during memory pressure","fromName":"Brian Downing","fromEmail":"bdowning@lavos.net","sentAt":"2008-02-12T17:14:03Z","receivedAt":"2008-02-12T17:14:03Z","isPatch":true,"sender":{"key":"bdowning@lavos.net","avatar":"https://avatars.githubusercontent.com/u/366426?v=4"},"body":"On Tue, Feb 12, 2008 at 09:26:25AM -0500, Nicolas Pitre wrote:\n> I think your use case has merits, but the previous behavior had \n> semantics problems. We always had constant window size with dynamic \n> memory usage, and now we have constant window size with bounded memory \n> usage.\n> \n> If what you want is really to have a dynamic window size using a \n> constant memory usage then it needs a different and coherent way to be \n> specified.\n\nSometimes I want a bounded window size with bounded memory usage; i.e. a\nmaximum of 50 entries OR 256 megs worth. That's for everyday repacking\nof my troublesome repository; without the window going down to less than\n10 or so for the large files, it still takes way too long, but doing the\nwhole thing at 10 makes for very poor packing.\n\nSo that gives four options:\n\n1. No memory limit, constant entry depth. Original Git behavior.\n2. The above with an additional memory-based depth limit. This is \n what was added with --memory-limit.\n3. Constant entry depth with a memory-usage limit. This is what the\n proposed patch does.\n4. Dynamic entry depth, with a memory-based limit. This is I believe\n what you are proposing above, and what I emulate by setting\n --window=$bignum --window-memory=x.\n\nI'm willing to try and make all of those work. (Though frankly I don't\ncare much about #3; setting the window entry size to something \"large\nenough\" seems a simple enough work-around for me, and it prevents what's\nprobably some truly ridiculous behavior if you have a gigantic number of\ntiny, say, tree objects. Having a cap on window depth stops that case\nfrom taking a truly inordinate amount of time.)\n\nHowever, I can't figure out what sensible command-line and/or config\nparameters would be for the cases above. Any ideas?\n\n-bcd\n"},{"id":"68533","messageId":"20080212171713.GH27535@lavos.net","threadId":"12025","inReplyTo":"20080212171403.GG27535@lavos.net","subject":"Re: [PATCH] pack-objects: only throw away data during memory pressure","fromName":"Brian Downing","fromEmail":"bdowning@lavos.net","sentAt":"2008-02-12T17:17:13Z","receivedAt":"2008-02-12T17:17:13Z","isPatch":true,"sender":{"key":"bdowning@lavos.net","avatar":"https://avatars.githubusercontent.com/u/366426?v=4"},"body":"On Tue, Feb 12, 2008 at 11:14:03AM -0600, Brian Downing wrote:\n> Sometimes I want a bounded window size with bounded memory usage; i.e. a\n> maximum of 50 entries OR 256 megs worth. That's for everyday repacking\n> of my troublesome repository; without the window going down to less than\n> 10 or so for the large files, it still takes way too long, but doing the\n> whole thing at 10 makes for very poor packing.\n\n(Yeah, the default depth is 10. I admit to shooting from the hip a bit\nin my description above, but I do really use that mode of operation, and\nwouldn't like to lose it.)\n\n-bcd\n"},{"id":"68535","messageId":"20080212172824.GI27535@lavos.net","threadId":"12025","inReplyTo":"20080212171403.GG27535@lavos.net","subject":"Re: [PATCH] pack-objects: only throw away data during memory pressure","fromName":"Brian Downing","fromEmail":"bdowning@lavos.net","sentAt":"2008-02-12T17:28:24Z","receivedAt":"2008-02-12T17:28:24Z","isPatch":true,"sender":{"key":"bdowning@lavos.net","avatar":"https://avatars.githubusercontent.com/u/366426?v=4"},"body":"On Tue, Feb 12, 2008 at 11:14:03AM -0600, Brian Downing wrote:\n> 3. Constant entry depth with a memory-usage limit. This is what the\n> proposed patch does.\n> 4. Dynamic entry depth, with a memory-based limit. This is I believe\n> what you are proposing above, and what I emulate by setting\n> --window=$bignum --window-memory=x.\n\n[...]\n\n> (Though frankly I don't > care much about #3; setting the window entry\n> size to something \"large enough\" seems a simple enough work-around for\n> me...)\n\nI meant #4 of course, I renumbered one case but not the other. I need\nto not post right after I wake up. :)\n\n-bcd\n"},{"id":"68536","messageId":"alpine.LFD.1.00.0802121229570.2732@xanadu.home","threadId":"12025","inReplyTo":"20080212171403.GG27535@lavos.net","subject":"Re: [PATCH] pack-objects: only throw away data during memory pressure","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-02-12T17:39:39Z","receivedAt":"2008-02-12T17:39:39Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 12 Feb 2008, Brian Downing wrote:\n\n> So that gives four options:\n> \n> 1. No memory limit, constant entry depth. Original Git behavior.\n> 2. The above with an additional memory-based depth limit. This is \n> what was added with --memory-limit.\n> 3. Constant entry depth with a memory-usage limit. This is what the\n> proposed patch does.\n\n... and it was merged already.\n\n> 4. Dynamic entry depth, with a memory-based limit. This is I believe\n> what you are proposing above, and what I emulate by setting\n> --window=$bignum --window-memory=x.\n> \n> I'm willing to try and make all of those work. (Though frankly I don't\n> care much about #3; setting the window entry size to something \"large\n> enough\" seems a simple enough work-around for me, and it prevents what's\n> probably some truly ridiculous behavior if you have a gigantic number of\n> tiny, say, tree objects. Having a cap on window depth stops that case\n> from taking a truly inordinate amount of time.)\n> \n> However, I can't figure out what sensible command-line and/or config\n> parameters would be for the cases above. Any ideas?\n\nI explicitly avoided suggesting something in my previous email exactly \nbecause I don't have a clear idea myself for a sensible parameter. ;-)\n\nMaybe using a negative window size could mean it is dynamic, but caped \nto the provided absolute value? Although a bit strange, this is still a \nspecialized mode of operation and having an odd argument for it \nshouldn't rebute those who understands the implication and are willing \nto use it. The more natural mode of operation with a memory cap is not \nto change the end result but maybe experience a slowdown (what the \nmerged patch does), which is why I considered that patch good.\n\n\nNicolas\n"},{"id":"68591","messageId":"alpine.LFD.1.00.0802122025490.2732@xanadu.home","threadId":"12025","inReplyTo":"alpine.LFD.1.00.0802110942310.2732@xanadu.home","subject":"Re: [PATCH] pack-objects: only throw away data during memory pressure","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-02-13T01:48:06Z","receivedAt":"2008-02-13T01:48:06Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 11 Feb 2008, Nicolas Pitre wrote:\n\n> On Mon, 11 Feb 2008, Martin Koegler wrote:\n> \n> > If pack-objects hit the memory limit, it deletes objects from the delta\n> > window.\n> > \n> > This patch make it only delete the data, which is recomputed, if needed again.\n> > \n> > Signed-off-by: Martin Koegler \n> \n> Looks fine.\n> \n> Acked-by: Nicolas Pitre \n\nWell, I take that back.\n\nSome testing on the OOO repository with this turns out to be \ncompletely unusable.\n\nBy the time this gets into action and data is actively thrown away, \nperformance simply goes down the drain due to the data constantly being\nreloaded over and over and over and over and over and over again, to the \npoint of virtually making no relative progress at all.\n\nSo this change is not actually helping anything. The previous behavior \nof enforcing the memory limit by dynamically shrinking the window size \nat least had the effect of allowing some kind of progress, even if the \nend result wouldn't be optimal.\n\nAnd that's the whole point behind this memory limiting feature: allowing \nsome progress to be made when resources are too limited to let the \nrepack go unbounded.\n\nTherefore I think commit 9c2174350cc0ae0f6bad126e15fe1f9f044117ab should \nbe reverted.\n\n\nNicolas\n"}]}