{"thread":{"id":"36184","subject":"[PATCH 0/4] Better \"gc --aggressive\"","startedAt":"2014-03-16T13:34:59Z","lastAt":"2014-03-20T01:31:07Z","messageCount":21,"participants":["Nguyễn Thái Ngọc Duy","Duy Nguyen","Junio C Hamano","Jeff King","David Kastrup"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"236829","messageId":"1394976904-15395-1-git-send-email-pclouds@gmail.com","threadId":"36184","inReplyTo":null,"subject":"[PATCH 0/4] Better \"gc --aggressive\"","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-16T13:34:59Z","receivedAt":"2014-03-16T13:34:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"See [1] for the discussion that led to this series. It attempts to\npack the repo with two different depths: old history tightly packed\n(smaller but also takes longer time to access) and recent history on\nthe opposite.\n\nFirst draft, probably still some bugs lurking in pack_old_history().\nIt would be great if people could try it out on large repos and report\nback the results (pack size between the old and new aggressive, gc\ntime, git log and blame speed...)\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/242277\n\nNguyễn Thái Ngọc Duy (4):\n  environment.c: fix constness for odb_pack_keep()\n  pack-objects: support --keep\n  gc --aggressive: make --depth configurable\n  gc --aggressive: three phase repacking\n\n Documentation/config.txt           |  24 ++++++++\n Documentation/git-gc.txt           |   3 +\n Documentation/git-pack-objects.txt |   4 ++\n builtin/gc.c                       | 113 ++++++++++++++++++++++++++++++++++++-\n builtin/pack-objects.c             |  26 +++++++++\n environment.c                      |   2 +-\n git-compat-util.h                  |   2 +-\n 7 files changed, 169 insertions(+), 5 deletions(-)\n\n-- \n1.9.0.40.gaa8c3ea\n"},{"id":"236830","messageId":"1394976904-15395-2-git-send-email-pclouds@gmail.com","threadId":"36184","inReplyTo":"1394976904-15395-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 1/4] environment.c: fix constness for odb_pack_keep()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-16T13:35:00Z","receivedAt":"2014-03-16T13:35:00Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n environment.c     | 2 +-\n git-compat-util.h | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/environment.c b/environment.c\nindex c3c8606..5c4815d 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -237,7 +237,7 @@ int odb_mkstemp(char *template, size_t limit, const char *pattern)\n \treturn xmkstemp_mode(template, mode);\n }\n \n-int odb_pack_keep(char *name, size_t namesz, unsigned char *sha1)\n+int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1)\n {\n \tint fd;\n \ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 585ef8a..adbfb5e 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -533,7 +533,7 @@ extern FILE *xfdopen(int fd, const char *mode);\n extern int xmkstemp(char *template);\n extern int xmkstemp_mode(char *template, int mode);\n extern int odb_mkstemp(char *template, size_t limit, const char *pattern);\n-extern int odb_pack_keep(char *name, size_t namesz, unsigned char *sha1);\n+extern int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1);\n \n static inline size_t xsize_t(off_t len)\n {\n-- \n1.9.0.40.gaa8c3ea\n"},{"id":"236831","messageId":"1394976904-15395-3-git-send-email-pclouds@gmail.com","threadId":"36184","inReplyTo":"1394976904-15395-1-git-send-email-pclouds@gmail.com","subject":"[PATCH] index-pack: do not segfault when keep_name is NULL","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-16T13:35:01Z","receivedAt":"2014-03-16T13:35:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"keep_name is used to print error messages a couple lines down. Reset\nit to the real path returned by odb_pack_keep() if it's set to NULL by\ncaller.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n One of these moments I will make git log and friends optionally recognize\n \"Diff-Options:\" line in commit message. Adding -U14 in this case\n should help the reviewer to see how those error messages are printed.\n\n builtin/index-pack.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex a6b1c17..d95c3dc 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1283,9 +1283,10 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n \tif (keep_msg) {\n \t\tint keep_fd, keep_msg_len = strlen(keep_msg);\n \n-\t\tif (!keep_name)\n+\t\tif (!keep_name) {\n \t\t\tkeep_fd = odb_pack_keep(name, sizeof(name), sha1);\n-\t\telse\n+\t\t\tkeep_name = name;\n+\t\t} else\n \t\t\tkeep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600);\n \n \t\tif (keep_fd < 0) {\n-- \n1.9.0.40.gaa8c3ea\n"},{"id":"236832","messageId":"1394976904-15395-4-git-send-email-pclouds@gmail.com","threadId":"36184","inReplyTo":"1394976904-15395-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/4] pack-objects: support --keep","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-16T13:35:02Z","receivedAt":"2014-03-16T13:35:02Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-pack-objects.txt |  4 ++++\n builtin/pack-objects.c             | 26 ++++++++++++++++++++++++++\n 2 files changed, 30 insertions(+)\n\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex cdab9ed..8aae3b5 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -117,6 +117,10 @@ base-name::\n \thas a .keep file to be ignored, even if it would have\n \totherwise been packed.\n \n+--keep::\n+\tBefore moving the index into its final destination\n+\tcreate an empty .keep file for the associated pack file.\n+\n --incremental::\n \tThis flag causes an object already in a pack to be ignored\n \teven if it would have otherwise been packed.\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1fb972f..46c7a57 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -57,6 +57,7 @@ static int num_preferred_base;\n static struct progress *progress_state;\n static int pack_compression_level = Z_DEFAULT_COMPRESSION;\n static int pack_compression_seen;\n+static int keep_pack;\n \n static struct packed_git *reuse_packfile;\n static uint32_t reuse_packfile_objects;\n@@ -745,6 +746,23 @@ static off_t write_reused_pack(struct sha1file *f)\n \treturn reuse_packfile_offset - sizeof(struct pack_header);\n }\n \n+static void write_keep_file(const unsigned char *sha1)\n+{\n+\tconst char *msg = \"pack-objects --keep\\n\";\n+\tchar name[PATH_MAX];\n+\tint keep_fd = odb_pack_keep(name, sizeof(name), sha1);\n+\tif (keep_fd < 0) {\n+\t\tif (errno != EEXIST)\n+\t\t\tdie_errno(_(\"cannot write keep file '%s'\"),\n+\t\t\t\t  name);\n+\t\treturn;\n+\t}\n+\twrite_or_die(keep_fd, msg, strlen(msg));\n+\tif (close(keep_fd) != 0)\n+\t\tdie_errno(_(\"cannot close written keep file '%s'\"),\n+\t\t\t  name);\n+}\n+\n static void write_pack_file(void)\n {\n \tuint32_t i = 0, j;\n@@ -805,6 +823,9 @@ static void write_pack_file(void)\n \t\t\tstruct stat st;\n \t\t\tchar tmpname[PATH_MAX];\n \n+\t\t\tif (keep_pack)\n+\t\t\t\twrite_keep_file(sha1);\n+\n \t\t\t/*\n \t\t\t * Packs are runtime accessed in their mtime\n \t\t\t * order since newer packs are more likely to contain\n@@ -2609,6 +2630,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"use a bitmap index if available to speed up counting objects\")),\n \t\tOPT_BOOL(0, \"write-bitmap-index\", &write_bitmap_index,\n \t\t\t N_(\"write a bitmap index together with the pack index\")),\n+\t\tOPT_BOOL(0, \"keep\", &keep_pack,\n+\t\t\t N_(\"create .keep for the new pack\")),\n \t\tOPT_END(),\n \t};\n \n@@ -2672,6 +2695,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tif (!pack_to_stdout && thin)\n \t\tdie(\"--thin cannot be used to build an indexable pack.\");\n \n+\tif (pack_to_stdout && keep_pack)\n+\t\tdie(\"--keep cannot be used with --stdout\");\n+\n \tif (keep_unreachable && unpack_unreachable)\n \t\tdie(\"--keep-unreachable and --unpack-unreachable are incompatible.\");\n \n-- \n1.9.0.40.gaa8c3ea\n"},{"id":"236833","messageId":"1394976904-15395-5-git-send-email-pclouds@gmail.com","threadId":"36184","inReplyTo":"1394976904-15395-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 3/4] gc --aggressive: make --depth configurable","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-16T13:35:03Z","receivedAt":"2014-03-16T13:35:03Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When 1c192f3 (gc --aggressive: make it really aggressive - 2007-12-06)\nmade --depth=250 the default value, it didn't really explain the\nreason behind, especially the pros and cons of --depth=250.\n\nAn old mail from Linus below explains it at length. Long story short,\n--depth=250 is a disk saver and a performance killer. Not everybody\nagrees on that aggressiveness. Let the user configure it.\n\n-- 8< --\nFrom: Linus Torvalds <torvalds@linux-foundation.org>\nSubject: Re: [PATCH] gc --aggressive: make it really aggressive\nDate: Thu, 6 Dec 2007 08:19:24 -0800 (PST)\nMessage-ID: <alpine.LFD.0.9999.0712060803430.13796@woody.linux-foundation.org>\nGmane-URL: http://article.gmane.org/gmane.comp.gcc.devel/94637\n\nOn Thu, 6 Dec 2007, Harvey Harrison wrote:\n>\n> 7:41:25elapsed 86%CPU\n\nHeh. And this is why you want to do it exactly *once*, and then just\nexport the end result for others ;)\n\n> -r--r--r-- 1 hharrison hharrison 324094684 2007-12-06 07:26 pack-1d46ca030c3d6d6b95ad316deb922be06b167a3d.pack\n\nBut yeah, especially if you allow longer delta chains, the end result can\nbe much smaller (and what makes the one-time repack more expensive is the\nwindow size, not the delta chain - you could make the delta chains longer\nwith no cost overhead at packing time)\n\nHOWEVER.\n\nThe longer delta chains do make it potentially much more expensive to then\nuse old history. So there's a trade-off. And quite frankly, a delta depth\nof 250 is likely going to cause overflows in the delta cache (which is\nonly 256 entries in size *and* it's a hash, so it's going to start having\nhash conflicts long before hitting the 250 depth limit).\n\nSo when I said \"--depth=250 --window=250\", I chose those numbers more as\nan example of extremely aggressive packing, and I'm not at all sure that\nthe end result is necessarily wonderfully usable. It's going to save disk\nspace (and network bandwidth - the delta's will be re-used for the network\nprotocol too!), but there are definitely downsides too, and using long\ndelta chains may simply not be worth it in practice.\n\n(And some of it might just want to have git tuning, ie if people think\nthat long deltas are worth it, we could easily just expand on the delta\nhash, at the cost of some more memory used!)\n\nThat said, the good news is that working with *new* history will not be\naffected negatively, and if you want to be _really_ sneaky, there are ways\nto say \"create a pack that contains the history up to a version one year\nago, and be very aggressive about those old versions that we still want to\nhave around, but do a separate pack for newer stuff using less aggressive\nparameters\"\n\nSo this is something that can be tweaked, although we don't really have\nany really nice interfaces for stuff like that (ie the git delta cache\nsize is hardcoded in the sources and cannot be set in the config file, and\nthe \"pack old history more aggressively\" involves some manual scripting\nand knowing how \"git pack-objects\" works rather than any nice simple\ncommand line switch).\n\nSo the thing to take away from this is:\n - git is certainly flexible as hell\n - .. but to get the full power you may need to tweak things\n - .. happily you really only need to have one person to do the tweaking,\n   and the tweaked end results will be available to others that do not\n   need to know/care.\n\nAnd whether the difference between 320MB and 500MB is worth any really\ninvolved tweaking (considering the potential downsides), I really don't\nknow. Only testing will tell.\n\n\t\t\tLinus\n-- 8< --\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/config.txt | 5 +++++\n Documentation/git-gc.txt | 3 +++\n builtin/gc.c             | 8 +++++++-\n 3 files changed, 15 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 79e5768..5ce7f9a 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1151,6 +1151,11 @@ filter.<driver>.smudge::\n \tobject to a worktree file upon checkout.  See\n \tlinkgit:gitattributes[5] for details.\n \n+gc.aggressiveDepth::\n+\tThe depth parameter used in the delta compression\n+\talgorithm used by 'git gc --aggressive'.  This defaults\n+\tto 250.\n+\n gc.aggressiveWindow::\n \tThe window size parameter used in the delta compression\n \talgorithm used by 'git gc --aggressive'.  This defaults\ndiff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\nindex e158a3b..273c466 100644\n--- a/Documentation/git-gc.txt\n+++ b/Documentation/git-gc.txt\n@@ -124,6 +124,9 @@ the value, the more time is spent optimizing the delta compression.  See\n the documentation for the --window' option in linkgit:git-repack[1] for\n more details.  This defaults to 250.\n \n+Similarly, the optional configuration variable 'gc.aggressiveDepth'\n+controls --depth option in linkgit:git-repack[1]. This defaults to 250.\n+\n The optional configuration variable 'gc.pruneExpire' controls how old\n the unreferenced loose objects have to be before they are pruned.  The\n default is \"2 weeks ago\".\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 63d400b..72aa912 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -26,6 +26,7 @@ static const char * const builtin_gc_usage[] = {\n };\n \n static int pack_refs = 1;\n+static int aggressive_depth = 250;\n static int aggressive_window = 250;\n static int gc_auto_threshold = 6700;\n static int gc_auto_pack_limit = 50;\n@@ -66,6 +67,10 @@ static int gc_config(const char *var, const char *value, void *cb)\n \t\taggressive_window = git_config_int(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"gc.aggressivedepth\")) {\n+\t\taggressive_depth = git_config_int(var, value);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"gc.auto\")) {\n \t\tgc_auto_threshold = git_config_int(var, value);\n \t\treturn 0;\n@@ -294,7 +299,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \n \tif (aggressive) {\n \t\targv_array_push(&repack, \"-f\");\n-\t\targv_array_push(&repack, \"--depth=250\");\n+\t\tif (aggressive_depth > 0)\n+\t\t\targv_array_pushf(&repack, \"--depth=%d\", aggressive_depth);\n \t\tif (aggressive_window > 0)\n \t\t\targv_array_pushf(&repack, \"--window=%d\", aggressive_window);\n \t}\n-- \n1.9.0.40.gaa8c3ea\n"},{"id":"236834","messageId":"1394976904-15395-6-git-send-email-pclouds@gmail.com","threadId":"36184","inReplyTo":"1394976904-15395-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 4/4] gc --aggressive: three phase repacking","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-16T13:35:04Z","receivedAt":"2014-03-16T13:35:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"As explained in the previous commit, current aggressive settings\n--depth=250 --window=250 could slow down repository access\nsignificantly. Notice that people usually work on recent history only,\nwe could keep recent history more loosely packed, so that repo access\nis fast most of the time while the pack file remains small.\n\nThree more configuration variables are used to make that happen. The\nfirst one, gc.aggressiveCommitLimits covers the old history part,\nwhich will be tightly packed. The remaining part will be packed with\ngc.lessAggresiveWindow and gc.lessAggressiveDepth. If\ngc.aggressiveCommitLimits is empty, everything will be tightly packed\nas before.\n\nThe repack process becomes:\n\n - repack -adf on old history (e.g. the default --before=1.year.ago)\n   mark to keep that pack\n\n - repack the second time with lessAggressive settings, the kept pack\n   should be left untouched.\n\n - remove .keep file and repack the final time, reusing all deltas\n\nThis process costs more time, but produce a more effecient pack. It is\nassumed that people who do \"gc --aggressive\" do not do this often and\ndo not mind if it takes a bit longer.\n\ngit.git is not a great repo to test it because its size is modest but\nso are my laptop's cpu and memory, so here are the timings and pack\nsizes\n\n            size  time\nold aggr.   36MB  5m51\nnew aggr.   37MB  6m13\nrepack -adf 48MB  1m12\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/config.txt |  19 +++++++++\n builtin/gc.c             | 109 +++++++++++++++++++++++++++++++++++++++++++++--\n 2 files changed, 124 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 5ce7f9a..47979dc 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1161,6 +1161,25 @@ gc.aggressiveWindow::\n \talgorithm used by 'git gc --aggressive'.  This defaults\n \tto 250.\n \n+gc.aggressiveCommitLimits::\n+\tThis one parameter to linkgit:git-rev-list[1] to select\n+\tcommits that are repacked with gc.aggressiveDepth and\n+\tgc.aggressiveWindow, while the remaining commits are repacked\n+\twith gc.lessAggressiveDepth and gc.lessAggressiveWindow.\n++\n+If this is an empty string, everything will be repacked with\n+gc.aggressiveWindow and gc.aggressiveDepth.\n+\n+gc.lessAggressiveDepth::\n+\tThe depth parameter used in the delta compression\n+\talgorithm used by 'git gc --aggressive' when\n+\tgc.aggressiveCommitLimits is set.  This defaults to 50.\n+\n+gc.lessAggressiveWindow::\n+\tThe window size parameter used in the delta compression\n+\talgorithm used by 'git gc --aggressive' when\n+\tgc.aggressiveCommitLimits is set.  This defaults to 250.\n+\n gc.auto::\n \tWhen there are approximately more than this many loose\n \tobjects in the repository, `git gc --auto` will pack them.\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 72aa912..37fc603 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -28,10 +28,14 @@ static const char * const builtin_gc_usage[] = {\n static int pack_refs = 1;\n static int aggressive_depth = 250;\n static int aggressive_window = 250;\n+static const char *aggressive_rev_list = \"--before=1.year.ago\";\n+static int less_aggressive_depth = 50;\n+static int less_aggressive_window = 250;\n static int gc_auto_threshold = 6700;\n static int gc_auto_pack_limit = 50;\n static int detach_auto = 1;\n static const char *prune_expire = \"2.weeks.ago\";\n+static int delta_base_offset = 1;\n \n static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;\n static struct argv_array reflog = ARGV_ARRAY_INIT;\n@@ -39,10 +43,13 @@ static struct argv_array repack = ARGV_ARRAY_INIT;\n static struct argv_array prune = ARGV_ARRAY_INIT;\n static struct argv_array rerere = ARGV_ARRAY_INIT;\n \n+static char *keep_file;\n static char *pidfile;\n \n static void remove_pidfile(void)\n {\n+\tif (keep_file)\n+\t\tunlink_or_warn(keep_file);\n \tif (pidfile)\n \t\tunlink(pidfile);\n }\n@@ -54,6 +61,63 @@ static void remove_pidfile_on_signal(int signo)\n \traise(signo);\n }\n \n+static void pack_old_history(int quiet)\n+{\n+\tstruct child_process pack_objects;\n+\tstruct child_process rev_list;\n+\tstruct argv_array av_po = ARGV_ARRAY_INIT;\n+\tstruct argv_array av_rl = ARGV_ARRAY_INIT;\n+\tchar sha1[41];\n+\n+\targv_array_pushl(&av_rl, \"rev-list\", \"--all\", \"--objects\",\n+\t\t\t \"--reflog\", NULL);\n+\targv_array_push(&av_rl, aggressive_rev_list);\n+\n+\tmemset(&rev_list, 0, sizeof(rev_list));\n+\trev_list.no_stdin = 1;\n+\trev_list.out = -1;\n+\trev_list.git_cmd = 1;\n+\trev_list.argv = av_rl.argv;\n+\n+\tif (start_command(&rev_list))\n+\t\tdie(_(\"gc: unable to fork git-rev-list\"));\n+\n+\targv_array_pushl(&av_po, \"pack-objects\", \"--keep-true-parents\",\n+\t\t\t \"--honor-pack-keep\", \"--non-empty\", \"--no-reuse-delta\",\n+\t\t\t \"--keep\", \"--local\", NULL);\n+\tif (delta_base_offset)\n+\t\targv_array_push(&av_po,  \"--delta-base-offset\");\n+\tif (quiet)\n+\t\targv_array_push(&av_po, \"-q\");\n+\tif (aggressive_window)\n+\t\targv_array_pushf(&av_po, \"--window=%d\", aggressive_window);\n+\tif (aggressive_depth)\n+\t\targv_array_pushf(&av_po, \"--depth=%d\", aggressive_depth);\n+\targv_array_push(&av_po, git_path(\"objects/pack/pack\"));\n+\n+\tmemset(&pack_objects, 0, sizeof(pack_objects));\n+\tpack_objects.in = rev_list.out;\n+\tpack_objects.out = -1;\n+\tpack_objects.git_cmd = 1;\n+\tpack_objects.argv = av_po.argv;\n+\n+\tif (start_command(&pack_objects))\n+\t\tdie(_(\"gc: unable to fork git-pack-objects\"));\n+\n+\tif (read_in_full(pack_objects.out, sha1, 41) != 41 ||\n+\t    sha1[40] != '\\n')\n+\t\tdie_errno(_(\"gc: pack-objects did not return the new pack's SHA-1\"));\n+\tsha1[40] = '\\0';\n+\tkeep_file = git_pathdup(\"objects/pack/pack-%s.keep\", sha1);\n+\tclose(pack_objects.out);\n+\n+\tif (finish_command(&rev_list))\n+\t\tdie(_(\"gc: git-rev-list died with error\"));\n+\n+\tif (finish_command(&pack_objects))\n+\t\tdie(_(\"gc: git-pack-objects died with error\"));\n+}\n+\n static int gc_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(var, \"gc.packrefs\")) {\n@@ -71,6 +135,22 @@ static int gc_config(const char *var, const char *value, void *cb)\n \t\taggressive_depth = git_config_int(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"gc.aggressivecommitlimits\")) {\n+\t\taggressive_rev_list = value && *value ? xstrdup(value) : NULL;\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(var, \"gc.lessaggressivewindow\")) {\n+\t\tless_aggressive_window = git_config_int(var, value);\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(var, \"gc.lessaggressivedepth\")) {\n+\t\tless_aggressive_depth = git_config_int(var, value);\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(var, \"repack.usedeltabaseoffset\")) {\n+\t\tdelta_base_offset = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"gc.auto\")) {\n \t\tgc_auto_threshold = git_config_int(var, value);\n \t\treturn 0;\n@@ -298,11 +378,19 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(builtin_gc_usage, builtin_gc_options);\n \n \tif (aggressive) {\n+\t\tint depth, window;\n+\t\tif (aggressive_rev_list) {\n+\t\t\tdepth = less_aggressive_depth;\n+\t\t\twindow = less_aggressive_window;\n+\t\t} else {\n+\t\t\tdepth = aggressive_depth;\n+\t\t\twindow = aggressive_window;\n+\t\t}\n \t\targv_array_push(&repack, \"-f\");\n-\t\tif (aggressive_depth > 0)\n-\t\t\targv_array_pushf(&repack, \"--depth=%d\", aggressive_depth);\n-\t\tif (aggressive_window > 0)\n-\t\t\targv_array_pushf(&repack, \"--window=%d\", aggressive_window);\n+\t\tif (depth > 0)\n+\t\t\targv_array_pushf(&repack, \"--depth=%d\", depth);\n+\t\tif (window > 0)\n+\t\t\targv_array_pushf(&repack, \"--window=%d\", window);\n \t}\n \tif (quiet)\n \t\targv_array_push(&repack, \"-q\");\n@@ -343,9 +431,22 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \tif (run_command_v_opt(reflog.argv, RUN_GIT_CMD))\n \t\treturn error(FAILED_RUN, reflog.argv[0]);\n \n+\tif (aggressive && aggressive_rev_list)\n+\t\tpack_old_history(quiet);\n+\n \tif (run_command_v_opt(repack.argv, RUN_GIT_CMD))\n \t\treturn error(FAILED_RUN, repack.argv[0]);\n \n+\tif (aggressive && aggressive_rev_list) {\n+\t\tif (keep_file)\n+\t\t\tunlink_or_warn(keep_file);\n+\t\targv_array_clear(&repack);\n+\t\targv_array_pushl(&repack, \"repack\", \"-d\", \"-l\", NULL);\n+\t\tadd_repack_all_option();\n+\t\tif (run_command_v_opt(repack.argv, RUN_GIT_CMD))\n+\t\t\treturn error(FAILED_RUN, repack.argv[0]);\n+\t}\n+\n \tif (prune_expire) {\n \t\targv_array_push(&prune, prune_expire);\n \t\tif (quiet)\n-- \n1.9.0.40.gaa8c3ea\n"},{"id":"236846","messageId":"CACsJy8BazLsv5PACp4WE5ToAr6S7JuByRYZ=2FErDPbiOZRZWw@mail.gmail.com","threadId":"36184","inReplyTo":"CAG+J_Dw=Y5d2JTOngkxH=vNg3C43nP5=y7S6VXS=aHgmBshYZQ@mail.gmail.com","subject":"Re: [PATCH 3/4] gc --aggressive: make --depth configurable","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-16T23:06:04Z","receivedAt":"2014-03-16T23:06:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Mar 17, 2014 at 12:10 AM, Jay Soffian <jaysoffian@gmail.com> wrote:\n> On Sun, Mar 16, 2014 at 9:35 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> wrote:\n>>\n>> When 1c192f3 (gc --aggressive: make it really aggressive - 2007-12-06)\n>> made --depth=250 the default value, it didn't really explain the\n>> reason behind, especially the pros and cons of --depth=250.\n>>\n>> An old mail from Linus below explains it at length. Long story short,\n>> --depth=250 is a disk saver and a performance killer. Not everybody\n>> agrees on that aggressiveness. Let the user configure it.\n>\n>\n> Suggestion to link to the email discussion(s) on gmane in the Documentation\n> or at least the commit message?\n\nIf you mean the discussion back in 2007, there is a fake header\n\"Gmane-URL\" in Linus' mail in the commit message.\n-- \nDuy\n"},{"id":"236929","messageId":"xmqqfvmgbhlb.fsf@gitster.dls.corp.google.com","threadId":"36184","inReplyTo":"1394976904-15395-6-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-17T22:12:00Z","receivedAt":"2014-03-17T22:12:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> As explained in the previous commit,...\n\n[PATCH 3/4] becomes a commit with an empty log message for some\nreason.  Have you tried running \"am -s\" on it?\n"},{"id":"236936","messageId":"CACsJy8BgHp=w3RU==9tiebnOPxzCnwurpZbNjxtS5O=q_kDcOA@mail.gmail.com","threadId":"36184","inReplyTo":"xmqqfvmgbhlb.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-17T22:59:32Z","receivedAt":"2014-03-17T22:59:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 18, 2014 at 5:12 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> As explained in the previous commit,...\n>\n> [PATCH 3/4] becomes a commit with an empty log message for some\n> reason.  Have you tried running \"am -s\" on it?\n\n\"am -s\" worked fine. \"am -s --scissors\" did not (because of my use of\nscissors in the commit message). Not sure what happened on your side..\n-- \nDuy\n"},{"id":"236941","messageId":"xmqq8us8a0gk.fsf@gitster.dls.corp.google.com","threadId":"36184","inReplyTo":"CACsJy8BgHp=w3RU==9tiebnOPxzCnwurpZbNjxtS5O=q_kDcOA@mail.gmail.com","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-17T23:07:23Z","receivedAt":"2014-03-17T23:07:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> On Tue, Mar 18, 2014 at 5:12 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>>\n>>> As explained in the previous commit,...\n>>\n>> [PATCH 3/4] becomes a commit with an empty log message for some\n>> reason.  Have you tried running \"am -s\" on it?\n>\n> \"am -s\" worked fine. \"am -s --scissors\" did not (because of my use of\n> scissors in the commit message). Not sure what happened on your side..\n\nYeah, I meant \"am -c\".\n"},{"id":"236958","messageId":"20140318045050.GB8240@sigill.intra.peff.net","threadId":"36184","inReplyTo":"1394976904-15395-6-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-18T04:50:50Z","receivedAt":"2014-03-18T04:50:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 16, 2014 at 08:35:04PM +0700, Nguyễn Thái Ngọc Duy wrote:\n\n> As explained in the previous commit, current aggressive settings\n> --depth=250 --window=250 could slow down repository access\n> significantly. Notice that people usually work on recent history only,\n> we could keep recent history more loosely packed, so that repo access\n> is fast most of the time while the pack file remains small.\n\nOne thing I have not seen is real-world timings showing the slowdown\nbased on --depth. Did I miss them, or are we just making assumptions\nbased on one old case from 2009 (that, AFAIK does not have real numbers,\njust speculation)? Has anyone measured the effect of bumping the delta\ncache size (and its hash implementation)?\n\n> git.git is not a great repo to test it because its size is modest but\n> so are my laptop's cpu and memory, so here are the timings and pack\n> sizes\n> \n>             size  time\n> old aggr.   36MB  5m51\n> new aggr.   37MB  6m13\n> repack -adf 48MB  1m12\n\nI am not clear on what these times mean. It looks like the new code is\nslower _and_ bigger. Can you explain them?\n\n-Peff\n"},{"id":"236959","messageId":"CACsJy8CU3JyL74OuurjDr5-FHmBOCRahtXukukCqGx1cTdjvMQ@mail.gmail.com","threadId":"36184","inReplyTo":"20140318045050.GB8240@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-18T05:00:48Z","receivedAt":"2014-03-18T05:00:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 18, 2014 at 11:50 AM, Jeff King <peff@peff.net> wrote:\n> On Sun, Mar 16, 2014 at 08:35:04PM +0700, Nguyễn Thái Ngọc Duy wrote:\n>\n>> As explained in the previous commit, current aggressive settings\n>> --depth=250 --window=250 could slow down repository access\n>> significantly. Notice that people usually work on recent history only,\n>> we could keep recent history more loosely packed, so that repo access\n>> is fast most of the time while the pack file remains small.\n>\n> One thing I have not seen is real-world timings showing the slowdown\n> based on --depth. Did I miss them, or are we just making assumptions\n> based on one old case from 2009 (that, AFAIK does not have real numbers,\n> just speculation)? Has anyone measured the effect of bumping the delta\n> cache size (and its hash implementation)?\n\nDavid tested it with git-blame [1]. I should probably run some tests\ntoo (I don't remember if I tested some operations last time).\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/242277/focus=242435\n\n>> git.git is not a great repo to test it because its size is modest but\n>> so are my laptop's cpu and memory, so here are the timings and pack\n>> sizes\n>>\n>>             size  time\n>> old aggr.   36MB  5m51\n>> new aggr.   37MB  6m13\n>> repack -adf 48MB  1m12\n>\n> I am not clear on what these times mean. It looks like the new code is\n> slower _and_ bigger. Can you explain them?\n\nThat's right :) The upside is faster operations, which is complely\nmissed here. The good thing from those numbers is pack size does not\nincrease much (the upper limit would be repack -adf with default\nsettings).\n-- \nDuy\n"},{"id":"236960","messageId":"20140318050727.GA14769@sigill.intra.peff.net","threadId":"36184","inReplyTo":"20140318045050.GB8240@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-18T05:07:27Z","receivedAt":"2014-03-18T05:07:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 18, 2014 at 12:50:50AM -0400, Jeff King wrote:\n\n> On Sun, Mar 16, 2014 at 08:35:04PM +0700, Nguyễn Thái Ngọc Duy wrote:\n> \n> > As explained in the previous commit, current aggressive settings\n> > --depth=250 --window=250 could slow down repository access\n> > significantly. Notice that people usually work on recent history only,\n> > we could keep recent history more loosely packed, so that repo access\n> > is fast most of the time while the pack file remains small.\n> \n> One thing I have not seen is real-world timings showing the slowdown\n> based on --depth. Did I miss them, or are we just making assumptions\n> based on one old case from 2009 (that, AFAIK does not have real numbers,\n> just speculation)? Has anyone measured the effect of bumping the delta\n> cache size (and its hash implementation)?\n\nJust as a very quick, rough data point, here are before-and-after\ntimings for the patch below doing \"git rev-list --objects --all\" on my\nlinux.git, which is a mix of \"--aggressive\" and normal packing (I didn't\ndo a \"repack -f\", but it's partially what I've downloaded from k.org and\nwhat I've repacked in various experiments over the past few months).\n\n  [before]\n  real    0m28.824s\n  user    0m28.620s\n  sys     0m0.232s\n\n  [after]\n  real    0m21.694s\n  user    0m21.544s\n  sys     0m0.172s\n\nThe numbers below are completely pulled out of a hat, so we can perhaps\ndo even better. But I think it shows that there is room for improvement\nin the delta base cache.\n\n---\ndiff --git a/environment.c b/environment.c\nindex c3c8606..73ed670 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -37,7 +37,7 @@ int core_compression_seen;\n int fsync_object_files;\n size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;\n size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;\n-size_t delta_base_cache_limit = 16 * 1024 * 1024;\n+size_t delta_base_cache_limit = 128 * 1024 * 1024;\n unsigned long big_file_threshold = 512 * 1024 * 1024;\n const char *pager_program;\n int pager_use_color = 1;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex b37c6f6..a9ab8e3 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1944,7 +1944,7 @@ static void *unpack_compressed_entry(struct packed_git *p,\n \treturn buffer;\n }\n \n-#define MAX_DELTA_CACHE (256)\n+#define MAX_DELTA_CACHE (1024)\n \n static size_t delta_base_cached;\n \n"},{"id":"236961","messageId":"20140318051342.GA17200@sigill.intra.peff.net","threadId":"36184","inReplyTo":"CACsJy8CU3JyL74OuurjDr5-FHmBOCRahtXukukCqGx1cTdjvMQ@mail.gmail.com","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-18T05:13:43Z","receivedAt":"2014-03-18T05:13:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 18, 2014 at 12:00:48PM +0700, Duy Nguyen wrote:\n\n> On Tue, Mar 18, 2014 at 11:50 AM, Jeff King <peff@peff.net> wrote:\n> > On Sun, Mar 16, 2014 at 08:35:04PM +0700, Nguyễn Thái Ngọc Duy wrote:\n> >\n> >> As explained in the previous commit, current aggressive settings\n> >> --depth=250 --window=250 could slow down repository access\n> >> significantly. Notice that people usually work on recent history only,\n> >> we could keep recent history more loosely packed, so that repo access\n> >> is fast most of the time while the pack file remains small.\n> >\n> > One thing I have not seen is real-world timings showing the slowdown\n> > based on --depth. Did I miss them, or are we just making assumptions\n> > based on one old case from 2009 (that, AFAIK does not have real numbers,\n> > just speculation)? Has anyone measured the effect of bumping the delta\n> > cache size (and its hash implementation)?\n> \n> David tested it with git-blame [1]. I should probably run some tests\n> too (I don't remember if I tested some operations last time).\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/242277/focus=242435\n\nAh, thanks. I do remember that thread now.\n\nIt looks like David's last word is that he gets a significant\nperformance from bumping the delta base cache size (and number of\nbuckets). And that matches the timings I just did. I suspect there are\nstill pathological cases that could behave worse, but it really sounds\nlike we should be looking into improving that cache as a first step.\n\n-Peff\n"},{"id":"236962","messageId":"CACsJy8DZsH_2CpaUUpc5xyunHD42CN24m6Mb+9vsKyaRqJ6q3w@mail.gmail.com","threadId":"36184","inReplyTo":"20140318050727.GA14769@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-18T05:16:04Z","receivedAt":"2014-03-18T05:16:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 18, 2014 at 12:07 PM, Jeff King <peff@peff.net> wrote:\n>   [before]\n>   real    0m28.824s\n>   user    0m28.620s\n>   sys     0m0.232s\n>\n>   [after]\n>   real    0m21.694s\n>   user    0m21.544s\n>   sys     0m0.172s\n>\n> The numbers below are completely pulled out of a hat, so we can perhaps\n> do even better. But I think it shows that there is room for improvement\n> in the delta base cache.\n\nThere is definitely room for improvement in delta base cache:\nincreasing cache size, better eviction strategies.. But I think it's\northogonal to gc --aggressive improvement. From what Linus described\n---aggressive is more for archival purposes but I think people have\nbeen using it for upstream repos as well. Better packed repos could\nspeed up current clients immediately (although this argument is\nsomewhat flawed, server updates are usually at slower pace than\nclients..).\n-- \nDuy\n"},{"id":"236965","messageId":"87pplkvxoq.fsf@fencepost.gnu.org","threadId":"36184","inReplyTo":"20140318051342.GA17200@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-18T06:16:21Z","receivedAt":"2014-03-18T06:16:21Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Mar 18, 2014 at 12:00:48PM +0700, Duy Nguyen wrote:\n>\n>> On Tue, Mar 18, 2014 at 11:50 AM, Jeff King <peff@peff.net> wrote:\n>> > On Sun, Mar 16, 2014 at 08:35:04PM +0700, Nguyễn Thái Ngọc Duy wrote:\n>> >\n>> >> As explained in the previous commit, current aggressive settings\n>> >> --depth=250 --window=250 could slow down repository access\n>> >> significantly. Notice that people usually work on recent history only,\n>> >> we could keep recent history more loosely packed, so that repo access\n>> >> is fast most of the time while the pack file remains small.\n>> >\n>> > One thing I have not seen is real-world timings showing the slowdown\n>> > based on --depth. Did I miss them, or are we just making assumptions\n>> > based on one old case from 2009 (that, AFAIK does not have real numbers,\n>> > just speculation)? Has anyone measured the effect of bumping the delta\n>> > cache size (and its hash implementation)?\n>> \n>> David tested it with git-blame [1]. I should probably run some tests\n>> too (I don't remember if I tested some operations last time).\n>> \n>> http://thread.gmane.org/gmane.comp.version-control.git/242277/focus=242435\n>\n> Ah, thanks. I do remember that thread now.\n>\n> It looks like David's last word is that he gets a significant\n> performance from bumping the delta base cache size (and number of\n> buckets).\n\nIncreasing number of buckets was having comparatively minor effects\n(that was the suggestion I started with), actually _degrading_\nperformance rather soon.  The delta base cache size was much more\nnoticeable.  I had prepared a patch serious increasing it.  The reason\nI have not submitted it yet is that I have not found a compelling\nreal-world test case _apart_ from the fast git-blame that is still\nmissing implementation of -M and -C options.\n\nThere should be other commands digging through large amounts of old\nhistory, but I did not really find something benchmarking convincingly.\nEither most stuff is inefficient anyway, or the access order is\nbetter-behaved, causing fewer unwanted cache flushes.\n\nAccess order in the optimized git-blame case is basically done with a\nreverse commit-time based priority queue leading to a breadth-first\nstrategy.  It still beats unsorted access solidly in its timing.  Don't\nthink I compared depth-first results (inversing the priority queue\nsorting condition) with regard to cache results, but it's bad for\ninteractive use as it tends to leave some recent history unblamed for a\nlong time while digging up stuff in the remote past.\n\nModerate cache size increases seem like a better strategy, and the\ndefault size of 16M does not make a lot of sense with modern computers.\nIn particular since the history digging is rarely competing with other\nmemory intensive operations at the same time.\n\n> And that matches the timings I just did. I suspect there are still\n> pathological cases that could behave worse, but it really sounds like\n> we should be looking into improving that cache as a first step.\n\nI can put up a patch.  My git-blame experiments used 128M, and the patch\nproposes a more conservative 64M.  I don't actually have made\nexperiments for the 64M setting, though.  The current default is 16M.\n\n-- \nDavid Kastrup\n"},{"id":"236964","messageId":"87lhw8vxj4.fsf@fencepost.gnu.org","threadId":"36184","inReplyTo":"20140318050727.GA14769@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-18T06:19:43Z","receivedAt":"2014-03-18T06:19:43Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Mar 18, 2014 at 12:50:50AM -0400, Jeff King wrote:\n>\n>> On Sun, Mar 16, 2014 at 08:35:04PM +0700, Nguyễn Thái Ngọc Duy wrote:\n>> \n>> > As explained in the previous commit, current aggressive settings\n>> > --depth=250 --window=250 could slow down repository access\n>> > significantly. Notice that people usually work on recent history only,\n>> > we could keep recent history more loosely packed, so that repo access\n>> > is fast most of the time while the pack file remains small.\n>> \n>> One thing I have not seen is real-world timings showing the slowdown\n>> based on --depth. Did I miss them, or are we just making assumptions\n>> based on one old case from 2009 (that, AFAIK does not have real numbers,\n>> just speculation)? Has anyone measured the effect of bumping the delta\n>> cache size (and its hash implementation)?\n>\n> Just as a very quick, rough data point, here are before-and-after\n> timings for the patch below doing \"git rev-list --objects --all\" on my\n> linux.git, which is a mix of \"--aggressive\" and normal packing (I didn't\n> do a \"repack -f\", but it's partially what I've downloaded from k.org and\n> what I've repacked in various experiments over the past few months).\n>\n>   [before]\n>   real    0m28.824s\n>   user    0m28.620s\n>   sys     0m0.232s\n>\n>   [after]\n>   real    0m21.694s\n>   user    0m21.544s\n>   sys     0m0.172s\n>\n> The numbers below are completely pulled out of a hat, so we can perhaps\n> do even better. But I think it shows that there is room for improvement\n> in the delta base cache.\n>\n> ---\n> diff --git a/environment.c b/environment.c\n> index c3c8606..73ed670 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -37,7 +37,7 @@ int core_compression_seen;\n>  int fsync_object_files;\n>  size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;\n>  size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;\n> -size_t delta_base_cache_limit = 16 * 1024 * 1024;\n> +size_t delta_base_cache_limit = 128 * 1024 * 1024;\n\nYou need to change a file in Documentation as well.  Can offer a patch.\n\n>  unsigned long big_file_threshold = 512 * 1024 * 1024;\n>  const char *pager_program;\n>  int pager_use_color = 1;\n> diff --git a/sha1_file.c b/sha1_file.c\n> index b37c6f6..a9ab8e3 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -1944,7 +1944,7 @@ static void *unpack_compressed_entry(struct packed_git *p,\n>  \treturn buffer;\n>  }\n>  \n> -#define MAX_DELTA_CACHE (256)\n> +#define MAX_DELTA_CACHE (1024)\n\nThis one really needs experimentation.  I found that increases here lead\nto performance degradation rather soon, probably because of decreased\nmemory locality without significant reduction in cache collisions.  Not\nsure whether it's worth touching at all.\n\n-- \nDavid Kastrup\n"},{"id":"236966","messageId":"CACsJy8BhqiZcJY2=VWzu=ENGbbigWAQXQh3fYxLEtUxDd=8Z+Q@mail.gmail.com","threadId":"36184","inReplyTo":"CACsJy8DZsH_2CpaUUpc5xyunHD42CN24m6Mb+9vsKyaRqJ6q3w@mail.gmail.com","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-18T06:19:54Z","receivedAt":"2014-03-18T06:19:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 18, 2014 at 12:16 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n> But I think it's orthogonal to gc --aggressive improvement.\n\nThere's another reason that improving gc may be a good idea (or not).\nIt depends on how other git implementations handle long delta chains.\nIf they hate long delta chains like current git too, then more reason\nto make gc less aggressive.\n-- \nDuy\n"},{"id":"236967","messageId":"87d2hkvtvd.fsf@fencepost.gnu.org","threadId":"36184","inReplyTo":"CACsJy8BhqiZcJY2=VWzu=ENGbbigWAQXQh3fYxLEtUxDd=8Z+Q@mail.gmail.com","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-18T07:38:46Z","receivedAt":"2014-03-18T07:38:46Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> On Tue, Mar 18, 2014 at 12:16 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n>> But I think it's orthogonal to gc --aggressive improvement.\n>\n> There's another reason that improving gc may be a good idea (or not).\n> It depends on how other git implementations handle long delta chains.\n\n\"Other git implementations\" including current installations that have a\nhalf-life of at last a year.\n\n-- \nDavid Kastrup\n"},{"id":"237059","messageId":"CACsJy8BVmc0K5Rdr_putTQM5Hs2agizyW8cxAPFLb0LnXntG7Q@mail.gmail.com","threadId":"36184","inReplyTo":"CACsJy8CU3JyL74OuurjDr5-FHmBOCRahtXukukCqGx1cTdjvMQ@mail.gmail.com","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-19T11:03:41Z","receivedAt":"2014-03-19T11:03:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 18, 2014 at 12:00 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n>>>             size  time\n>>> old aggr.   36MB  5m51\n>>> new aggr.   37MB  6m13\n>>> repack -adf 48MB  1m12\n>>\n>> I am not clear on what these times mean. It looks like the new code is\n>> slower _and_ bigger. Can you explain them?\n>\n> That's right :) The upside is faster operations, which is complely\n> missed here. The good thing from those numbers is pack size does not\n> increase much (the upper limit would be repack -adf with default\n> settings).\n\nSomething is not right. The performance numbers are against me. Still looking..\n-- \nDuy\n"},{"id":"237137","messageId":"CACsJy8AoyWbeUVjha7iiPPB8BA1k0L4ZQGs2WMb5yZwc0NZr2Q@mail.gmail.com","threadId":"36184","inReplyTo":"CALbm-EbZSuzynXoUNEifP=Ga_mj6Fp9L9Do-mxhRdMvUEfogig@mail.gmail.com","subject":"Re: [PATCH 4/4] gc --aggressive: three phase repacking","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-20T01:31:07Z","receivedAt":"2014-03-20T01:31:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 19, 2014 at 9:14 PM, Stefan Beller <stefanbeller@gmail.com> wrote:\n> Maybe we should introduce another option --pack-for-archive\n> which features the characteristics of the old --aggressive.\n> And the new --aggressive would be a tradeoff between space and\n> time?\n> Thinking further we could add a couple of options there like\n>  --developer-friendly which makes blaming fast\n>  --hosting-friendly which makes it most efficient when upstream\n>  --archival-friendly (the old --aggressive)\n>  --...\n\n--aggressive-mode=<mode> would be a good option to select those\nfriendly modes instead of a bunch of new options. Well, maby \"gc\n--mode\" is enough, we already have two modes: normal and aggressive\n(or although maybe it should be renamed archive).\n-- \nDuy\n"}]}