{"thread":{"id":"7942","subject":"[RFD/PATCH] Implement pack.compression and pack-objects --compression=N","startedAt":"2007-05-02T03:18:05Z","lastAt":"2007-05-04T20:17:35Z","messageCount":11,"participants":["Dana How","Junio C Hamano","Nicolas Pitre"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"40877","messageId":"463802ED.1080200@gmail.com","threadId":"7942","inReplyTo":null,"subject":"[RFD/PATCH] Implement pack.compression and pack-objects --compression=N","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-02T03:18:05Z","receivedAt":"2007-05-02T03:18:05Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"\nGit's object store and packing are optimized for\n* Expensive repo to repo bandwidth; and\n* Small-ish files\nThis justifies extensive use of compression.\n\nIn a multi-developer *office* with inter-repository\ntransfers occurring over a 100Mb+ LAN, there is less\nreason to compress files and slow down response times.\nResponse times suffer even more when large files are involved.\nHowever, *off-line* pack compression may still be\ndesirable to reduce storage space.\n\nConsequently, for such a usage pattern it is useful\nto specify different compression levels for loose\nobjects and packs. This patch implements a config\nvariable pack.compression in addition to the existing\ncore.compression, meant to be used for repacking.\nIt also adds --compression=N to pack-objects,\nmeant for push/pull/fetch, if different, or if different\non a per-repository basis.\n\n** THIS PATCH IS UNTESTED AND MEANT FOR DISCUSSION. **\ngit-repack.sh might also need to be modified,\nand how to pass --compression=N during push/pull/fetch\nhas not been investigated.\n\nThis applies on top of the git-repack --max-pack-size patchset.\n\nSigned-off-by: Dana L. How \n---\n builtin-pack-objects.c | 56 ++++++++++++++++++++++++++++++++++++------------\n 1 files changed, 42 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 69fec34..b663c15 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -70,6 +70,7 @@ static uint32_t pack_size_limit;\n static int pack_to_stdout;\n static int num_preferred_base;\n static struct progress progress_state;\n+static int pack_compression_level, pack_compression_seen;\n \n /*\n * The object names in objects array are hashed with this hashtable,\n@@ -414,6 +415,16 @@ static unsigned long write_object(struct sha1file *f,\n \t/* write limit if limited packsize and not first object */\n \tunsigned long limit = pack_size_limit && nr_written ?\n \t\t\t\tpack_size_limit - write_offset : 0;\n+\t\t\t\t/* no if no delta */\n+\tint usable_delta =\t!entry->delta ? 0 :\n+\t\t\t\t/* yes if unlimited packfile */\n+\t\t\t\t!pack_size_limit ? 1 :\n+\t\t\t\t/* no if base written to previous pack */\n+\t\t\t\tentry->delta->offset == (off_t)-1 ? 0 :\n+\t\t\t\t/* otherwise double-check written to this\n+\t\t\t\t * pack, like we do below\n+\t\t\t\t */\n+\t\t\t\tentry->delta->offset ? 1 : 0;\n \n \tif (!pack_to_stdout)\n \t\tcrc32_begin(f);\n@@ -423,8 +434,7 @@ static unsigned long write_object(struct sha1file *f,\n \t\tto_reuse = 0;\t/* can't reuse what we don't have */\n \telse if (obj_type == OBJ_REF_DELTA || obj_type == OBJ_OFS_DELTA)\n \t\t\t\t/* check_object() decided it for us ... */\n-\t\tto_reuse = !pack_size_limit ||\n-\t\t\t(entry->delta->offset && entry->delta->offset != (off_t)-1);\n+\t\tto_reuse = usable_delta;\n \t\t\t\t/* ... but pack split may override that */\n \telse if (obj_type != entry->in_pack_type)\n \t\tto_reuse = 0;\t/* pack has delta which is unusable */\n@@ -435,6 +445,10 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t\t * and we do not need to deltify it.\n \t\t\t\t */\n \n+\t/* differing core & pack compression when loose object -> must recompress */\n+\tif (!entry->in_pack && pack_compression_level != zlib_compression_level)\n+\t\tto_reuse = 0;\n+\telse\n \tif (!entry->in_pack && !entry->delta) {\n \t\tunsigned char *map;\n \t\tunsigned long mapsize;\n@@ -462,16 +476,6 @@ static unsigned long write_object(struct sha1file *f,\n \t\tz_stream stream;\n \t\tunsigned long maxsize;\n \t\tvoid *out;\n-\t\t\t\t\t/* no if no delta */\n-\t\tint usable_delta =\t!entry->delta ? 0 :\n-\t\t\t\t\t/* yes if unlimited packfile */\n-\t\t\t\t\t!pack_size_limit ? 1 :\n-\t\t\t\t\t/* no if base written to previous pack */\n-\t\t\t\t\tentry->delta->offset == (off_t)-1 ? 0 :\n-\t\t\t\t\t/* otherwise double-check written to this\n-\t\t\t\t\t * pack, like we do below\n-\t\t\t\t\t */\n-\t\t\t\t\tentry->delta->offset ? 1 : 0;\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@@ -493,7 +497,7 @@ static unsigned long write_object(struct sha1file *f,\n \t\t}\n \t\t/* compress the data to store and put compressed length in datalen */\n \t\tmemset(&stream, 0, sizeof(stream));\n-\t\tdeflateInit(&stream, zlib_compression_level);\n+\t\tdeflateInit(&stream, pack_compression_level);\n \t\tmaxsize = deflateBound(&stream, size);\n \t\tout = xmalloc(maxsize);\n \t\t/* Compress it */\n@@ -606,7 +610,7 @@ static unsigned long write_object(struct sha1file *f,\n \t\tunuse_pack(&w_curs);\n \t\treused++;\n \t}\n-\tif (entry->delta)\n+\tif (usable_delta)\n \t\twritten_delta++;\n \twritten++;\n \tif (!pack_to_stdout)\n@@ -1622,6 +1626,16 @@ static int git_pack_config(const char *k, const char *v)\n \t\twindow = git_config_int(k, v);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(k, \"pack.compression\")) {\n+\t\tint level = git_config_int(k, v);\n+\t\tif (level == -1)\n+\t\t\tlevel = Z_DEFAULT_COMPRESSION;\n+\t\telse if (level < 0 || level > Z_BEST_COMPRESSION)\n+\t\t\tdie(\"bad pack compression level %d\", level);\n+\t\tpack_compression_level = level;\n+\t\tpack_compression_seen = 1;\n+\t\treturn 0;\n+\t}\n \treturn git_default_config(k, v);\n }\n \n@@ -1732,6 +1746,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \trp_ac = 2;\n \n \tgit_config(git_pack_config);\n+\tif (!pack_compression_seen)\n+\t\tpack_compression_level = zlib_compression_level;\n \n \tprogress = isatty(2);\n \tfor (i = 1; i < argc; i++) {\n@@ -1759,6 +1775,18 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\t\tusage(pack_usage);\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!prefixcmp(arg, \"--compression=\")) {\n+\t\t\tchar *end;\n+\t\t\tint level = strtoul(arg+14, &end, 0);\n+\t\t\tif (!arg[14] || *end)\n+\t\t\t\tusage(pack_usage);\n+\t\t\tif (level == -1)\n+\t\t\t\tlevel = Z_DEFAULT_COMPRESSION;\n+\t\t\telse if (level < 0 || level > Z_BEST_COMPRESSION)\n+\t\t\t\tdie(\"bad pack compression level %d\", level);\n+\t\t\tpack_compression_level = level;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!prefixcmp(arg, \"--window=\")) {\n \t\t\tchar *end;\n \t\t\twindow = strtoul(arg+9, &end, 0);\n-- \n1.5.2.rc0.787.g0014\n"},{"id":"40917","messageId":"7vk5vr3w8n.fsf@assigned-by-dhcp.cox.net","threadId":"7942","inReplyTo":"463802ED.1080200@gmail.com","subject":"Re: [RFD/PATCH] Implement pack.compression and pack-objects --compression=N","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-02T18:40:08Z","receivedAt":"2007-05-02T18:40:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dana How writes:\n\n> Consequently, for such a usage pattern it is useful\n> to specify different compression levels for loose\n> objects and packs. This patch implements a config\n> variable pack.compression in addition to the existing\n> core.compression, meant to be used for repacking.\n> It also adds --compression=N to pack-objects,\n> meant for push/pull/fetch, if different, or if different\n> on a per-repository basis.\n>\n> ** THIS PATCH IS UNTESTED AND MEANT FOR DISCUSSION. **\n\nI think we tweaked this area in the past, but I do not think\nthe current setting was determined to be the best tradeoff for\nall workloads. To be able to discuss the patch, I think it\nneeds to come with benchmark numbers using publicly available\nrepositories as guinea pigs and set of typical git operations,\nso people can reproduce and compare notes.\n"},{"id":"40919","messageId":"56b7f5510705021155k40b576d6ke286e65494fc8b05@mail.gmail.com","threadId":"7942","inReplyTo":"7vk5vr3w8n.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFD/PATCH] Implement pack.compression and pack-objects --compression=N","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-02T18:55:15Z","receivedAt":"2007-05-02T18:55:15Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/2/07, Junio C Hamano wrote:\n> Dana How writes:\n> > Consequently, for such a usage pattern it is useful\n> > to specify different compression levels for loose\n> > objects and packs. This patch implements a config\n> > variable pack.compression in addition to the existing\n> > core.compression, meant to be used for repacking.\n> > It also adds --compression=N to pack-objects,\n> > meant for push/pull/fetch, if different, or if different\n> > on a per-repository basis.\n> >\n> > ** THIS PATCH IS UNTESTED AND MEANT FOR DISCUSSION. **\n>\n> I think we tweaked this area in the past, but I do not think\n> the current setting was determined to be the best tradeoff for\n> all workloads. To be able to discuss the patch, I think it\n> needs to come with benchmark numbers using publicly available\n> repositories as guinea pigs and set of typical git operations,\n> so people can reproduce and compare notes.\n\nOK, but this patch doesn't mandate any particular setting.\n\nIts motivation in my work environment is for pack.compression\nto be what core.compression currently is, and to set\ncore.compression to 0 to speed up large commits\n(the resulting space-inefficient loose objects will be scrubbed away\n by a later off-line repack).\nThus, my config settings (almost) change the gzip's behind a git-add to cp's.\nDo you want me to submit timings for a git-add/git-commit -a\non a typical 50-file commit I would be interested in,\nwith the (new) settings that I would use?\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"40925","messageId":"56b7f5510705021551q2d48f1e5i3f4a5c2f9891368a@mail.gmail.com","threadId":"7942","inReplyTo":"56b7f5510705021155k40b576d6ke286e65494fc8b05@mail.gmail.com","subject":"Re: [RFD/PATCH] Implement pack.compression and pack-objects --compression=N","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-02T22:51:25Z","receivedAt":"2007-05-02T22:51:25Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/2/07, Dana How wrote:\n> On 5/2/07, Junio C Hamano wrote:\n> > Dana How writes:\n> > > Consequently, for such a usage pattern it is useful\n> > > to specify different compression levels for loose\n> > > objects and packs. This patch implements a config\n> > > variable pack.compression in addition to the existing\n> > > core.compression, meant to be used for repacking.\n> > > It also adds --compression=N to pack-objects,\n> > > meant for push/pull/fetch, if different, or if different\n> > > on a per-repository basis.\n> > >\n> > > ** THIS PATCH IS UNTESTED AND MEANT FOR DISCUSSION. **\n> >\n> > I think we tweaked this area in the past, but I do not think\n> > the current setting was determined to be the best tradeoff for\n> > all workloads. To be able to discuss the patch, I think it\n> > needs to come with benchmark numbers using publicly available\n> > repositories as guinea pigs and set of typical git operations,\n> > so people can reproduce and compare notes.\n>\n> OK, but this patch doesn't mandate any particular setting.\n>\n> Its motivation in my work environment is for pack.compression\n> to be what core.compression currently is, and to set\n> core.compression to 0 to speed up large commits\n> (the resulting space-inefficient loose objects will be scrubbed away\n> by a later off-line repack).\n> Thus, my config settings (almost) change the gzip's behind a git-add to cp's.\n> Do you want me to submit timings for a git-add/git-commit -a\n> on a typical 50-file commit I would be interested in,\n> with the (new) settings that I would use?\n\nNote the linux-2.6 git tree from a week ago has 22K checked-out files\nof average size 11KB; the largest is fs/nls/nls-cp949.c at 874KB.\n(The largest file in git is gitk at 176K.)\n\nThe tree I'm interested in maintaining with git is almost 70GB\nchecked-out in 13K files of average size >5.2MB. This is over\n2 orders of magnitude larger average file size than current git users.\n(Some of these numbers may decrease after a little retraining ;-).)\nI would like git to perform as responsively as possible on files\nup to ~500MB. Within this tree, the largest file is 1234MB\n[I think checking this in was a mistake!]\nand I did the following experiments on it:\n\n$ rm -rf .git\n$ git-init\nInitialized empty Git repository in .git/\n$ git-config core.compression -1\n$ wc large.spef\n 12762072 37832482 1234082774 large.spef\n$ /usr/bin/time git-add large.spef\n41.54user 0.70system 0:49.76elapsed 84%CPU (0avgtext+0avgdata 0maxresident)k\n$ ls -lR .git/objects/??\n.git/objects/d5:\ntotal 83836\n-r--r--r-- 1 how group 85670068 May 2 15:11\nd6cde2af063cdfa835038385f29a897bf9533b\n\n$ rm -rf .git\n$ git-init\nInitialized empty Git repository in .git/\n$ git-config core.compression 1\n$ wc large.spef\n 12762072 37832482 1234082774 large.spef\n$ /usr/bin/time git-add large.spef\n23.66user 0.74system 0:34.07elapsed 71%CPU (0avgtext+0avgdata 0maxresident)k\n$ ls -lR .git/objects/??\n.git/objects/d5:\ntotal 105116\n-r--r--r-- 1 how group 107419557 May 2 15:13\nd6cde2af063cdfa835038385f29a897bf9533b\n\nSo for a 25% increase in blob size I get 33% less elapsed time\nin git-add, all by changing core.compression from -1 to 1.\nI'll definitely take that improvement. [For the compressible files\nwe typically have, using 0 is a bad idea: the CPU \"advantage\"\nis swamped out by the time to write a much larger file.]\n\nSince I don't care [to the same degree] about the responsiveness of\npacking, I'd rather pack with -1 or better to keep packs small.\n(And inflation time seems independent of compression setting.)\nSince someone might be working while the packing is happening,\nI'd rather not change the config setting to achieve this.\nHence the patch.\n\nConcerning various public repositories, clearly the patch has no\nimpact if you don't specify different core.compression and pack.compression\nvalues. If you do specify different values, I doubt there would be much\nnoticeable speed-up for e.g. the linux-2.6 repo stats I included above.\nThere would be some, but that wasn't the motivation for the patch.\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"41007","messageId":"7vejlxxho1.fsf@assigned-by-dhcp.cox.net","threadId":"7942","inReplyTo":"56b7f5510705021551q2d48f1e5i3f4a5c2f9891368a@mail.gmail.com","subject":"Re: [RFD/PATCH] Implement pack.compression and pack-objects --compression=N","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-04T05:49:02Z","receivedAt":"2007-05-04T05:49:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Dana How\" writes:\n\n> So for a 25% increase in blob size I get 33% less elapsed time\n> in git-add, all by changing core.compression from -1 to 1.\n> I'll definitely take that improvement. [For the compressible files\n> we typically have, using 0 is a bad idea: the CPU \"advantage\"\n> is swamped out by the time to write a much larger file.]\n\nThe above number is about loose objects, right?\n\n> Since I don't care [to the same degree] about the responsiveness of\n> packing, I'd rather pack with -1 or better to keep packs small.\n\nI see. You are saying that the fact that core.compression is\nused also for packing makes the variable less useful.\n\nI agree that it would make sense to have at least the pack and\ncore compression independent. I am not sure if we would also\nwant to make the pack compression tweakable depending on the\npurpose of the packing (network transfer vs .git/objects/pack/).\n\n> (And inflation time seems independent of compression setting.)\n\nTrue.\n"},{"id":"41009","messageId":"56b7f5510705040001y6d865d98x6a5cdb1d96cfc0fe@mail.gmail.com","threadId":"7942","inReplyTo":"7vejlxxho1.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFD/PATCH] Implement pack.compression and pack-objects --compression=N","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-04T07:01:14Z","receivedAt":"2007-05-04T07:01:14Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/3/07, Junio C Hamano wrote:\n> \"Dana How\" writes:\n> > So for a 25% increase in blob size I get 33% less elapsed time\n> > in git-add, all by changing core.compression from -1 to 1.\n> > I'll definitely take that improvement. [For the compressible files\n> > we typically have, using 0 is a bad idea: the CPU \"advantage\"\n> > is swamped out by the time to write a much larger file.]\n> The above number is about loose objects, right?\nCorrect.\n\n> > Since I don't care [to the same degree] about the responsiveness of\n> > packing, I'd rather pack with -1 or better to keep packs small.\n> I see. You are saying that the fact that core.compression is\n> used also for packing makes the variable less useful.\nExactly.\n\n> I agree that it would make sense to have at least the pack and\n> core compression independent. I am not sure if we would also\n> want to make the pack compression tweakable depending on the\n> purpose of the packing (network transfer vs .git/objects/pack/).\nThe final 12-line hunk in the patch implements --pack-compression=N.\nTo be blunt, I'm not sure *I* need it. Perhaps a high-use web-accessible\npublic repository would be interested in repacking off-line with 9, and\ncreating packs for each puller using -1 to reduce CPU load. In the latter\ncase, due to builtin-pack-objects.c:write_object():to_reuse==1,\nmuch of the transferred data would still be at the better compression\nbut without again paying the CPU cost. Shall I remove it?\n\nBTW, I'm now in the habit of browsing Git's Gitweb at git.or.cz and notice\n\"pu\" has --max-pack-size. You may want to upgrade to the last 5-patch\nversion in which --max-pack-size no longer forces --no-reuse-delta at\nShawn's request, among other clean-ups.\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"41055","messageId":"alpine.LFD.0.99.0705040937250.24220@xanadu.home","threadId":"7942","inReplyTo":"7vejlxxho1.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFD/PATCH] Implement pack.compression and pack-objects --compression=N","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-05-04T13:47:50Z","receivedAt":"2007-05-04T13:47:50Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 3 May 2007, Junio C Hamano wrote:\n\n> \"Dana How\" writes:\n> \n> > So for a 25% increase in blob size I get 33% less elapsed time\n> > in git-add, all by changing core.compression from -1 to 1.\n> > I'll definitely take that improvement. [For the compressible files\n> > we typically have, using 0 is a bad idea: the CPU \"advantage\"\n> > is swamped out by the time to write a much larger file.]\n> \n> The above number is about loose objects, right?\n> \n> > Since I don't care [to the same degree] about the responsiveness of\n> > packing, I'd rather pack with -1 or better to keep packs small.\n> \n> I see. You are saying that the fact that core.compression is\n> used also for packing makes the variable less useful.\n\nI think that would make sense to have separate configs for pack and \nloose object compression. When not specified they should simply default \nto core.compression if it exists. Otherwise I'd suggest that pack \ncompression default level be Z_DEFAULT_COMPRESSION and loose object \ncompression default level be Z_BEST_SPEED. This would make interactive \noperations like git-add and git-commit even faster by default.\n\n\nNicolas\n"},{"id":"41062","messageId":"56b7f5510705040910i3cac723co3aed45034c097def@mail.gmail.com","threadId":"7942","inReplyTo":"alpine.LFD.0.99.0705040937250.24220@xanadu.home","subject":"Re: [RFD/PATCH] Implement pack.compression and pack-objects --compression=N","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-04T16:10:38Z","receivedAt":"2007-05-04T16:10:38Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/4/07, Nicolas Pitre wrote:\n> On Thu, 3 May 2007, Junio C Hamano wrote:\n>\n> > \"Dana How\" writes:\n> >\n> > > So for a 25% increase in blob size I get 33% less elapsed time\n> > > in git-add, all by changing core.compression from -1 to 1.\n> > > I'll definitely take that improvement. [For the compressible files\n> > > we typically have, using 0 is a bad idea: the CPU \"advantage\"\n> > > is swamped out by the time to write a much larger file.]\n> >\n> > The above number is about loose objects, right?\n> >\n> > > Since I don't care [to the same degree] about the responsiveness of\n> > > packing, I'd rather pack with -1 or better to keep packs small.\n> >\n> > I see. You are saying that the fact that core.compression is\n> > used also for packing makes the variable less useful.\n>\n> I think that would make sense to have separate configs for pack and\n> loose object compression. When not specified they should simply default\n> to core.compression if it exists. Otherwise I'd suggest that pack\n> compression default level be Z_DEFAULT_COMPRESSION and loose object\n> compression default level be Z_BEST_SPEED. This would make interactive\n> operations like git-add and git-commit even faster by default.\n\nI agree with your Z_BEST_SPEED idea. I did not include it in\nthe patch b/c I didn't want to change any behavior in the absence\nof new config settings.\n\nAre you actually arguing for *3* different compression-related config\nvariables? How about:\n(a) core.compression controls loose objects. defaults to Z_BEST_SPEED.\n(b) pack.compression controls packing. defaults to Z_DEFAULT_COMPRESSION\nif neither variable exists. defaults to core.compression if only that exists\n\nThe last sentence mimics current behavior and to me seems least\nsurprising. Or pack.compression could be simpler: default to\nZ_DEFAULT_COMPRESSION if pack.compression doesn't exist\n(no interaction with core.compression).\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"41063","messageId":"alpine.LFD.0.99.0705041218130.24220@xanadu.home","threadId":"7942","inReplyTo":"56b7f5510705040910i3cac723co3aed45034c097def@mail.gmail.com","subject":"Re: [RFD/PATCH] Implement pack.compression and pack-objects --compression=N","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-05-04T16:30:24Z","receivedAt":"2007-05-04T16:30:24Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 4 May 2007, Dana How wrote:\n\n> On 5/4/07, Nicolas Pitre wrote:\n> > On Thu, 3 May 2007, Junio C Hamano wrote:\n> > \n> > > \"Dana How\" writes:\n> > >\n> > > > So for a 25% increase in blob size I get 33% less elapsed time\n> > > > in git-add, all by changing core.compression from -1 to 1.\n> > > > I'll definitely take that improvement. [For the compressible files\n> > > > we typically have, using 0 is a bad idea: the CPU \"advantage\"\n> > > > is swamped out by the time to write a much larger file.]\n> > >\n> > > The above number is about loose objects, right?\n> > >\n> > > > Since I don't care [to the same degree] about the responsiveness of\n> > > > packing, I'd rather pack with -1 or better to keep packs small.\n> > >\n> > > I see. You are saying that the fact that core.compression is\n> > > used also for packing makes the variable less useful.\n> > \n> > I think that would make sense to have separate configs for pack and\n> > loose object compression. When not specified they should simply default\n> > to core.compression if it exists. Otherwise I'd suggest that pack\n> > compression default level be Z_DEFAULT_COMPRESSION and loose object\n> > compression default level be Z_BEST_SPEED. This would make interactive\n> > operations like git-add and git-commit even faster by default.\n> \n> I agree with your Z_BEST_SPEED idea. I did not include it in\n> the patch b/c I didn't want to change any behavior in the absence\n> of new config settings.\n> \n> Are you actually arguing for *3* different compression-related config\n> variables?\n\nYes.\n\n> How about:\n> (a) core.compression controls loose objects. defaults to Z_BEST_SPEED.\n> (b) pack.compression controls packing. defaults to Z_DEFAULT_COMPRESSION\n> if neither variable exists. defaults to core.compression if only that exists\n\nYes, although I wouldn't default pack.compression to core.compression \nif pack.compression doesn't exist. The documentation about \ncore.compression currently talks \n(wrongly) only about loose objects anyway, so making pack.compression \nstand on its own won't be that bad.\n\n\nNicolas\n"},{"id":"41083","messageId":"56b7f5510705041251v1a09103eif677c02efc53059e@mail.gmail.com","threadId":"7942","inReplyTo":"alpine.LFD.0.99.0705041218130.24220@xanadu.home","subject":"Re: [RFD/PATCH] Implement pack.compression and pack-objects --compression=N","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-04T19:51:15Z","receivedAt":"2007-05-04T19:51:15Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/4/07, Nicolas Pitre wrote:\n> On Fri, 4 May 2007, Dana How wrote:\n> > On 5/4/07, Nicolas Pitre wrote:\n> > > I think that would make sense to have separate configs for pack and\n> > > loose object compression. When not specified they should simply default\n> > > to core.compression if it exists. Otherwise I'd suggest that pack\n> > > compression default level be Z_DEFAULT_COMPRESSION and loose object\n> > > compression default level be Z_BEST_SPEED. This would make interactive\n> > > operations like git-add and git-commit even faster by default.\n> > I agree with your Z_BEST_SPEED idea. I did not include it in\n> > the patch b/c I didn't want to change any behavior in the absence\n> > of new config settings.\n> > Are you actually arguing for *3* different compression-related config\n> > variables?\n> Yes.\n>\n> > How about:\n> > (a) core.compression controls loose objects. defaults to Z_BEST_SPEED.\n> > (b) pack.compression controls packing. defaults to Z_DEFAULT_COMPRESSION\n> > if neither variable exists. defaults to core.compression if only that exists\n> Yes, although I wouldn't default pack.compression to core.compression\n> if pack.compression doesn't exist. The documentation about\n> core.compression currently talks\n> (wrongly) only about loose objects anyway, so making pack.compression\n> stand on its own won't be that bad.\n\nNow that I'm awake your original quote at the top suggests:\n(a) zlib_compression_level =\n isset(core.loosecompression) ? core.loosecompression :\n isset(core.compression) ? core.compression : Z_BEST_SPEED;\n(b) pack_compression_level =\n isset(pack.compression) ? pack.compression :\n isset(core.compression) ? core.compression : Z_DEFAULT_COMPRESSION;\n\nYour later reaction to my quoted (a)/(b) table suggests:\n(a) zlib_compression_level =\n isset(core.compression) ? core.compression : Z_BEST_SPEED;\n(b) pack_compresion_level =\n isset(pack.compression) ? pack.compression : Z_DEFAULT_COMPRESSION;\n\nIn either case, the C variable\nzlib_compression_level controls compression level for loose objects,\nand pack_compression_level controls compression in a pack.\n\"isset()\" means an active setting does appear in a config file\n(it could be the default value).\n\nThe 2nd behavior table is sufficient for me and simpler than my patch.\nDo you want the 1st or 2nd behavior?\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"41084","messageId":"alpine.LFD.0.99.0705041609260.24220@xanadu.home","threadId":"7942","inReplyTo":"56b7f5510705041251v1a09103eif677c02efc53059e@mail.gmail.com","subject":"Re: [RFD/PATCH] Implement pack.compression and pack-objects --compression=N","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-05-04T20:17:35Z","receivedAt":"2007-05-04T20:17:35Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 4 May 2007, Dana How wrote:\n\n> Now that I'm awake your original quote at the top suggests:\n> (a) zlib_compression_level =\n> isset(core.loosecompression) ? core.loosecompression :\n> isset(core.compression) ? core.compression : Z_BEST_SPEED;\n> (b) pack_compression_level =\n> isset(pack.compression) ? pack.compression :\n> isset(core.compression) ? core.compression : Z_DEFAULT_COMPRESSION;\n> \n> Your later reaction to my quoted (a)/(b) table suggests:\n> (a) zlib_compression_level =\n> isset(core.compression) ? core.compression : Z_BEST_SPEED;\n> (b) pack_compresion_level =\n> isset(pack.compression) ? pack.compression : Z_DEFAULT_COMPRESSION;\n\nMy final comment is that I think the former looks more coherent, while \nthe later is simpler to implement.\n\n\nNicolas\n"}]}