{"thread":{"id":"8255","subject":"[PATCH] git-pack-objects: cache small deltas between big objects","startedAt":"2007-05-20T21:11:04Z","lastAt":"2007-05-22T09:25:24Z","messageCount":8,"participants":["Martin Koegler","Dana How","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"42795","messageId":"11796954641778-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"8255","inReplyTo":null,"subject":"[PATCH] git-pack-objects: cache small deltas between big objects","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-05-20T21:11:04Z","receivedAt":"2007-05-20T21:11:04Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Creating deltas between big blobs is a CPU and memory intensive task.\nIn the writing phase, all (not reused) deltas are redone.\n\nThis patch adds support for caching deltas from the deltifing phase, so\nthat that the writing phase is faster.\n\nThe caching is limited to small deltas to avoid increasing memory usage very much.\nThe implemented limit is (memory needed to create the delta)/1024.\n\nSigned-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>\n---\n builtin-pack-objects.c |   35 +++++++++++++++++++++++++----------\n 1 files changed, 25 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex d165f10..13429d0 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -35,6 +35,7 @@ struct object_entry {\n \tstruct object_entry *delta_sibling; /* other deltified objects who\n \t\t\t\t\t     * uses the same base as me\n \t\t\t\t\t     */\n+\tvoid *delta_data;\t/* cached delta (uncompressed) */\n \tunsigned long delta_size;\t/* delta data size (uncompressed) */\n \tenum object_type type;\n \tenum object_type in_pack_type;\t/* could be delta */\n@@ -380,17 +381,24 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t\t */\n \n \tif (!to_reuse) {\n-\t\tbuf = read_sha1_file(entry->sha1, &type, &size);\n-\t\tif (!buf)\n-\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->sha1));\n-\t\tif (size != entry->size)\n-\t\t\tdie(\"object %s size inconsistency (%lu vs %lu)\",\n-\t\t\t    sha1_to_hex(entry->sha1), size, entry->size);\n-\t\tif (entry->delta) {\n-\t\t\tbuf = delta_against(buf, size, entry);\n+\t\tif (entry->delta_data) {\n+\t\t\tbuf = entry->delta_data;\n \t\t\tsize = entry->delta_size;\n \t\t\tobj_type = (allow_ofs_delta && entry->delta->offset) ?\n-\t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n+\t\t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n+\t\t} else {\n+\t\t\tbuf = read_sha1_file(entry->sha1, &type, &size);\n+\t\t\tif (!buf)\n+\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->sha1));\n+\t\t\tif (size != entry->size)\n+\t\t\t\tdie(\"object %s size inconsistency (%lu vs %lu)\",\n+\t\t\t\t    sha1_to_hex(entry->sha1), size, entry->size);\n+\t\t\tif (entry->delta) {\n+\t\t\t\tbuf = delta_against(buf, size, entry);\n+\t\t\t\tsize = entry->delta_size;\n+\t\t\t\tobj_type = (allow_ofs_delta && entry->delta->offset) ?\n+\t\t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n+\t\t\t}\n \t\t}\n \t\t/*\n \t\t * The object header is a byte of 'type' followed by zero or\n@@ -1294,10 +1302,17 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \tif (!delta_buf)\n \t\treturn 0;\n \n+\tif (trg_entry->delta_data)\n+\t\tfree (trg_entry->delta_data);\n+\ttrg_entry->delta_data = 0;\n \ttrg_entry->delta = src_entry;\n \ttrg_entry->delta_size = delta_size;\n \ttrg_entry->depth = src_entry->depth + 1;\n-\tfree(delta_buf);\n+\t/* cache delta, if objects are large enough compared to delta size */\n+\tif ((src_size >> 20) + (trg_size >> 21) > (delta_size >> 10))\n+\t\ttrg_entry->delta_data = delta_buf;\n+\telse\n+\t\tfree(delta_buf);\n \treturn 1;\n }\n \n-- \n1.5.2.rc3.802.g4b4b7\n"},{"id":"42838","messageId":"56b7f5510705202135s8c9cd9qf4489b2b5bb2e264@mail.gmail.com","threadId":"8255","inReplyTo":"11796954641778-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"Re: [PATCH] git-pack-objects: cache small deltas between big objects","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-21T04:35:56Z","receivedAt":"2007-05-21T04:35:56Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/20/07, Martin Koegler <mkoegler@auto.tuwien.ac.at> wrote:\n> Creating deltas between big blobs is a CPU and memory intensive task.\n> In the writing phase, all (not reused) deltas are redone.\n\nActually,  just the ones selected,  which is approx 1/window.\nDo you have any numbers describing the effects on runtime\nand memory size for a known repo like linux-2.6?\n\n> This patch adds support for caching deltas from the deltifing phase, so\n> that that the writing phase is faster.\n>\n> The caching is limited to small deltas to avoid increasing memory usage very much.\n> The implemented limit is (memory needed to create the delta)/1024.\n\nYour limit is applied per-object,  and there is no overall limit\non the amount of memory not freed in the delta phase.\nI suspect this caching would be disastrous for the large repo\nwith \"megablobs\" I'm trying to wrestle with at the moment.\n\n> @@ -1294,10 +1302,17 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n>         trg_entry->delta = src_entry;\n>         trg_entry->delta_size = delta_size;\n>         trg_entry->depth = src_entry->depth + 1;\n> -       free(delta_buf);\n> +       /* cache delta, if objects are large enough compared to delta size */\n> +       if ((src_size >> 20) + (trg_size >> 21) > (delta_size >> 10))\n> +               trg_entry->delta_data = delta_buf;\n> +       else\n> +               free(delta_buf);\n>         return 1;\n>  }\n\n-- \nDana L. How  danahow@gmail.com  +1 650 804 5991 cell\n"},{"id":"42839","messageId":"7v646mixnm.fsf@assigned-by-dhcp.cox.net","threadId":"8255","inReplyTo":"11796954641778-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"Re: [PATCH] git-pack-objects: cache small deltas between big objects","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-21T04:54:53Z","receivedAt":"2007-05-21T04:54:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Koegler <mkoegler@auto.tuwien.ac.at> writes:\n\n> Creating deltas between big blobs is a CPU and memory intensive task.\n> In the writing phase, all (not reused) deltas are redone.\n>\n> This patch adds support for caching deltas from the deltifing phase, so\n> that that the writing phase is faster.\n>\n> The caching is limited to small deltas to avoid increasing memory usage very much.\n> The implemented limit is (memory needed to create the delta)/1024.\n>\n> Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>\n> ---\n>  builtin-pack-objects.c |   35 +++++++++++++++++++++++++----------\n>  1 files changed, 25 insertions(+), 10 deletions(-)\n\nThis is an interesting idea.\n\n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index d165f10..13429d0 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> ...\n> @@ -1294,10 +1302,17 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n>  \tif (!delta_buf)\n>  \t\treturn 0;\n>  \n> +\tif (trg_entry->delta_data)\n> +\t\tfree (trg_entry->delta_data);\n> +\ttrg_entry->delta_data = 0;\n>  \ttrg_entry->delta = src_entry;\n>  \ttrg_entry->delta_size = delta_size;\n>  \ttrg_entry->depth = src_entry->depth + 1;\n> -\tfree(delta_buf);\n> +\t/* cache delta, if objects are large enough compared to delta size */\n> +\tif ((src_size >> 20) + (trg_size >> 21) > (delta_size >> 10))\n> +\t\ttrg_entry->delta_data = delta_buf;\n> +\telse\n> +\t\tfree(delta_buf);\n>  \treturn 1;\n>  }\n\nCare to justify this arithmetic?  Why isn't it for example like\nthis?\n\n\t((src_size + trg_size) >> 10) > delta_size\n\nI am puzzled by the shifts on both ends, and differences between\n20 and 21.\n"},{"id":"42904","messageId":"20070521170043.GA27063@auto.tuwien.ac.at","threadId":"8255","inReplyTo":"7v646mixnm.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-pack-objects: cache small deltas between big objects","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-05-21T17:00:43Z","receivedAt":"2007-05-21T17:00:43Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"On Sun, May 20, 2007 at 09:54:53PM -0700, Junio C Hamano wrote:\n> Martin Koegler <mkoegler@auto.tuwien.ac.at> writes:\n> > diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> > index d165f10..13429d0 100644\n> > --- a/builtin-pack-objects.c\n> > +++ b/builtin-pack-objects.c\n> > ...\n> > @@ -1294,10 +1302,17 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n> >  \tif (!delta_buf)\n> >  \t\treturn 0;\n> >  \n> > +\tif (trg_entry->delta_data)\n> > +\t\tfree (trg_entry->delta_data);\n> > +\ttrg_entry->delta_data = 0;\n> >  \ttrg_entry->delta = src_entry;\n> >  \ttrg_entry->delta_size = delta_size;\n> >  \ttrg_entry->depth = src_entry->depth + 1;\n> > -\tfree(delta_buf);\n> > +\t/* cache delta, if objects are large enough compared to delta size */\n> > +\tif ((src_size >> 20) + (trg_size >> 21) > (delta_size >> 10))\n> > +\t\ttrg_entry->delta_data = delta_buf;\n> > +\telse\n> > +\t\tfree(delta_buf);\n> >  \treturn 1;\n> >  }\n> \n> Care to justify this arithmetic?  Why isn't it for example like\n> this?\n> \n> \t((src_size + trg_size) >> 10) > delta_size\n\nI wanted to avoid a possible overflow in (src_size + trg_size), so\nI shift both sides.\n\n> I am puzzled by the shifts on both ends, and differences between\n> 20 and 21.\n\nI base the maximum allowed delta_size for caching on the required\nmemory for creating the delta. For the src entry, you need need a\ndelta index, which has (about) the same size of the src entry. So I\ncount the src entry double.\n\nI divide the requried memory by 1024, so that the delta size is some\nmagnitudes smaller and will not cause a big increase of memory usage,\neg:\n\nFor two 100 MB (uncompressed) blobs, we need 300MB of memory to do the\ndelta (with the default window size of 10 up to 1900MB for all delta\nindexes in the worst case). The patch will limit the delta size for\nthe target blob to 150kB.\n\nThe caching policy does only cache really small deltas for really big\nobjects, as I wanted to avoid out of memory situations. Futurer patch\nshould probably replace it with a better strategy.\n\nmfg Martin Kögler\n"},{"id":"42907","messageId":"20070521175950.GA13818@auto.tuwien.ac.at","threadId":"8255","inReplyTo":"56b7f5510705202135s8c9cd9qf4489b2b5bb2e264@mail.gmail.com","subject":"Re: [PATCH] git-pack-objects: cache small deltas between big objects","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-05-21T17:59:50Z","receivedAt":"2007-05-21T17:59:50Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"On Sun, May 20, 2007 at 09:35:56PM -0700, Dana How wrote:\n> On 5/20/07, Martin Koegler <mkoegler@auto.tuwien.ac.at> wrote:\n> >Creating deltas between big blobs is a CPU and memory intensive task.\n> >In the writing phase, all (not reused) deltas are redone.\n> \n> Actually,  just the ones selected,  which is approx 1/window.\n> Do you have any numbers describing the effects on runtime\n> and memory size for a known repo like linux-2.6?\n\nObjects below 1 MB are not considered for caching. \nThe linux kernel has only such objects:\nlinux.git$ find -size +1000k |grep -v \".git\"|wc\n      0       0       0\nSo no caching happens. The required memory is only increased by the\nnew pointer in object_entry. \n\nAt runtime, we have additional (#object)*(window size+1) null pointer\nchecks, (#objects)*(window size) pointer initialiations with zero and\ncheck (#objects)*(window size) times the caching policy check: ((src_size\n>> 20) + (trg_size >> 21) > (delta_size >> 10))\n\nWriting a cached delta is faster, as we avoid creating a delta. Some\ncalls to free are delayed.\n\n> >This patch adds support for caching deltas from the deltifing phase, so\n> >that that the writing phase is faster.\n> >\n> >The caching is limited to small deltas to avoid increasing memory usage \n> >very much.\n> >The implemented limit is (memory needed to create the delta)/1024.\n> \n> Your limit is applied per-object,  and there is no overall limit\n> on the amount of memory not freed in the delta phase.\n> I suspect this caching would be disastrous for the large repo\n> with \"megablobs\" I'm trying to wrestle with at the moment.\n\nhttp://www.spinics.net/lists/git/msg31241.html:\n> At the moment I'm experimenting on a git repository with\n> a 4.5GB checkout,  and 18 months of history in 4K commits\n> comprising 100GB (uncompressed) of blobs stored in\n> 7 packfiles of 2GB or less. Hopefully I'll be able to say\n> more about tweaking packing shortly.\n\nI you have 100 GB of uncompressed data in your pack files, the cache\nlimit is between 100MB and 200MB with the current policy.\n\nThe aim of my patch is to speed up pack writing without increasing\nmemory usage very much, if you have blobs of some hundred MB size in\nyour repository.\n\nThe caching policy could be extended to speed more memory on caching\nother deltas. Ideas on this topic are welcome.\n\nmfg Martin Kögler\nPS: If you are trying to optimize packing speed/size, you could test\nthe following patch: http://marc.info/?l=git&m=117908942525171&w=2\n"},{"id":"42957","messageId":"56b7f5510705220001q78b42d08kffd95d25c0af478e@mail.gmail.com","threadId":"8255","inReplyTo":"20070521175950.GA13818@auto.tuwien.ac.at","subject":"Re: [PATCH] git-pack-objects: cache small deltas between big objects","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-22T07:01:10Z","receivedAt":"2007-05-22T07:01:10Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/21/07, Martin Koegler <mkoegler@auto.tuwien.ac.at> wrote:\n> On Sun, May 20, 2007 at 09:35:56PM -0700, Dana How wrote:\n> > On 5/20/07, Martin Koegler <mkoegler@auto.tuwien.ac.at> wrote:\n> > > This patch adds support for caching deltas from the deltifing phase, so\n> > > that that the writing phase is faster.\n> > >\n> > > The caching is limited to small deltas to avoid increasing memory usage\n> > > very much.\n> > > The implemented limit is (memory needed to create the delta)/1024.\n> >\n> > Your limit is applied per-object,  and there is no overall limit\n> > on the amount of memory not freed in the delta phase.\n> > I suspect this caching would be disastrous for the large repo\n> > with \"megablobs\" I'm trying to wrestle with at the moment.\n>\n> http://www.spinics.net/lists/git/msg31241.html:\n> > At the moment I'm experimenting on a git repository with\n> > a 4.5GB checkout,  and 18 months of history in 4K commits\n> > comprising 100GB (uncompressed) of blobs stored in\n> > 7 packfiles of 2GB or less. Hopefully I'll be able to say\n> > more about tweaking packing shortly.\n>\n> I you have 100 GB of uncompressed data in your pack files, the cache\n> limit is between 100MB and 200MB with the current policy.\nYes,  there is an implicit limit in your patch, and it would be\nsufficient in my case.  It's still the case that there is no absolute\nlimit,  but perhaps you have to do something truly insane\nfor that to matter.\n\n> The aim of my patch is to speed up pack writing without increasing\n> memory usage very much, if you have blobs of some hundred MB size in\n> your repository.\n>\n> The caching policy could be extended to speed more memory on caching\n> other deltas. Ideas on this topic are welcome.\nThere _is_ something useful in your patch.\nUnfortunately I don't think it helps my problem that much.\n\n> PS: If you are trying to optimize packing speed/size, you could test\n> the following patch: http://marc.info/?l=git&m=117908942525171&w=2\nI remember this post -- I hope you continue to refine it.\n\nWhat I've concluded is that there are cases where the packfile\ntreatment is just not appropriate for some part of the data.\n[NOTE: I'm talking about disk storage here, not packs for communications.]\nWith the \"delta\" attribute Junio proposed,  and the \"repack\"\nattribute I proposed in response,  we were starting to move in that\ndirection already.\n\nThe order of objects in the packfile(s) in my test repo after repacking\nseems to be commit+ [ tree+ blob+ ]+, in other words,  the commits\nare all at the beginning and the new tree blobs are interspersed amongst\nthe data blobs (this was imported with only straightline history, no branching).\nIf some of these blobs are enormous,  the tree blobs which are accessed\nall the time get pushed apart.  This seemed to really hurt performance.\n\nIf I simply refuse to insert enormous blobs in the packfiles,  and keep\nthem loose,  the performance is better.  More importantly,  my packfiles\nare now sized like everyone else's, so I'm in an operating regime which\neveryone is testing and optimizing.  This was not true with 12GB+ of packfiles.\nOf course, loose objects are slower, but slight extra overhead to access\nsomething large enough to be noticeable already doesn't bother me.\n\nFinally, loose objects don't get deltified.  This is a problem,  but I would\nneed to repack at least every week,  and nonzero window/depth would\nbe prohibitive with large objects included.  So if I put the large objects\nin the packs,  not only are the large objects still undeltified, but everything\nelse is undeltified as well.  Note also that Perforce, what we're currently\nusing, doesn't deltify large objects either,  so people here who migrate\nto git aren't going to lose anything, but they will gain compression\non the remaining \"normal\" objects (Perforce uses deltification or compression,\nbut not both).\n\nSo at the moment I'm finding keeping enormous objects loose\nto be a reasonable compromise which keeps my packfiles\n\"normal\" and imposes overheads only on objects whose size\nalready imposes an even larger overhead.\n\nThanks,\n-- \nDana L. How  danahow@gmail.com  +1 650 804 5991 cell\n"},{"id":"42960","messageId":"7vhcq58et1.fsf@assigned-by-dhcp.cox.net","threadId":"8255","inReplyTo":"56b7f5510705220001q78b42d08kffd95d25c0af478e@mail.gmail.com","subject":"Re: [PATCH] git-pack-objects: cache small deltas between big objects","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-22T08:04:26Z","receivedAt":"2007-05-22T08:04:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Dana How\" <danahow@gmail.com> writes:\n\n> If I simply refuse to insert enormous blobs in the packfiles,  and keep\n> them loose,  the performance is better.  More importantly,  my packfiles\n> are now sized like everyone else's, so I'm in an operating regime which\n> everyone is testing and optimizing.  This was not true with 12GB+ of packfiles.\n> Of course, loose objects are slower, but slight extra overhead to access\n> something large enough to be noticeable already doesn't bother me.\n>\n> Finally, loose objects don't get deltified.  This is a problem,  but I would\n> need to repack at least every week,  and nonzero window/depth would\n> be prohibitive with large objects included.\n\nHere are a few quick comments before going to bed.\n\n * The objects in the packfile are ordered in \"recency\" order,\n   as \"rev-list --objects\" feeds you, so it is correct that we\n   get trees and blobs mixed.  It might be an interesting\n   experiment, especially with a repository without huge blobs,\n   to see how much improvement we might get if we keep the\n   recency order _but_ emit tags, commits, trees, and then\n   blobs, in this order.  In write_pack_file() we have a single\n   loop to call write_one(), but we could make it a nested loop\n   that writes only objects of each type.\n\n * Also my earlier \"nodelta\" attribute thing would be worth\n   trying with your repository with huge blobs, with the above\n   \"group by object type\" with further tweak to write blobs\n   without \"nodelta\" marker first and then finally blobs with\n   \"nodelta\" marker.\n\nI suspect the above two should help \"git log\" and \"git log --\npathspec...\"  performance, as these two do not look at blobs at\nall (pathspec limiting does invoke diff machinery, but that is\nonly at the tree level).\n\nThe \"I want to have packs with reasonable size as everybody\nelse\" (which I think is a reasonable thing to want, but does not\nhave much technical meaning as other issues do) wish is\nsomething we cannot _measure_ to judge pros and cons, but with\nthe above experiment, you could come up with three set of packs\nsuch that, all three sets use \"nodelta\" to leave the huge blobs\nundeltified, and use the default window and depth for others,\nand:\n\n (1) One set has trees and blobs mixed;\n\n (2) Another set has trees and blobs grouped, but \"nodelta\" blobs\n     and others are not separated;\n\n (3) The third set has trees and blobs grouped, and \"nodelta\"\n     blobs and others are separated.\n\nComparing (1) and (2) would show how bad it is to have huge\nblobs in between trees (which are presumably accessed more\noften).  I suspect that comparing (2) and (3) would show that\nfor most workloads, the split is not worth it.\n\nAnd compare (3) with another case where you leave \"nodelta\"\nblobs loose.  That's the true comparison that would demonstrate\nwhy placing huge blobs in packs is bad and they should be left\nloose.  I'm skeptical if there will be significant differences,\nthough.\n"},{"id":"42963","messageId":"56b7f5510705220225m754a7e8fmb90a0cc0200060d3@mail.gmail.com","threadId":"8255","inReplyTo":"7vhcq58et1.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-pack-objects: cache small deltas between big objects","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-22T09:25:24Z","receivedAt":"2007-05-22T09:25:24Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/22/07, Junio C Hamano <junkio@cox.net> wrote:\n> \"Dana How\" <danahow@gmail.com> writes:\n> > If I simply refuse to insert enormous blobs in the packfiles,  and keep\n> > them loose,  the performance is better.  More importantly,  my packfiles\n> > are now sized like everyone else's, so I'm in an operating regime which\n> > everyone is testing and optimizing.  This was not true with 12GB+ of packfiles.\n> > Of course, loose objects are slower, but slight extra overhead to access\n> > something large enough to be noticeable already doesn't bother me.\n> >\n> > Finally, loose objects don't get deltified.  This is a problem,  but I would\n> > need to repack at least every week,  and nonzero window/depth would\n> > be prohibitive with large objects included.\n>\n> Here are a few quick comments before going to bed.\n>\n>  * The objects in the packfile are ordered in \"recency\" order,\n>    as \"rev-list --objects\" feeds you, so it is correct that we\n>    get trees and blobs mixed.  It might be an interesting\n>    experiment, especially with a repository without huge blobs,\n>    to see how much improvement we might get if we keep the\n>    recency order _but_ emit tags, commits, trees, and then\n>    blobs, in this order.  In write_pack_file() we have a single\n>    loop to call write_one(), but we could make it a nested loop\n>    that writes only objects of each type.\nAlready tried that, almost.  Added a --types=[ctgb]+ flag to\npack-objects, and changed to git-repack to run in 2 passes\nwhen -a && --max-pack-size.  The first pass would create\npackfiles with all Commits/Trees/taGs [of course just 1],\nthe second made packfiles with just Blobs.  With a warm cache,\nthis was 3X to 7X slower than --max-blob-size= approach\n(for the git-log --pretty=oneline example).  Why?  I'm guessing\nbecause each lookup had to go through 7 index files instead of 1,\nwhich would be significant when processing very small blobs\n(commits and trees).  And the _slower_ one had window/depth=0/0,\nso it had no delta expansion to do.\n\n>  * Also my earlier \"nodelta\" attribute thing would be worth\n>    trying with your repository with huge blobs, with the above\n>    \"group by object type\" with further tweak to write blobs\n>    without \"nodelta\" marker first and then finally blobs with\n>    \"nodelta\" marker.\nI started out enthusiastic about \"nodelta\",  causing me to\nquickly propose \"norepack\" as well.  However, there is no\nsimple way in my repository to specify these.  Most of the\nenormous files have certain suffixes,  but each of these\nappears on a continuum of file sizes,  so I can't write\nany *.sfx rules in .gitattributes.  I could make rules\nspecific to specific files,  but then I would have to write\nscripts to auto-generate them.  (At commit time?)\n\nAssuming I *could* get \"nodelta\" properly specified,\nputting these last would help somewhat.  But we would\nstill be left with the problem caused by extra index files\n(resulting from 2GB packfile limit).\n\n> I suspect the above two should help \"git log\" and \"git log --\n> pathspec...\"  performance, as these two do not look at blobs at\n> all (pathspec limiting does invoke diff machinery, but that is\n> only at the tree level).\n>\n> The \"I want to have packs with reasonable size as everybody\n> else\" (which I think is a reasonable thing to want, but does not\n> have much technical meaning as other issues do) wish is\n> something we cannot _measure_ to judge pros and cons, ...\n?? A depressingly large portion of my career has been spent\nfooling optimization programs to work on new problems,  by\nmaking the new problem look just like what they're used to.  So\nwanting a program's input to look \"conventional\", or similar\nto something in a regression,  seems pretty reasonable.\nIt's just the data-side version of preferring small changes in an algorithm.\n\n> ... but with\n> the above experiment, you could come up with three set of packs\n> such that, all three sets use \"nodelta\" to leave the huge blobs\n> undeltified, and use the default window and depth for others,\n> and:\n>\n>  (1) One set has trees and blobs mixed;\n>\n>  (2) Another set has trees and blobs grouped, but \"nodelta\" blobs\n>      and others are not separated;\n>\n>  (3) The third set has trees and blobs grouped, and \"nodelta\"\n>      blobs and others are separated.\n>\n> Comparing (1) and (2) would show how bad it is to have huge\n> blobs in between trees (which are presumably accessed more\n> often).  I suspect that comparing (2) and (3) would show that\n> for most workloads, the split is not worth it.\n>\n> And compare (3) with another case where you leave \"nodelta\"\n> blobs loose.  That's the true comparison that would demonstrate\n> why placing huge blobs in packs is bad and they should be left\n> loose.  I'm skeptical if there will be significant differences,\n> though.\nI think the difference will come from at least the different number of\nindex files,\nas pointed out above.  I can certainly start on these comparisons.\n\nI must say,  getting the whole repository to repack -a under an hour\nwith great git-log performance after just a 55-line change was a much better\nexperience than the 10X larger max-pack-size patch...\n\nBTW,  why the attachment to keeping *everything* in a packfile?\nIf I implement the changes above,  they will be more extensive than\nmax-blob-size (even max-pack-size only added *1* new nested loop\nto pack-objects),  and they'll be climbing uphill due to the packfiles\nbeing THREE orders of magnitude larger and the index files one\norder of magnitude more numerous.\n\nThanks,\n-- \nDana L. How  danahow@gmail.com  +1 650 804 5991 cell\n"}]}