{"thread":{"id":"35055","subject":"[PATCH] add: add --bulk to index all objects into a pack file","startedAt":"2013-10-03T04:00:11Z","lastAt":"2013-10-04T12:38:57Z","messageCount":7,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen","Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"228530","messageId":"1380772811-15415-1-git-send-email-pclouds@gmail.com","threadId":"35055","inReplyTo":null,"subject":"[PATCH] add: add --bulk to index all objects into a pack file","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-03T04:00:11Z","receivedAt":"2013-10-03T04:00:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The use case is\n\n tar -xzf bigproject.tar.gz\n cd bigproject\n git init\n git add .\n # git grep or something\n\nThe first add will generate a bunch of loose objects. With --bulk, all\nof them are forced into a single pack instead, less clutter on disk\nand maybe faster object access.\n\nThis is the equivalent of \"git -c core.bigFileThreshold=0 add .\" so\nit's not really a new functionality. Just some convenient and public\nexposure.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Interestingly index_stream() seems a bit slower than standard\n index_core(). Perhaps mmap() is faster than a series of read() for\n small files. Room for improvement later.\n\n Documentation/git-add.txt | 6 ++++++\n builtin/add.c | 10 +++++++++-\n 2 files changed, 15 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 48754cb..36a77f6 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -160,6 +160,12 @@ today's \"git add ...\", ignoring removed files.\n \tbe ignored, no matter if they are already present in the work\n \ttree or not.\n \n+--bulk::\n+\tNormally new objects are indexed and stored in loose format,\n+\tone file per new object in \"$GIT_DIR/objects\". This option\n+\tforces putting all objects into a single new pack. This may\n+\tbe useful when you need to add a lot of files initially.\n+\n \\--::\n \tThis option can be used to separate command-line options from\n \tthe list of files, (useful when filenames might be mistaken\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 226f758..40cbb71 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -336,7 +336,7 @@ static struct lock_file lock_file;\n static const char ignore_error[] =\n N_(\"The following paths are ignored by one of your .gitignore files:\\n\");\n \n-static int verbose, show_only, ignored_too, refresh_only;\n+static int verbose, show_only, ignored_too, refresh_only, bulk_index;\n static int ignore_add_errors, intent_to_add, ignore_missing;\n \n #define ADDREMOVE_DEFAULT 0 /* Change to 1 in Git 2.0 */\n@@ -368,6 +368,7 @@ static struct option builtin_add_options[] = {\n \tOPT_BOOL( 0 , \"refresh\", &refresh_only, N_(\"don't add, only refresh the index\")),\n \tOPT_BOOL( 0 , \"ignore-errors\", &ignore_add_errors, N_(\"just skip files which cannot be added because of errors\")),\n \tOPT_BOOL( 0 , \"ignore-missing\", &ignore_missing, N_(\"check if - even missing - files are ignored in dry run\")),\n+\tOPT_BOOL( 0 , \"bulk\", &bulk_index, N_(\"pack all objects instead of creating loose ones\")),\n \tOPT_END(),\n };\n \n@@ -560,6 +561,13 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tfree(seen);\n \t}\n \n+\tif (bulk_index)\n+\t\t/*\n+\t\t * Pretend all blobs are \"large\" files, forcing them\n+\t\t * all into a pack\n+\t\t */\n+\t\tbig_file_threshold = 0;\n+\n \tplug_bulk_checkin();\n \n \tif ((flags & ADD_CACHE_IMPLICIT_DOT) && prefix) {\n-- \n1.8.2.82.gc24b958\n"},{"id":"228531","messageId":"xmqqsiwin9b2.fsf@gitster.dls.corp.google.com","threadId":"35055","inReplyTo":"1380772811-15415-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] add: add --bulk to index all objects into a pack file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-03T06:43:45Z","receivedAt":"2013-10-03T06:43:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> The use case is\n>\n> tar -xzf bigproject.tar.gz\n> cd bigproject\n> git init\n> git add .\n> # git grep or something\n\nTwo obvious thoughts, and a half.\n\n (1) This particular invocation of \"git add\" can easily detect that\n it is run in a repository with no $GIT_INDEX_FILE yet, which is\n the most typical case for a big initial import. It could even\n ask if the current branch is unborn if you wanted to make the\n heuristic more specific to this use case. Perhaps it would\n make sense to automatically plug the bulk import machinery in\n such a case without an option?\n\n (2) Imagine performing a dry-run of update_files_in_cache() using a\n different diff-files callback that is similar to the\n update_callback() but that uses the lstat(2) data to see how\n big an import this really is, instead of calling\n add_file_to_index(), before actually registering the data to\n the object database. If you benchmark to see how expensive it\n is, you may find that such a scheme might be a workable\n auto-tuning mechanism to trigger this. Even if it were\n moderately expensive, when combined with the heuristics above\n for (1), it might be a worthwhile thing to do only when it is\n likely to be an initial import.\n\n (3) Is it always a good idea to send everything to a packfile on a\n large addition, or are you often better off importing the\n initial fileset as loose objects? If the latter, then the\n option name \"--bulk\" may give users a wrong hint \"if you are\n doing a bulk-import, you are bettern off using this option\".\n\nThis is a very logical extension to what was started at 568508e7\n(bulk-checkin: replace fast-import based implementation,\n2011-10-28), and I like it. I suspect \"--bulk=\" might\nbe a better alternative than setting the threshold unconditionally\nto zero, though.\n"},{"id":"228533","messageId":"CACsJy8AYAaXosV_T-JF8HvObVun_=q0UXPB8e8peHDxyvFG+RA@mail.gmail.com","threadId":"35055","inReplyTo":"xmqqsiwin9b2.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] add: add --bulk to index all objects into a pack file","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-03T12:26:35Z","receivedAt":"2013-10-03T12:26:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Oct 3, 2013 at 1:43 PM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> The use case is\n>>\n>> tar -xzf bigproject.tar.gz\n>> cd bigproject\n>> git init\n>> git add .\n>> # git grep or something\n>\n> Two obvious thoughts, and a half.\n>\n> (1) This particular invocation of \"git add\" can easily detect that\n> it is run in a repository with no $GIT_INDEX_FILE yet, which is\n> the most typical case for a big initial import. It could even\n> ask if the current branch is unborn if you wanted to make the\n> heuristic more specific to this use case. Perhaps it would\n> make sense to automatically plug the bulk import machinery in\n> such a case without an option?\n\nYeah! I did not even think of that.\n\n> (2) Imagine performing a dry-run of update_files_in_cache() using a\n> different diff-files callback that is similar to the\n> update_callback() but that uses the lstat(2) data to see how\n> big an import this really is, instead of calling\n> add_file_to_index(), before actually registering the data to\n> the object database. If you benchmark to see how expensive it\n> is, you may find that such a scheme might be a workable\n> auto-tuning mechanism to trigger this. Even if it were\n> moderately expensive, when combined with the heuristics above\n> for (1), it might be a worthwhile thing to do only when it is\n> likely to be an initial import.\n\nWe do a lot of lstats nowadays for refreshing index so yeah it's\nlikely reasonably cheap, but I doubt people do mass update on existing\nfiles often. Adding a large amount of new files (even when .git/index\nexists) may be a better indication of an import and we already have\nthat information from fill_directory().\n\nFor the no .git/index case, packing with bulk-checkin probably\nproduces a reasonably good pack because they don't delta well anyway.\nThere are no previous versions to delta against. They can delta\nagainst other files but I don't think we'll have good compression with\nthat. For the case where .git/index exists, we may intefere with \"git\ngc --auto\". We create a less optimal pack, but it's a pack and may\ndelay gc time longer..\n\n> (3) Is it always a good idea to send everything to a packfile on a\n> large addition, or are you often better off importing the\n> initial fileset as loose objects? If the latter, then the\n> option name \"--bulk\" may give users a wrong hint \"if you are\n> doing a bulk-import, you are bettern off using this option\".\n\nHard question. Fewer files are definitely a good thing, for example\nwhen you \"rm -rf\" the whole thing :-) Disk usage is another. On\ngdb-7.3.1, \"du -sh\" reports .git with loose objects takes 57M, while\nthe packed one takes 29M. Disk access is slightly faster on packed\n.git, at least for \"git grep --cached\": 0.71s vs 0.83s.\n\n> This is a very logical extension to what was started at 568508e7\n> (bulk-checkin: replace fast-import based implementation,\n> 2011-10-28), and I like it. I suspect \"--bulk=\" might\n> be a better alternative than setting the threshold unconditionally\n> to zero, though.\n\nThe threshold may be better in form of a config setting because I\nwon't want to specify that every time. But does one really want to\nkeep some small files around in loose format? I don't see it because\nmy goal is to keep a clean .git, but maybe loose format has some\nadvantages..\n-- \nDuy\n"},{"id":"228548","messageId":"1380869871-31631-1-git-send-email-pclouds@gmail.com","threadId":"35055","inReplyTo":"1380772811-15415-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2] add: add --bulk to index all objects into a pack file","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-04T06:57:51Z","receivedAt":"2013-10-04T06:57:51Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The use case is\n\n tar -xzf bigproject.tar.gz\n cd bigproject\n git init\n git add .\n # git grep or something\n\nThe first add will generate a bunch of loose objects. With --bulk, all\nof them are forced into a single pack instead, less clutter on disk\nand maybe faster object access.\n\nOn gdb-7.5.1 source directory, the loose .git directory takes 66M\naccording to `du` while the packed one takes 32M. Timing of\n\"git grep --cached\":\n\n loose packed\nreal 0m1.671s 0m1.372s\nuser 0m1.542s 0m1.313s\nsys 0m0.126s 0m0.056s\n\nIt's not an all-win situation though. --bulk is slower than --no-bulk\nbecause:\n\n - Triple hashing: we need to calculate both object SHA-1s _and_ pack\n SHA-1. At the end we have to fix up the pack, which means rehashing\n the entire pack again. --no-bulk only cares about object SHA-1s.\n\n - We write duplicate objects to the pack then truncate it, because we\n don't know if it's a duplicate until we're done writing, and cannot\n keep it in core because it's potentially big. So extra I/O (but\n hopefully not too much because duplicate objects should not happen\n often).\n\n - Sort and write .idx file.\n\n - (For the future) --no-bulk could benefit from multithreading\n because this is CPU bound operation. --bulk could not.\n\nBut again this comparison is not fair, --bulk is closer to:\n\n git add . &&\n git ls-files --stage | awk '{print $2;}'| \\\n git pack-objects .git/objects/pack-\n\nexcept that it does not deltifies nor sort objects.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n v2 examines pros and cons of --bulk and I'm not sure if turning it on\n automatically (with heuristics) is a good idea anymore.\n\n Oh and it fixes not packing empty files.\n\n Documentation/git-add.txt | 10 ++++++++++\n builtin/add.c | 10 +++++++++-\n sha1_file.c | 3 ++-\n 3 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 48754cb..147d191 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -160,6 +160,16 @@ today's \"git add ...\", ignoring removed files.\n \tbe ignored, no matter if they are already present in the work\n \ttree or not.\n \n+--bulk::\n+\tNormally new objects are indexed and stored in loose format,\n+\tone file per new object in \"$GIT_DIR/objects\". This option\n+\tforces putting all objects into a single new pack. This may\n+\tbe useful when you need to add a lot of files initially.\n++\n+This option is equivalent to running `git -c core.bigFileThreshold=0 add`.\n+If you want to only pack files larger than a size threshold, use the\n+long form.\n+\n \\--::\n \tThis option can be used to separate command-line options from\n \tthe list of files, (useful when filenames might be mistaken\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 226f758..40cbb71 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -336,7 +336,7 @@ static struct lock_file lock_file;\n static const char ignore_error[] =\n N_(\"The following paths are ignored by one of your .gitignore files:\\n\");\n \n-static int verbose, show_only, ignored_too, refresh_only;\n+static int verbose, show_only, ignored_too, refresh_only, bulk_index;\n static int ignore_add_errors, intent_to_add, ignore_missing;\n \n #define ADDREMOVE_DEFAULT 0 /* Change to 1 in Git 2.0 */\n@@ -368,6 +368,7 @@ static struct option builtin_add_options[] = {\n \tOPT_BOOL( 0 , \"refresh\", &refresh_only, N_(\"don't add, only refresh the index\")),\n \tOPT_BOOL( 0 , \"ignore-errors\", &ignore_add_errors, N_(\"just skip files which cannot be added because of errors\")),\n \tOPT_BOOL( 0 , \"ignore-missing\", &ignore_missing, N_(\"check if - even missing - files are ignored in dry run\")),\n+\tOPT_BOOL( 0 , \"bulk\", &bulk_index, N_(\"pack all objects instead of creating loose ones\")),\n \tOPT_END(),\n };\n \n@@ -560,6 +561,13 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tfree(seen);\n \t}\n \n+\tif (bulk_index)\n+\t\t/*\n+\t\t * Pretend all blobs are \"large\" files, forcing them\n+\t\t * all into a pack\n+\t\t */\n+\t\tbig_file_threshold = 0;\n+\n \tplug_bulk_checkin();\n \n \tif ((flags & ADD_CACHE_IMPLICIT_DOT) && prefix) {\ndiff --git a/sha1_file.c b/sha1_file.c\nindex f80bbe4..8b66840 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -3137,7 +3137,8 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st,\n \n \tif (!S_ISREG(st->st_mode))\n \t\tret = index_pipe(sha1, fd, type, path, flags);\n-\telse if (size <= big_file_threshold || type != OBJ_BLOB ||\n+\telse if ((big_file_threshold && size <= big_file_threshold) ||\n+\t\t type != OBJ_BLOB ||\n \t\t (path && would_convert_to_git(path, NULL, 0, 0)))\n \t\tret = index_core(sha1, fd, size, type, path, flags);\n \telse\n-- \n1.8.2.82.gc24b958\n"},{"id":"228549","messageId":"vpqfvshecjg.fsf@anie.imag.fr","threadId":"35055","inReplyTo":"1380869871-31631-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2] add: add --bulk to index all objects into a pack file","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-10-04T07:10:59Z","receivedAt":"2013-10-04T07:10:59Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> except that it does not deltifies nor sort objects.\n\nI think this should be mentionned in the doc. Otherwise, it seems like\n\"git add --bulk\" is like \"git add && git repack\".\n\nBTW, will the next \"git gc\" be efficient after a \"add --bulk\"? I mean:\nwill it consider the pack as \"already pack\" and let it as-is, without\ndeltification, or will it get a chance to actually repack efficiently?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"228550","messageId":"CACsJy8DOziJ9z2p-2BttJf=zZUC6GsRVtKru3R6Kn32-B0tAdA@mail.gmail.com","threadId":"35055","inReplyTo":"vpqfvshecjg.fsf@anie.imag.fr","subject":"Re: [PATCH v2] add: add --bulk to index all objects into a pack file","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-04T07:19:45Z","receivedAt":"2013-10-04T07:19:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Oct 4, 2013 at 2:10 PM, Matthieu Moy\n wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> except that it does not deltifies nor sort objects.\n>\n> I think this should be mentionned in the doc. Otherwise, it seems like\n> \"git add --bulk\" is like \"git add && git repack\".\n\nYep. Will do.\n\n> BTW, will the next \"git gc\" be efficient after a \"add --bulk\"? I mean:\n> will it consider the pack as \"already pack\" and let it as-is, without\n> deltification, or will it get a chance to actually repack efficiently?\n\ngc does \"repack -A\" so all separate packs will be merged. It may delay\ngc time though because it'll take more time to hit the loose object\nlimit. I think pack-objects will try to deltify so we're good. If we\nproduce bad deltas in --bulk then that's another story because\npack-objects will try to blindly reuse them.\n-- \nDuy\n"},{"id":"228556","messageId":"CACsJy8BLyOtWck1ZidnBZo+KuBiSa5a4mZtbG6LQxTnv3sQXQw@mail.gmail.com","threadId":"35055","inReplyTo":"1380869871-31631-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2] add: add --bulk to index all objects into a pack file","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-04T12:38:57Z","receivedAt":"2013-10-04T12:38:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Oct 4, 2013 at 1:57 PM, Nguyễn Thái Ngọc Duy wrote:\n> It's not an all-win situation though. --bulk is slower than --no-bulk\n> because:\n>\n> - Triple hashing: we need to calculate both object SHA-1s _and_ pack\n> SHA-1. At the end we have to fix up the pack, which means rehashing\n> the entire pack again. --no-bulk only cares about object SHA-1s.\n\nOops, it's quadruple hashing: one for object sha-1 (1), one for pack\nsha-1 while writing pack (2), two for pack header fixup: one after the\nthe header fixup (3), and one repeating the same (2) just to verify\nthat it matches (2) exactly (4). Do we have to be this paranoid? I\nthink we could drop (2) and (4) in this case.\n-- \nDuy\n"}]}