{"thread":{"id":"58673","subject":"[PATCH] pack-objects: introduce --exclude-delta=<pattern> option","startedAt":"2022-10-22T15:46:14Z","lastAt":"2022-10-22T17:00:50Z","messageCount":2,"participants":["ZheNing Hu via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"465565","messageId":"pull.1392.git.1666453564661.gitgitgadget@gmail.com","threadId":"58673","inReplyTo":null,"subject":"[PATCH] pack-objects: introduce --exclude-delta=<pattern> option","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-22T15:46:04Z","receivedAt":"2022-10-22T15:46:14Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\nThe server uses delta compression during git clone to reduce\nthe amount of data transferred over the network, but delta\ncompression for large binary blobs often does not reduce\nstorage size significantly and wastes a lot of CPU. Git now\ndisables delta compression for objects that meet these conditions:\n\n1. files that have -delta set in .gitattributes\n2. files that its size exceed the big_file_threshold\n\nHowever, in 1, .gitattributes needs to be set manually by the user,\nand in most cases the user does not actively set it, and it is not\nsomething that can be actively adjusted on the server aside. In 2,\nthe big_file_threshold now defaults to 512MB, and many binary files\nsmaller than that will be uselessly delta-compressed, and this is\nmade worse if the server actively increases the big_file_threshold.\n\nTherefore, we need a way to be able to actively skip the delta\ncompression of some files on the server. Introduces the\n`-exclude-delta=<pattern>` option, which can be used to disable delta\ncompression for objects that satisfy the pattern.\n\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n    pack-objects: introduce --exclude-delta= option\n    \n    While analyzing some repositories using git filter-repo -analyze, I\n    noticed that many huge binaries in the repositories were\n    delta-compressed without much reduction in size.\n    \n    $ cat .git/filter-repo/analysis/path-all-sizes.txt | more === All paths\n    by reverse accumulated size === Format: unpacked size, packed size, date\n    deleted, path name 23816778 23765921 2022-08-22\n    managed/src/universal/ybc/ybc-1.0.0-b1-linux-x86_64.tar.gz 22504398\n    22445676 2022-08-22\n    managed/src/universal/ybc/ybc-1.0.0-b1-el8-aarch64.tar.gz 11726471\n    6424233 2022-08-09 managed/yba-installer/yba-installer_linux_amd64\n    294644800 5794201 src/yb/master/catalog_manager.cc 2912780 2872186\n    docs/static/images/yp/tables-view-ycql.png 2992192 2634232\n    docs/static/images/yb-cloud/cloud-clusters-backups.png 2757095 2501915\n    docs/static/images/deploy/aws/aws-cf-configure-options.png ...\n    \n    The current solution to avoid delta compression is not very suitable for\n    git servers. First, files that exceed the big_file_threshold are not\n    delta compressed, but the above analysis indicates that many big binary\n    files do not exceed the the big_file_threshold (default to 512MB).\n    Second, there is not .gitattrbutes to disable delta compression for\n    them, we also don't really can let repo administrators add it manually.\n    \n    But we can also see that the large files in these repositories often\n    have some common characteristics: they end in \".tar.gz\"or “.png\". So\n    perhaps we can take advantage of this feature and disable delta\n    compression on the server for some common type binary files.\n    \n    This is currently implemented by command line parameters\n    --exclude-delta=<pattern>. But maybe we can also try passing it through\n    git config.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1392%2Fadlternative%2Fadl%2Fpack-object-no-try-delta-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1392/adlternative/adl/pack-object-no-try-delta-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1392\n\n Documentation/git-pack-objects.txt |  6 +++++-\n builtin/pack-objects.c             | 28 +++++++++++++++++++++++++++-\n 2 files changed, 32 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex a9995a932ca..92cfee83df5 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n \t[--no-reuse-delta] [--delta-base-offset] [--non-empty]\n \t[--local] [--incremental] [--window=<n>] [--depth=<n>]\n \t[--revs [--unpacked | --all]] [--keep-pack=<pack-name>]\n-\t[--cruft] [--cruft-expiration=<time>]\n+\t[--cruft] [--cruft-expiration=<time>] [--exclude-delta=<file>]\n \t[--stdout [--filter=<filter-spec>] | <base-name>]\n \t[--shallow] [--keep-true-parents] [--[no-]sparse] < <object-list>\n \n@@ -221,6 +221,10 @@ depth is 4095.\n \tThis flag tells the command not to reuse existing deltas\n \tbut compute them from scratch.\n \n+--exclude-delta=<pattern>::\n+\tDelta compression will not be attempted for blobs for paths\n+\tmatching pattern. See linkgit:gitignore[5] for pattern details.\n+\n --no-reuse-object::\n \tThis flag tells the command not to reuse existing object data at all,\n \tincluding non deltified object, forcing recompression of everything.\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 3658c05cafc..ab9cff98e3a 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -272,6 +272,8 @@ static struct commit **indexed_commits;\n static unsigned int indexed_commits_nr;\n static unsigned int indexed_commits_alloc;\n \n+static struct pattern_list *exclude_delta_patterns;\n+\n static void index_commit_for_bitmap(struct commit *commit)\n {\n \tif (indexed_commits_nr >= indexed_commits_alloc) {\n@@ -1315,13 +1317,20 @@ static void write_pack_file(void)\n static int no_try_delta(const char *path)\n {\n \tstatic struct attr_check *check;\n+\tint dtype;\n \n \tif (!check)\n \t\tcheck = attr_check_initl(\"delta\", NULL);\n \tgit_check_attr(the_repository->index, path, check);\n \tif (ATTR_FALSE(check->items[0].value))\n \t\treturn 1;\n-\treturn 0;\n+\n+\treturn exclude_delta_patterns &&\n+\t\tpath_matches_pattern_list(path,\n+\t\t\t\t\t  strlen(path),\n+\t\t\t\t\t  path, &dtype,\n+\t\t\t\t\t  exclude_delta_patterns,\n+\t\t\t\t\t  the_repository->index) == MATCHED;\n }\n \n /*\n@@ -4149,6 +4158,19 @@ static int option_parse_cruft_expiration(const struct option *opt,\n \treturn 0;\n }\n \n+static int option_parse_exclude_delta(const struct option *opt,\n+\t\t\t\t\t const char *arg, int unset)\n+{\n+\tBUG_ON_OPT_NEG(unset);\n+\n+\tif (!exclude_delta_patterns)\n+\t\texclude_delta_patterns = xcalloc(1, sizeof(*exclude_delta_patterns));\n+\n+\tif (arg)\n+\t\tadd_pattern(arg, \"\", 0, exclude_delta_patterns, 0);\n+\treturn 0;\n+}\n+\n struct po_filter_data {\n \tunsigned have_revs:1;\n \tstruct rev_info revs;\n@@ -4242,6 +4264,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tOPT_CALLBACK_F(0, \"cruft-expiration\", NULL, N_(\"time\"),\n \t\t  N_(\"expire cruft objects older than <time>\"),\n \t\t  PARSE_OPT_OPTARG, option_parse_cruft_expiration),\n+\t\tOPT_CALLBACK_F(0, \"exclude-delta\", NULL, N_(\"pattern\"),\n+\t\t  N_(\"disable delta compression for files matching pattern\"),\n+\t\t  PARSE_OPT_NONEG, option_parse_exclude_delta),\n \t\tOPT_BOOL(0, \"sparse\", &sparse,\n \t\t\t N_(\"use the sparse reachability algorithm\")),\n \t\tOPT_BOOL(0, \"thin\", &thin,\n@@ -4514,6 +4539,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \n cleanup:\n \tstrvec_clear(&rp);\n+\tFREE_AND_NULL(exclude_delta_patterns);\n \n \treturn 0;\n }\n\nbase-commit: 1fc3c0ad407008c2f71dd9ae1241d8b75f8ef886\n-- \ngitgitgadget\n"},{"id":"465568","messageId":"xmqqr0yzhjyj.fsf@gitster.g","threadId":"58673","inReplyTo":"pull.1392.git.1666453564661.gitgitgadget@gmail.com","subject":"Re: [PATCH] pack-objects: introduce --exclude-delta=<pattern> option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-22T17:00:36Z","receivedAt":"2022-10-22T17:00:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: ZheNing Hu <adlternative@gmail.com>\n>\n> The server uses delta compression during git clone to reduce\n> the amount of data transferred over the network, but delta\n> compression for large binary blobs often does not reduce\n> storage size significantly and wastes a lot of CPU. Git now\n> disables delta compression for objects that meet these conditions:\n>\n> 1. files that have -delta set in .gitattributes\n> 2. files that its size exceed the big_file_threshold\n>\n> However, in 1, .gitattributes needs to be set manually by the user,\n> and in most cases the user does not actively set it, and it is not\n> something that can be actively adjusted on the server aside. In 2,\n> the big_file_threshold now defaults to 512MB, and many binary files\n> smaller than that will be uselessly delta-compressed, and this is\n> made worse if the server actively increases the big_file_threshold.\n\nWho are you trying to help, though?  The user has to somehow\nmanually cause the --exclude-delta=<pattern> to be added at the\nserver side, so I do not see the new feature as correcting the\nweakness you perceive in the approach to mark the undeltifiable\nblobs in the attributes system at all.  Does this feature assume\nthat the server operator knows better than the project that can\nmaintain their own .gitattributes in tree?  If so, the server\noperator can already use <bare-repository>/info/attributes file\nto achieve that already, no?\n\nNow hosting sites may not give hosted projects flexibility to\nconfigure their own server side repositories, like its \"config\"\nand \"info/attributes\", but that limitation would equally apply\nwhat command line options pack-objects runs with.\n\nSo, again, it is not clear who this patch is trying to help and how.\nIf we assume that the hosting operators can give project owners more\ncontrol of how the server side is configured, then we can do that\nalready without a new option, no?\n\nIOW\n\n> Therefore, we need a way to be able to actively skip the delta\n> compression of some files on the server.\n\nI do not quite follow the \"Therefore\" here.\n\n"}]}