{"thread":{"id":"48286","subject":"[PATCH 00/15] nd/pack-objects-pack-struct update","startedAt":"2018-04-14T15:35:32Z","lastAt":"2018-04-14T19:53:37Z","messageCount":17,"participants":["Nguyễn Thái Ngọc Duy","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":15},"messages":[{"id":"344671","messageId":"20180414153513.9902-1-pclouds@gmail.com","threadId":"48286","inReplyTo":null,"subject":"[PATCH 00/15] nd/pack-objects-pack-struct update","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:34:58Z","receivedAt":"2018-04-14T15:35:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is basically a resend from the last round but rebased on latest\nmaster to take advantage of new object-store and oid changes.\nOne minor change is t/README now mentions about git-config when a\nvariable accepts a boolean value.\n\nNguyễn Thái Ngọc Duy (15):\n read-cache.c: make $GIT_TEST_SPLIT_INDEX boolean\n pack-objects: a bit of document about struct object_entry\n pack-objects: turn type and in_pack_type to bitfields\n pack-objects: use bitfield for object_entry::dfs_state\n pack-objects: use bitfield for object_entry::depth\n pack-objects: move in_pack_pos out of struct object_entry\n pack-objects: move in_pack out of struct object_entry\n pack-objects: refer to delta objects by index instead of pointer\n pack-objects: shrink z_delta_size field in struct object_entry\n pack-objects: don't check size when the object is bad\n pack-objects: clarify the use of object_entry::size\n pack-objects: shrink size field in struct object_entry\n pack-objects: shrink delta_size field in struct object_entry\n pack-objects: reorder members to shrink struct object_entry\n ci: exercise the whole test suite with uncommon code in pack-objects\n\n Documentation/config.txt | 4 +-\n Documentation/git-pack-objects.txt | 4 +-\n Documentation/git-repack.txt | 4 +-\n builtin/pack-objects.c | 364 +++++++++++++++++++----------\n cache.h | 2 +\n ci/run-build-and-tests.sh | 5 +-\n object-store.h | 1 +\n object.h | 1 -\n pack-bitmap-write.c | 14 +-\n pack-bitmap.c | 2 +-\n pack-bitmap.h | 4 +-\n pack-objects.c | 68 ++++++\n pack-objects.h | 314 +++++++++++++++++++++++--\n read-cache.c | 4 +-\n t/README | 22 ++\n t/t5300-pack-object.sh | 5 +\n 16 files changed, 654 insertions(+), 164 deletions(-)\n\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344672","messageId":"20180414153513.9902-3-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 02/15] pack-objects: a bit of document about struct object_entry","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:00Z","receivedAt":"2018-04-14T15:35:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The role of this comment block becomes more important after we shuffle\nfields around to shrink this struct. It will be much harder to see what\nfield is related to what.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n pack-objects.h | 45 +++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 45 insertions(+)\n\ndiff --git a/pack-objects.h b/pack-objects.h\nindex 03f1191659..c0a1f61aac 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -1,6 +1,51 @@\n #ifndef PACK_OBJECTS_H\n #define PACK_OBJECTS_H\n \n+/*\n+ * basic object info\n+ * -----------------\n+ * idx.oid is filled up before delta searching starts. idx.crc32 is\n+ * only valid after the object is written out and will be used for\n+ * generating the index. idx.offset will be both gradually set and\n+ * used in writing phase (base objects get offset first, then deltas\n+ * refer to them)\n+ *\n+ * \"size\" is the uncompressed object size. Compressed size of the raw\n+ * data for an object in a pack is not stored anywhere but is computed\n+ * and made available when reverse .idx is made.\n+ *\n+ * \"hash\" contains a path name hash which is used for sorting the\n+ * delta list and also during delta searching. Once prepare_pack()\n+ * returns it's no longer needed.\n+ *\n+ * source pack info\n+ * ----------------\n+ * The (in_pack, in_pack_offset) tuple contains the location of the\n+ * object in the source pack. in_pack_header_size allows quickly\n+ * skipping the header and going straight to the zlib stream.\n+ *\n+ * \"type\" and \"in_pack_type\" both describe object type. in_pack_type\n+ * may contain a delta type, while type is always the canonical type.\n+ *\n+ * deltas\n+ * ------\n+ * Delta links (delta, delta_child and delta_sibling) are created to\n+ * reflect that delta graph from the source pack then updated or added\n+ * during delta searching phase when we find better deltas.\n+ *\n+ * delta_child and delta_sibling are last needed in\n+ * compute_write_order(). \"delta\" and \"delta_size\" must remain valid\n+ * at object writing phase in case the delta is not cached.\n+ *\n+ * If a delta is cached in memory and is compressed, delta_data points\n+ * to the data and z_delta_size contains the compressed size. If it's\n+ * uncompressed [1], z_delta_size must be zero. delta_size is always\n+ * the uncompressed size and must be valid even if the delta is not\n+ * cached.\n+ *\n+ * [1] during try_delta phase we don't bother with compressing because\n+ * the delta could be quickly replaced with a better one.\n+ */\n struct object_entry {\n \tstruct pack_idx_entry idx;\n \tunsigned long size;\t/* uncompressed size */\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344673","messageId":"20180414153513.9902-2-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 01/15] read-cache.c: make $GIT_TEST_SPLIT_INDEX boolean","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:34:59Z","receivedAt":"2018-04-14T15:35:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"While at there, document about this special mode when running the test\nsuite.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n ci/run-build-and-tests.sh | 2 +-\n read-cache.c | 4 ++--\n t/README | 11 +++++++++++\n 3 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex 3735ce413f..8190a753de 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -11,7 +11,7 @@ make --jobs=2\n make --quiet test\n if test \"$jobname\" = \"linux-gcc\"\n then\n-\tGIT_TEST_SPLIT_INDEX=YesPlease make --quiet test\n+\tGIT_TEST_SPLIT_INDEX=yes make --quiet test\n fi\n \n check_unignored_build_artifacts\ndiff --git a/read-cache.c b/read-cache.c\nindex 10f1c6bb8a..fa3df2e72e 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2268,7 +2268,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \n \tif (!istate->version) {\n \t\tistate->version = get_index_format_default();\n-\t\tif (getenv(\"GIT_TEST_SPLIT_INDEX\"))\n+\t\tif (git_env_bool(\"GIT_TEST_SPLIT_INDEX\", 0))\n \t\t\tinit_split_index(istate);\n \t}\n \n@@ -2559,7 +2559,7 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \t\tgoto out;\n \t}\n \n-\tif (getenv(\"GIT_TEST_SPLIT_INDEX\")) {\n+\tif (git_env_bool(\"GIT_TEST_SPLIT_INDEX\", 0)) {\n \t\tint v = si->base_sha1[0];\n \t\tif ((v & 15) < 6)\n \t\t\tistate->cache_changed |= SPLIT_INDEX_ORDERED;\ndiff --git a/t/README b/t/README\nindex 24ddebfabf..d5e0a3c786 100644\n--- a/t/README\n+++ b/t/README\n@@ -293,6 +293,17 @@ and know what setup is needed for it. Or when you want to run\n everything up to a certain test.\n \n \n+Running tests with special setups\n+---------------------------------\n+\n+The whole test suite could be run to test some special features\n+that cannot be easily covered by a few specific test cases. These\n+could be enabled by running the test suite with correct GIT_TEST_\n+environment set.\n+\n+GIT_TEST_SPLIT_INDEX= forces split-index mode on the whole\n+test suite. Accept any boolean values that are accepted by git-config.\n+\n Naming Tests\n ------------\n \n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344674","messageId":"20180414153513.9902-5-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 04/15] pack-objects: use bitfield for object_entry::dfs_state","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:02Z","receivedAt":"2018-04-14T15:35:38Z","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 \n---\n builtin/pack-objects.c | 3 +++\n pack-objects.h | 28 +++++++++++++++++-----------\n 2 files changed, 20 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 88877f1f59..cc3c31747e 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -3049,6 +3049,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tOPT_END(),\n \t};\n \n+\tif (DFS_NUM_STATES > (1 << OE_DFS_STATE_BITS))\n+\t\tBUG(\"too many dfs states, increase OE_DFS_STATE_BITS\");\n+\n \tcheck_replace_refs = 0;\n \n \treset_pack_idx_option(&pack_idx_opts);\ndiff --git a/pack-objects.h b/pack-objects.h\nindex b4a83a6123..080ef62d31 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -1,6 +1,21 @@\n #ifndef PACK_OBJECTS_H\n #define PACK_OBJECTS_H\n \n+#define OE_DFS_STATE_BITS\t2\n+\n+/*\n+ * State flags for depth-first search used for analyzing delta cycles.\n+ *\n+ * The depth is measured in delta-links to the base (so if A is a delta\n+ * against B, then A has a depth of 1, and B a depth of 0).\n+ */\n+enum dfs_state {\n+\tDFS_NONE = 0,\n+\tDFS_ACTIVE,\n+\tDFS_DONE,\n+\tDFS_NUM_STATES\n+};\n+\n /*\n * basic object info\n * -----------------\n@@ -73,19 +88,10 @@ struct object_entry {\n \tunsigned no_try_delta:1;\n \tunsigned tagged:1; /* near the very tip of refs */\n \tunsigned filled:1; /* assigned write-order */\n+\tunsigned dfs_state:OE_DFS_STATE_BITS;\n \n-\t/*\n-\t * State flags for depth-first search used for analyzing delta cycles.\n-\t *\n-\t * The depth is measured in delta-links to the base (so if A is a delta\n-\t * against B, then A has a depth of 1, and B a depth of 0).\n-\t */\n-\tenum {\n-\t\tDFS_NONE = 0,\n-\t\tDFS_ACTIVE,\n-\t\tDFS_DONE\n-\t} dfs_state;\n \tint depth;\n+\n };\n \n struct packing_data {\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344675","messageId":"20180414153513.9902-6-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 05/15] pack-objects: use bitfield for object_entry::depth","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:03Z","receivedAt":"2018-04-14T15:35:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Because of struct packing from now on we can only handle max depth\n4095 (or even lower when new booleans are added in this struct). This\nshould be ok since long delta chain will cause significant slow down\nanyway.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/config.txt | 1 +\n Documentation/git-pack-objects.txt | 4 +++-\n Documentation/git-repack.txt | 4 +++-\n builtin/pack-objects.c | 6 ++++++\n pack-objects.h | 5 ++---\n 5 files changed, 15 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2659153cb3..d97f10722c 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2422,6 +2422,7 @@ pack.window::\n pack.depth::\n \tThe maximum delta depth used by linkgit:git-pack-objects[1] when no\n \tmaximum depth is given on the command line. Defaults to 50.\n+\tMaximum value is 4095.\n \n pack.windowMemory::\n \tThe maximum size of memory that is consumed by each thread\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 81bc490ac5..3503c9e3e6 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -96,7 +96,9 @@ base-name::\n \tit too deep affects the performance on the unpacker\n \tside, because delta data needs to be applied that many\n \ttimes to get to the necessary object.\n-\tThe default value for --window is 10 and --depth is 50.\n++\n+The default value for --window is 10 and --depth is 50. The maximum\n+depth is 4095.\n \n --window-memory=::\n \tThis option provides an additional limit on top of `--window`;\ndiff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt\nindex ae750e9e11..25c83c4927 100644\n--- a/Documentation/git-repack.txt\n+++ b/Documentation/git-repack.txt\n@@ -90,7 +90,9 @@ other objects in that pack they already have locally.\n \tspace. `--depth` limits the maximum delta depth; making it too deep\n \taffects the performance on the unpacker side, because delta data needs\n \tto be applied that many times to get to the necessary object.\n-\tThe default value for --window is 10 and --depth is 50.\n++\n+The default value for --window is 10 and --depth is 50. The maximum\n+depth is 4095.\n \n --threads=::\n \tThis option is passed through to `git pack-objects`.\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex cc3c31747e..b231e80f17 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -3068,6 +3068,12 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tif (pack_to_stdout != !base_name || argc)\n \t\tusage_with_options(pack_usage, pack_objects_options);\n \n+\tif (depth >= (1 << OE_DEPTH_BITS)) {\n+\t\twarning(_(\"delta chain depth %d is too deep, forcing %d\"),\n+\t\t\tdepth, (1 << OE_DEPTH_BITS) - 1);\n+\t\tdepth = (1 << OE_DEPTH_BITS) - 1;\n+\t}\n+\n \targv_array_push(&rp, \"pack-objects\");\n \tif (thin) {\n \t\tuse_internal_rev_list = 1;\ndiff --git a/pack-objects.h b/pack-objects.h\nindex 080ef62d31..cdce1648de 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -2,6 +2,7 @@\n #define PACK_OBJECTS_H\n \n #define OE_DFS_STATE_BITS\t2\n+#define OE_DEPTH_BITS\t\t12\n \n /*\n * State flags for depth-first search used for analyzing delta cycles.\n@@ -89,9 +90,7 @@ struct object_entry {\n \tunsigned tagged:1; /* near the very tip of refs */\n \tunsigned filled:1; /* assigned write-order */\n \tunsigned dfs_state:OE_DFS_STATE_BITS;\n-\n-\tint depth;\n-\n+\tunsigned depth:OE_DEPTH_BITS;\n };\n \n struct packing_data {\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344676","messageId":"20180414153513.9902-8-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 07/15] pack-objects: move in_pack out of struct object_entry","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:05Z","receivedAt":"2018-04-14T15:35:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Instead of using 8 bytes (on 64 bit arch) to store a pointer to a\npack. Use an index instead since the number of packs should be\nrelatively small.\n\nThis limits the number of packs we can handle to 1k. Since we can't be\nsure people can never run into the situation where they have more than\n1k pack files. Provide a fall back route for it.\n\nIf we find out they have too many packs, the new in_pack_by_idx[]\narray (which has at most 1k elements) will not be used. Instead we\nallocate in_pack[] array that holds nr_objects elements. This is\nsimilar to how the optional in_pack_pos field is handled.\n\nThe new simple test is just to make sure the too-many-packs code path\nis at least executed. The true test is running\n\n make test GIT_TEST_FULL_IN_PACK_ARRAY=1\n\nto take advantage of other special case tests.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/pack-objects.c | 28 +++++++++++-------\n object-store.h | 1 +\n pack-objects.c | 65 ++++++++++++++++++++++++++++++++++++++++++\n pack-objects.h | 38 +++++++++++++++++++++++-\n t/README | 5 ++++\n t/t5300-pack-object.sh | 5 ++++\n 6 files changed, 130 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex d9c89e87cd..2784d58ec2 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -31,6 +31,8 @@\n #include \"packfile.h\"\n #include \"object-store.h\"\n \n+#define IN_PACK(obj) oe_in_pack(&to_pack, obj)\n+\n static const char *pack_usage[] = {\n \tN_(\"git pack-objects --stdout [...] [< | < ]\"),\n \tN_(\"git pack-objects [...] [< | < ]\"),\n@@ -367,7 +369,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent\n static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,\n \t\t\t\tunsigned long limit, int usable_delta)\n {\n-\tstruct packed_git *p = entry->in_pack;\n+\tstruct packed_git *p = IN_PACK(entry);\n \tstruct pack_window *w_curs = NULL;\n \tstruct revindex_entry *revidx;\n \toff_t offset;\n@@ -478,7 +480,7 @@ static off_t write_object(struct hashfile *f,\n \n \tif (!reuse_object)\n \t\tto_reuse = 0;\t/* explicit */\n-\telse if (!entry->in_pack)\n+\telse if (!IN_PACK(entry))\n \t\tto_reuse = 0;\t/* can't reuse what we don't have */\n \telse if (oe_type(entry) == OBJ_REF_DELTA ||\n \t\t oe_type(entry) == OBJ_OFS_DELTA)\n@@ -1074,7 +1076,7 @@ static void create_object_entry(const struct object_id *oid,\n \telse\n \t\tnr_result++;\n \tif (found_pack) {\n-\t\tentry->in_pack = found_pack;\n+\t\toe_set_in_pack(&to_pack, entry, found_pack);\n \t\tentry->in_pack_offset = found_offset;\n \t}\n \n@@ -1399,8 +1401,8 @@ static void cleanup_preferred_base(void)\n \n static void check_object(struct object_entry *entry)\n {\n-\tif (entry->in_pack) {\n-\t\tstruct packed_git *p = entry->in_pack;\n+\tif (IN_PACK(entry)) {\n+\t\tstruct packed_git *p = IN_PACK(entry);\n \t\tstruct pack_window *w_curs = NULL;\n \t\tconst unsigned char *base_ref = NULL;\n \t\tstruct object_entry *base_entry;\n@@ -1535,14 +1537,16 @@ static int pack_offset_sort(const void *_a, const void *_b)\n {\n \tconst struct object_entry *a = *(struct object_entry **)_a;\n \tconst struct object_entry *b = *(struct object_entry **)_b;\n+\tconst struct packed_git *a_in_pack = IN_PACK(a);\n+\tconst struct packed_git *b_in_pack = IN_PACK(b);\n \n \t/* avoid filesystem trashing with loose objects */\n-\tif (!a->in_pack && !b->in_pack)\n+\tif (!a_in_pack && !b_in_pack)\n \t\treturn oidcmp(&a->idx.oid, &b->idx.oid);\n \n-\tif (a->in_pack < b->in_pack)\n+\tif (a_in_pack < b_in_pack)\n \t\treturn -1;\n-\tif (a->in_pack > b->in_pack)\n+\tif (a_in_pack > b_in_pack)\n \t\treturn 1;\n \treturn a->in_pack_offset < b->in_pack_offset ? -1 :\n \t\t\t(a->in_pack_offset > b->in_pack_offset);\n@@ -1578,7 +1582,7 @@ static void drop_reused_delta(struct object_entry *entry)\n \n \toi.sizep = &entry->size;\n \toi.typep = &type;\n-\tif (packed_object_info(entry->in_pack, entry->in_pack_offset, &oi) < 0) {\n+\tif (packed_object_info(IN_PACK(entry), entry->in_pack_offset, &oi) < 0) {\n \t\t/*\n \t\t * We failed to get the info from this pack for some reason;\n \t\t * fall back to sha1_object_info, which may find another copy.\n@@ -1848,8 +1852,8 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \t * it, we will still save the transfer cost, as we already know\n \t * the other side has it and we won't send src_entry at all.\n \t */\n-\tif (reuse_delta && trg_entry->in_pack &&\n-\t trg_entry->in_pack == src_entry->in_pack &&\n+\tif (reuse_delta && IN_PACK(trg_entry) &&\n+\t IN_PACK(trg_entry) == IN_PACK(src_entry) &&\n \t !src_entry->preferred_base &&\n \t trg_entry->in_pack_type != OBJ_REF_DELTA &&\n \t trg_entry->in_pack_type != OBJ_OFS_DELTA)\n@@ -3192,6 +3196,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n+\tprepare_packing_data(&to_pack);\n+\n \tif (progress)\n \t\tprogress_state = start_progress(_(\"Counting objects\"), 0);\n \tif (!use_internal_rev_list)\ndiff --git a/object-store.h b/object-store.h\nindex fef33f345f..71d4087867 100644\n--- a/object-store.h\n+++ b/object-store.h\n@@ -69,6 +69,7 @@ struct packed_git {\n \tint index_version;\n \ttime_t mtime;\n \tint pack_fd;\n+\tint index; /* for builtin/pack-objects.c */\n \tunsigned pack_local:1,\n \t\t pack_keep:1,\n \t\t freshened:1,\ndiff --git a/pack-objects.c b/pack-objects.c\nindex 9558d13834..08cfe68b6b 100644\n--- a/pack-objects.c\n+++ b/pack-objects.c\n@@ -2,6 +2,8 @@\n #include \"object.h\"\n #include \"pack.h\"\n #include \"pack-objects.h\"\n+#include \"packfile.h\"\n+#include \"config.h\"\n \n static uint32_t locate_object_entry_hash(struct packing_data *pdata,\n \t\t\t\t\t const unsigned char *sha1,\n@@ -86,6 +88,63 @@ struct object_entry *packlist_find(struct packing_data *pdata,\n \treturn &pdata->objects[pdata->index[i] - 1];\n }\n \n+static void prepare_in_pack_by_idx(struct packing_data *pdata)\n+{\n+\tstruct packed_git **mapping, *p;\n+\tint cnt = 0, nr = 1U << OE_IN_PACK_BITS;\n+\n+\tALLOC_ARRAY(mapping, nr);\n+\t/*\n+\t * oe_in_pack() on an all-zero'd object_entry\n+\t * (i.e. in_pack_idx also zero) should return NULL.\n+\t */\n+\tmapping[cnt++] = NULL;\n+\tfor (p = get_packed_git(the_repository); p; p = p->next, cnt++) {\n+\t\tif (cnt == nr) {\n+\t\t\tfree(mapping);\n+\t\t\treturn;\n+\t\t}\n+\t\tp->index = cnt;\n+\t\tmapping[cnt] = p;\n+\t}\n+\tpdata->in_pack_by_idx = mapping;\n+}\n+\n+/*\n+ * A new pack appears after prepare_in_pack_by_idx() has been\n+ * run. This is likely a race.\n+ *\n+ * We could map this new pack to in_pack_by_idx[] array, but then we\n+ * have to deal with full array anyway. And since it's hard to test\n+ * this fall back code, just stay simple and fall back to using\n+ * in_pack[] array.\n+ */\n+void oe_map_new_pack(struct packing_data *pack,\n+\t\t struct packed_git *p)\n+{\n+\tuint32_t i;\n+\n+\tREALLOC_ARRAY(pack->in_pack, pack->nr_alloc);\n+\n+\tfor (i = 0; i < pack->nr_objects; i++)\n+\t\tpack->in_pack[i] = oe_in_pack(pack, pack->objects + i);\n+\n+\tFREE_AND_NULL(pack->in_pack_by_idx);\n+}\n+\n+/* assume pdata is already zero'd by caller */\n+void prepare_packing_data(struct packing_data *pdata)\n+{\n+\tif (git_env_bool(\"GIT_TEST_FULL_IN_PACK_ARRAY\", 0)) {\n+\t\t/*\n+\t\t * do not initialize in_pack_by_idx[] to force the\n+\t\t * slow path in oe_in_pack()\n+\t\t */\n+\t} else {\n+\t\tprepare_in_pack_by_idx(pdata);\n+\t}\n+}\n+\n struct object_entry *packlist_alloc(struct packing_data *pdata,\n \t\t\t\t const unsigned char *sha1,\n \t\t\t\t uint32_t index_pos)\n@@ -95,6 +154,9 @@ struct object_entry *packlist_alloc(struct packing_data *pdata,\n \tif (pdata->nr_objects >= pdata->nr_alloc) {\n \t\tpdata->nr_alloc = (pdata->nr_alloc + 1024) * 3 / 2;\n \t\tREALLOC_ARRAY(pdata->objects, pdata->nr_alloc);\n+\n+\t\tif (!pdata->in_pack_by_idx)\n+\t\t\tREALLOC_ARRAY(pdata->in_pack, pdata->nr_alloc);\n \t}\n \n \tnew_entry = pdata->objects + pdata->nr_objects++;\n@@ -107,5 +169,8 @@ struct object_entry *packlist_alloc(struct packing_data *pdata,\n \telse\n \t\tpdata->index[index_pos] = pdata->nr_objects;\n \n+\tif (pdata->in_pack)\n+\t\tpdata->in_pack[pdata->nr_objects - 1] = NULL;\n+\n \treturn new_entry;\n }\ndiff --git a/pack-objects.h b/pack-objects.h\nindex 71ea992c3c..7bedd5af81 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -1,8 +1,11 @@\n #ifndef PACK_OBJECTS_H\n #define PACK_OBJECTS_H\n \n+#include \"object-store.h\"\n+\n #define OE_DFS_STATE_BITS\t2\n #define OE_DEPTH_BITS\t\t12\n+#define OE_IN_PACK_BITS\t\t10\n \n /*\n * State flags for depth-first search used for analyzing delta cycles.\n@@ -65,7 +68,7 @@ enum dfs_state {\n struct object_entry {\n \tstruct pack_idx_entry idx;\n \tunsigned long size;\t/* uncompressed size */\n-\tstruct packed_git *in_pack;\t/* already in pack */\n+\tunsigned in_pack_idx:OE_IN_PACK_BITS;\t/* already in pack */\n \toff_t in_pack_offset;\n \tstruct object_entry *delta;\t/* delta base object */\n \tstruct object_entry *delta_child; /* deltified objects who bases me */\n@@ -100,8 +103,18 @@ struct packing_data {\n \tuint32_t index_size;\n \n \tunsigned int *in_pack_pos;\n+\n+\t/*\n+\t * Only one of these can be non-NULL and they have different\n+\t * sizes. if in_pack_by_idx is allocated, oe_in_pack() returns\n+\t * the pack of an object using in_pack_idx field. If not,\n+\t * in_pack[] array is used the same way as in_pack_pos[]\n+\t */\n+\tstruct packed_git **in_pack_by_idx;\n+\tstruct packed_git **in_pack;\n };\n \n+void prepare_packing_data(struct packing_data *pdata);\n struct object_entry *packlist_alloc(struct packing_data *pdata,\n \t\t\t\t const unsigned char *sha1,\n \t\t\t\t uint32_t index_pos);\n@@ -158,4 +171,27 @@ static inline void oe_set_in_pack_pos(const struct packing_data *pack,\n \tpack->in_pack_pos[e - pack->objects] = pos;\n }\n \n+static inline struct packed_git *oe_in_pack(const struct packing_data *pack,\n+\t\t\t\t\t const struct object_entry *e)\n+{\n+\tif (pack->in_pack_by_idx)\n+\t\treturn pack->in_pack_by_idx[e->in_pack_idx];\n+\telse\n+\t\treturn pack->in_pack[e - pack->objects];\n+}\n+\n+void oe_map_new_pack(struct packing_data *pack,\n+\t\t struct packed_git *p);\n+static inline void oe_set_in_pack(struct packing_data *pack,\n+\t\t\t\t struct object_entry *e,\n+\t\t\t\t struct packed_git *p)\n+{\n+\tif (!p->index)\n+\t\toe_map_new_pack(pack, p);\n+\tif (pack->in_pack_by_idx)\n+\t\te->in_pack_idx = p->index;\n+\telse\n+\t\tpack->in_pack[e - pack->objects] = p;\n+}\n+\n #endif\ndiff --git a/t/README b/t/README\nindex d5e0a3c786..a06a85bae8 100644\n--- a/t/README\n+++ b/t/README\n@@ -304,6 +304,11 @@ environment set.\n GIT_TEST_SPLIT_INDEX= forces split-index mode on the whole\n test suite. Accept any boolean values that are accepted by git-config.\n \n+GIT_TEST_FULL_IN_PACK_ARRAY= exercises the uncommon\n+pack-objects code path where there are more than 1024 packs even if\n+the actual number of packs in repository is below this limit. Accept\n+any boolean values that are accepted by git-config.\n+\n Naming Tests\n ------------\n \ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 65ff60f2ee..54eff03851 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -457,6 +457,11 @@ test_expect_success !PTHREADS,C_LOCALE_OUTPUT 'pack-objects --threads=N or pack.\n \tgrep -F \"no threads support, ignoring pack.threads\" err\n '\n \n+test_expect_success 'pack-objects in too-many-packs mode' '\n+\tGIT_TEST_FULL_IN_PACK_ARRAY=1 git repack -ad &&\n+\tgit fsck\n+'\n+\n #\n # WARNING!\n #\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344677","messageId":"20180414153513.9902-9-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 08/15] pack-objects: refer to delta objects by index instead of pointer","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:06Z","receivedAt":"2018-04-14T15:35:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"These delta pointers always point to elements in the objects[] array\nin packing_data struct. We can only hold maximum 4G of those objects\nbecause the array size in nr_objects is uint32_t. We could use\nuint32_t indexes to address these elements instead of pointers. On\n64-bit architecture (8 bytes per pointer) this would save 4 bytes per\npointer.\n\nConvert these delta pointers to indexes. Since we need to handle NULL\npointers as well, the index is shifted by one [1].\n\n[1] This means we can only index 2^32-2 objects even though nr_objects\n could contain 2^32-1 objects. It should not be a problem in\n practice because when we grow objects[], nr_alloc would probably\n blow up long before nr_objects hits the wall.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/pack-objects.c | 117 ++++++++++++++++++++++-------------------\n pack-objects.h | 67 +++++++++++++++++++++--\n 2 files changed, 125 insertions(+), 59 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 2784d58ec2..ec02641d2e 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -32,6 +32,12 @@\n #include \"object-store.h\"\n \n #define IN_PACK(obj) oe_in_pack(&to_pack, obj)\n+#define DELTA(obj) oe_delta(&to_pack, obj)\n+#define DELTA_CHILD(obj) oe_delta_child(&to_pack, obj)\n+#define DELTA_SIBLING(obj) oe_delta_sibling(&to_pack, obj)\n+#define SET_DELTA(obj, val) oe_set_delta(&to_pack, obj, val)\n+#define SET_DELTA_CHILD(obj, val) oe_set_delta_child(&to_pack, obj, val)\n+#define SET_DELTA_SIBLING(obj, val) oe_set_delta_sibling(&to_pack, obj, val)\n \n static const char *pack_usage[] = {\n \tN_(\"git pack-objects --stdout [...] [< | < ]\"),\n@@ -129,10 +135,11 @@ static void *get_delta(struct object_entry *entry)\n \tbuf = read_object_file(&entry->idx.oid, &type, &size);\n \tif (!buf)\n \t\tdie(\"unable to read %s\", oid_to_hex(&entry->idx.oid));\n-\tbase_buf = read_object_file(&entry->delta->idx.oid, &type, &base_size);\n+\tbase_buf = read_object_file(&DELTA(entry)->idx.oid, &type,\n+\t\t\t\t &base_size);\n \tif (!base_buf)\n \t\tdie(\"unable to read %s\",\n-\t\t oid_to_hex(&entry->delta->idx.oid));\n+\t\t oid_to_hex(&DELTA(entry)->idx.oid));\n \tdelta_buf = diff_delta(base_buf, base_size,\n \t\t\t buf, size, &delta_size, 0);\n \tif (!delta_buf || delta_size != entry->delta_size)\n@@ -288,12 +295,12 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent\n \t\tsize = entry->delta_size;\n \t\tbuf = entry->delta_data;\n \t\tentry->delta_data = NULL;\n-\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n+\t\ttype = (allow_ofs_delta && DELTA(entry)->idx.offset) ?\n \t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n \t} else {\n \t\tbuf = get_delta(entry);\n \t\tsize = entry->delta_size;\n-\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n+\t\ttype = (allow_ofs_delta && DELTA(entry)->idx.offset) ?\n \t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n \t}\n \n@@ -317,7 +324,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent\n \t\t * encoding of the relative offset for the delta\n \t\t * base from this object's position in the pack.\n \t\t */\n-\t\toff_t ofs = entry->idx.offset - entry->delta->idx.offset;\n+\t\toff_t ofs = entry->idx.offset - DELTA(entry)->idx.offset;\n \t\tunsigned pos = sizeof(dheader) - 1;\n \t\tdheader[pos] = ofs & 127;\n \t\twhile (ofs >>= 7)\n@@ -343,7 +350,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent\n \t\t\treturn 0;\n \t\t}\n \t\thashwrite(f, header, hdrlen);\n-\t\thashwrite(f, entry->delta->idx.oid.hash, 20);\n+\t\thashwrite(f, DELTA(entry)->idx.oid.hash, 20);\n \t\thdrlen += 20;\n \t} else {\n \t\tif (limit && hdrlen + datalen + 20 >= limit) {\n@@ -379,8 +386,8 @@ static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,\n \t\t dheader[MAX_PACK_OBJECT_HEADER];\n \tunsigned hdrlen;\n \n-\tif (entry->delta)\n-\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n+\tif (DELTA(entry))\n+\t\ttype = (allow_ofs_delta && DELTA(entry)->idx.offset) ?\n \t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n \thdrlen = encode_in_pack_object_header(header, sizeof(header),\n \t\t\t\t\t type, entry->size);\n@@ -408,7 +415,7 @@ static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,\n \t}\n \n \tif (type == OBJ_OFS_DELTA) {\n-\t\toff_t ofs = entry->idx.offset - entry->delta->idx.offset;\n+\t\toff_t ofs = entry->idx.offset - DELTA(entry)->idx.offset;\n \t\tunsigned pos = sizeof(dheader) - 1;\n \t\tdheader[pos] = ofs & 127;\n \t\twhile (ofs >>= 7)\n@@ -427,7 +434,7 @@ static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,\n \t\t\treturn 0;\n \t\t}\n \t\thashwrite(f, header, hdrlen);\n-\t\thashwrite(f, entry->delta->idx.oid.hash, 20);\n+\t\thashwrite(f, DELTA(entry)->idx.oid.hash, 20);\n \t\thdrlen += 20;\n \t\treused_delta++;\n \t} else {\n@@ -467,13 +474,13 @@ static off_t write_object(struct hashfile *f,\n \telse\n \t\tlimit = pack_size_limit - write_offset;\n \n-\tif (!entry->delta)\n+\tif (!DELTA(entry))\n \t\tusable_delta = 0;\t/* no delta */\n \telse if (!pack_size_limit)\n \t usable_delta = 1;\t/* unlimited packfile */\n-\telse if (entry->delta->idx.offset == (off_t)-1)\n+\telse if (DELTA(entry)->idx.offset == (off_t)-1)\n \t\tusable_delta = 0;\t/* base was written to another pack */\n-\telse if (entry->delta->idx.offset)\n+\telse if (DELTA(entry)->idx.offset)\n \t\tusable_delta = 1;\t/* base already exists in this pack */\n \telse\n \t\tusable_delta = 0;\t/* base could end up in another pack */\n@@ -489,7 +496,7 @@ static off_t write_object(struct hashfile *f,\n \t\t\t\t/* ... but pack split may override that */\n \telse if (oe_type(entry) != entry->in_pack_type)\n \t\tto_reuse = 0;\t/* pack has delta which is unusable */\n-\telse if (entry->delta)\n+\telse if (DELTA(entry))\n \t\tto_reuse = 0;\t/* we want to pack afresh */\n \telse\n \t\tto_reuse = 1;\t/* we have it in-pack undeltified,\n@@ -541,12 +548,12 @@ static enum write_one_status write_one(struct hashfile *f,\n \t}\n \n \t/* if we are deltified, write out base object first. */\n-\tif (e->delta) {\n+\tif (DELTA(e)) {\n \t\te->idx.offset = 1; /* now recurse */\n-\t\tswitch (write_one(f, e->delta, offset)) {\n+\t\tswitch (write_one(f, DELTA(e), offset)) {\n \t\tcase WRITE_ONE_RECURSIVE:\n \t\t\t/* we cannot depend on this one */\n-\t\t\te->delta = NULL;\n+\t\t\tSET_DELTA(e, NULL);\n \t\t\tbreak;\n \t\tdefault:\n \t\t\tbreak;\n@@ -608,34 +615,34 @@ static void add_descendants_to_write_order(struct object_entry **wo,\n \t\t\t/* add this node... */\n \t\t\tadd_to_write_order(wo, endp, e);\n \t\t\t/* all its siblings... */\n-\t\t\tfor (s = e->delta_sibling; s; s = s->delta_sibling) {\n+\t\t\tfor (s = DELTA_SIBLING(e); s; s = DELTA_SIBLING(s)) {\n \t\t\t\tadd_to_write_order(wo, endp, s);\n \t\t\t}\n \t\t}\n \t\t/* drop down a level to add left subtree nodes if possible */\n-\t\tif (e->delta_child) {\n+\t\tif (DELTA_CHILD(e)) {\n \t\t\tadd_to_order = 1;\n-\t\t\te = e->delta_child;\n+\t\t\te = DELTA_CHILD(e);\n \t\t} else {\n \t\t\tadd_to_order = 0;\n \t\t\t/* our sibling might have some children, it is next */\n-\t\t\tif (e->delta_sibling) {\n-\t\t\t\te = e->delta_sibling;\n+\t\t\tif (DELTA_SIBLING(e)) {\n+\t\t\t\te = DELTA_SIBLING(e);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\t/* go back to our parent node */\n-\t\t\te = e->delta;\n-\t\t\twhile (e && !e->delta_sibling) {\n+\t\t\te = DELTA(e);\n+\t\t\twhile (e && !DELTA_SIBLING(e)) {\n \t\t\t\t/* we're on the right side of a subtree, keep\n \t\t\t\t * going up until we can go right again */\n-\t\t\t\te = e->delta;\n+\t\t\t\te = DELTA(e);\n \t\t\t}\n \t\t\tif (!e) {\n \t\t\t\t/* done- we hit our original root node */\n \t\t\t\treturn;\n \t\t\t}\n \t\t\t/* pass it off to sibling at this level */\n-\t\t\te = e->delta_sibling;\n+\t\t\te = DELTA_SIBLING(e);\n \t\t}\n \t};\n }\n@@ -646,7 +653,7 @@ static void add_family_to_write_order(struct object_entry **wo,\n {\n \tstruct object_entry *root;\n \n-\tfor (root = e; root->delta; root = root->delta)\n+\tfor (root = e; DELTA(root); root = DELTA(root))\n \t\t; /* nothing */\n \tadd_descendants_to_write_order(wo, endp, root);\n }\n@@ -661,8 +668,8 @@ static struct object_entry **compute_write_order(void)\n \tfor (i = 0; i < to_pack.nr_objects; i++) {\n \t\tobjects[i].tagged = 0;\n \t\tobjects[i].filled = 0;\n-\t\tobjects[i].delta_child = NULL;\n-\t\tobjects[i].delta_sibling = NULL;\n+\t\tSET_DELTA_CHILD(&objects[i], NULL);\n+\t\tSET_DELTA_SIBLING(&objects[i], NULL);\n \t}\n \n \t/*\n@@ -672,11 +679,11 @@ static struct object_entry **compute_write_order(void)\n \t */\n \tfor (i = to_pack.nr_objects; i > 0;) {\n \t\tstruct object_entry *e = &objects[--i];\n-\t\tif (!e->delta)\n+\t\tif (!DELTA(e))\n \t\t\tcontinue;\n \t\t/* Mark me as the first child */\n-\t\te->delta_sibling = e->delta->delta_child;\n-\t\te->delta->delta_child = e;\n+\t\te->delta_sibling_idx = DELTA(e)->delta_child_idx;\n+\t\tSET_DELTA_CHILD(DELTA(e), e);\n \t}\n \n \t/*\n@@ -1493,10 +1500,10 @@ static void check_object(struct object_entry *entry)\n \t\t\t * circular deltas.\n \t\t\t */\n \t\t\toe_set_type(entry, entry->in_pack_type);\n-\t\t\tentry->delta = base_entry;\n+\t\t\tSET_DELTA(entry, base_entry);\n \t\t\tentry->delta_size = entry->size;\n-\t\t\tentry->delta_sibling = base_entry->delta_child;\n-\t\t\tbase_entry->delta_child = entry;\n+\t\t\tentry->delta_sibling_idx = base_entry->delta_child_idx;\n+\t\t\tSET_DELTA_CHILD(base_entry, entry);\n \t\t\tunuse_pack(&w_curs);\n \t\t\treturn;\n \t\t}\n@@ -1567,17 +1574,19 @@ static int pack_offset_sort(const void *_a, const void *_b)\n */\n static void drop_reused_delta(struct object_entry *entry)\n {\n-\tstruct object_entry **p = &entry->delta->delta_child;\n+\tunsigned *idx = &to_pack.objects[entry->delta_idx - 1].delta_child_idx;\n \tstruct object_info oi = OBJECT_INFO_INIT;\n \tenum object_type type;\n \n-\twhile (*p) {\n-\t\tif (*p == entry)\n-\t\t\t*p = (*p)->delta_sibling;\n+\twhile (*idx) {\n+\t\tstruct object_entry *oe = &to_pack.objects[*idx - 1];\n+\n+\t\tif (oe == entry)\n+\t\t\t*idx = oe->delta_sibling_idx;\n \t\telse\n-\t\t\tp = &(*p)->delta_sibling;\n+\t\t\tidx = &oe->delta_sibling_idx;\n \t}\n-\tentry->delta = NULL;\n+\tSET_DELTA(entry, NULL);\n \tentry->depth = 0;\n \n \toi.sizep = &entry->size;\n@@ -1617,7 +1626,7 @@ static void break_delta_chains(struct object_entry *entry)\n \n \tfor (cur = entry, total_depth = 0;\n \t cur;\n-\t cur = cur->delta, total_depth++) {\n+\t cur = DELTA(cur), total_depth++) {\n \t\tif (cur->dfs_state == DFS_DONE) {\n \t\t\t/*\n \t\t\t * We've already seen this object and know it isn't\n@@ -1642,7 +1651,7 @@ static void break_delta_chains(struct object_entry *entry)\n \t\t * it's not a delta, we're done traversing, but we'll mark it\n \t\t * done to save time on future traversals.\n \t\t */\n-\t\tif (!cur->delta) {\n+\t\tif (!DELTA(cur)) {\n \t\t\tcur->dfs_state = DFS_DONE;\n \t\t\tbreak;\n \t\t}\n@@ -1665,7 +1674,7 @@ static void break_delta_chains(struct object_entry *entry)\n \t\t * We keep all commits in the chain that we examined.\n \t\t */\n \t\tcur->dfs_state = DFS_ACTIVE;\n-\t\tif (cur->delta->dfs_state == DFS_ACTIVE) {\n+\t\tif (DELTA(cur)->dfs_state == DFS_ACTIVE) {\n \t\t\tdrop_reused_delta(cur);\n \t\t\tcur->dfs_state = DFS_DONE;\n \t\t\tbreak;\n@@ -1680,7 +1689,7 @@ static void break_delta_chains(struct object_entry *entry)\n \t * an extra \"next\" pointer to keep going after we reset cur->delta.\n \t */\n \tfor (cur = entry; cur; cur = next) {\n-\t\tnext = cur->delta;\n+\t\tnext = DELTA(cur);\n \n \t\t/*\n \t\t * We should have a chain of zero or more ACTIVE states down to\n@@ -1865,7 +1874,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \n \t/* Now some size filtering heuristics. */\n \ttrg_size = trg_entry->size;\n-\tif (!trg_entry->delta) {\n+\tif (!DELTA(trg_entry)) {\n \t\tmax_size = trg_size/2 - 20;\n \t\tref_depth = 1;\n \t} else {\n@@ -1939,7 +1948,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \tif (!delta_buf)\n \t\treturn 0;\n \n-\tif (trg_entry->delta) {\n+\tif (DELTA(trg_entry)) {\n \t\t/* Prefer only shallower same-sized deltas. */\n \t\tif (delta_size == trg_entry->delta_size &&\n \t\t src->depth + 1 >= trg->depth) {\n@@ -1968,7 +1977,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \t\tfree(delta_buf);\n \t}\n \n-\ttrg_entry->delta = src_entry;\n+\tSET_DELTA(trg_entry, src_entry);\n \ttrg_entry->delta_size = delta_size;\n \ttrg->depth = src->depth + 1;\n \n@@ -1977,13 +1986,13 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \n static unsigned int check_delta_limit(struct object_entry *me, unsigned int n)\n {\n-\tstruct object_entry *child = me->delta_child;\n+\tstruct object_entry *child = DELTA_CHILD(me);\n \tunsigned int m = n;\n \twhile (child) {\n \t\tunsigned int c = check_delta_limit(child, n + 1);\n \t\tif (m < c)\n \t\t\tm = c;\n-\t\tchild = child->delta_sibling;\n+\t\tchild = DELTA_SIBLING(child);\n \t}\n \treturn m;\n }\n@@ -2052,7 +2061,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,\n \t\t * otherwise they would become too deep.\n \t\t */\n \t\tmax_depth = depth;\n-\t\tif (entry->delta_child) {\n+\t\tif (DELTA_CHILD(entry)) {\n \t\t\tmax_depth -= check_delta_limit(entry, 0);\n \t\t\tif (max_depth <= 0)\n \t\t\t\tgoto next;\n@@ -2102,7 +2111,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,\n \t\t * depth, leaving it in the window is pointless. we\n \t\t * should evict it first.\n \t\t */\n-\t\tif (entry->delta && max_depth <= n->depth)\n+\t\tif (DELTA(entry) && max_depth <= n->depth)\n \t\t\tcontinue;\n \n \t\t/*\n@@ -2110,7 +2119,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,\n \t\t * currently deltified object, to keep it longer. It will\n \t\t * be the first base object to be attempted next.\n \t\t */\n-\t\tif (entry->delta) {\n+\t\tif (DELTA(entry)) {\n \t\t\tstruct unpacked swap = array[best_base];\n \t\t\tint dist = (window + idx - best_base) % window;\n \t\t\tint dst = best_base;\n@@ -2431,7 +2440,7 @@ static void prepare_pack(int window, int depth)\n \tfor (i = 0; i < to_pack.nr_objects; i++) {\n \t\tstruct object_entry *entry = to_pack.objects + i;\n \n-\t\tif (entry->delta)\n+\t\tif (DELTA(entry))\n \t\t\t/* This happens if we decided to reuse existing\n \t\t\t * delta from a pack. \"reuse_delta &&\" is implied.\n \t\t\t */\ndiff --git a/pack-objects.h b/pack-objects.h\nindex 7bedd5af81..e962dce3c0 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -70,11 +70,11 @@ struct object_entry {\n \tunsigned long size;\t/* uncompressed size */\n \tunsigned in_pack_idx:OE_IN_PACK_BITS;\t/* already in pack */\n \toff_t in_pack_offset;\n-\tstruct object_entry *delta;\t/* delta base object */\n-\tstruct object_entry *delta_child; /* deltified objects who bases me */\n-\tstruct object_entry *delta_sibling; /* other deltified objects who\n-\t\t\t\t\t * uses the same base as me\n-\t\t\t\t\t */\n+\tuint32_t delta_idx;\t/* delta base object */\n+\tuint32_t delta_child_idx; /* deltified objects who bases me */\n+\tuint32_t delta_sibling_idx; /* other deltified objects who\n+\t\t\t\t * uses the same base as me\n+\t\t\t\t */\n \tvoid *delta_data;\t/* cached delta (uncompressed) */\n \tunsigned long delta_size;\t/* delta data size (uncompressed) */\n \tunsigned long z_delta_size;\t/* delta data size (compressed) */\n@@ -194,4 +194,61 @@ static inline void oe_set_in_pack(struct packing_data *pack,\n \t\tpack->in_pack[e - pack->objects] = p;\n }\n \n+static inline struct object_entry *oe_delta(\n+\t\tconst struct packing_data *pack,\n+\t\tconst struct object_entry *e)\n+{\n+\tif (e->delta_idx)\n+\t\treturn &pack->objects[e->delta_idx - 1];\n+\treturn NULL;\n+}\n+\n+static inline void oe_set_delta(struct packing_data *pack,\n+\t\t\t\tstruct object_entry *e,\n+\t\t\t\tstruct object_entry *delta)\n+{\n+\tif (delta)\n+\t\te->delta_idx = (delta - pack->objects) + 1;\n+\telse\n+\t\te->delta_idx = 0;\n+}\n+\n+static inline struct object_entry *oe_delta_child(\n+\t\tconst struct packing_data *pack,\n+\t\tconst struct object_entry *e)\n+{\n+\tif (e->delta_child_idx)\n+\t\treturn &pack->objects[e->delta_child_idx - 1];\n+\treturn NULL;\n+}\n+\n+static inline void oe_set_delta_child(struct packing_data *pack,\n+\t\t\t\t struct object_entry *e,\n+\t\t\t\t struct object_entry *delta)\n+{\n+\tif (delta)\n+\t\te->delta_child_idx = (delta - pack->objects) + 1;\n+\telse\n+\t\te->delta_child_idx = 0;\n+}\n+\n+static inline struct object_entry *oe_delta_sibling(\n+\t\tconst struct packing_data *pack,\n+\t\tconst struct object_entry *e)\n+{\n+\tif (e->delta_sibling_idx)\n+\t\treturn &pack->objects[e->delta_sibling_idx - 1];\n+\treturn NULL;\n+}\n+\n+static inline void oe_set_delta_sibling(struct packing_data *pack,\n+\t\t\t\t\tstruct object_entry *e,\n+\t\t\t\t\tstruct object_entry *delta)\n+{\n+\tif (delta)\n+\t\te->delta_sibling_idx = (delta - pack->objects) + 1;\n+\telse\n+\t\te->delta_sibling_idx = 0;\n+}\n+\n #endif\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344678","messageId":"20180414153513.9902-10-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 09/15] pack-objects: shrink z_delta_size field in struct object_entry","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:07Z","receivedAt":"2018-04-14T15:35:51Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"We only cache deltas when it's smaller than a certain limit. This limit\ndefaults to 1000 but save its compressed length in a 64-bit field.\nShrink that field down to 20 bits, so you can only cache 1MB deltas.\nLarger deltas must be recomputed at when the pack is written down.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/config.txt | 3 ++-\n builtin/pack-objects.c | 24 ++++++++++++++++++------\n pack-objects.h | 3 ++-\n 3 files changed, 22 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex d97f10722c..a62134264e 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2459,7 +2459,8 @@ pack.deltaCacheLimit::\n \tThe maximum size of a delta, that is cached in\n \tlinkgit:git-pack-objects[1]. This cache is used to speed up the\n \twriting object phase by not having to recompute the final delta\n-\tresult once the best match for all objects is found. Defaults to 1000.\n+\tresult once the best match for all objects is found.\n+\tDefaults to 1000. Maximum value is 65535.\n \n pack.threads::\n \tSpecifies the number of threads to spawn when searching for best\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex ec02641d2e..211bb1ad0e 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2099,12 +2099,19 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,\n \t\t * between writes at that moment.\n \t\t */\n \t\tif (entry->delta_data && !pack_to_stdout) {\n-\t\t\tentry->z_delta_size = do_compress(&entry->delta_data,\n-\t\t\t\t\t\t\t entry->delta_size);\n-\t\t\tcache_lock();\n-\t\t\tdelta_cache_size -= entry->delta_size;\n-\t\t\tdelta_cache_size += entry->z_delta_size;\n-\t\t\tcache_unlock();\n+\t\t\tunsigned long size;\n+\n+\t\t\tsize = do_compress(&entry->delta_data, entry->delta_size);\n+\t\t\tif (size < (1U << OE_Z_DELTA_BITS)) {\n+\t\t\t\tentry->z_delta_size = size;\n+\t\t\t\tcache_lock();\n+\t\t\t\tdelta_cache_size -= entry->delta_size;\n+\t\t\t\tdelta_cache_size += entry->z_delta_size;\n+\t\t\t\tcache_unlock();\n+\t\t\t} else {\n+\t\t\t\tFREE_AND_NULL(entry->delta_data);\n+\t\t\t\tentry->z_delta_size = 0;\n+\t\t\t}\n \t\t}\n \n \t\t/* if we made n a delta, and if n is already at max\n@@ -3087,6 +3094,11 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\tdepth, (1 << OE_DEPTH_BITS) - 1);\n \t\tdepth = (1 << OE_DEPTH_BITS) - 1;\n \t}\n+\tif (cache_max_small_delta_size >= (1U << OE_Z_DELTA_BITS)) {\n+\t\twarning(_(\"pack.deltaCacheLimit is too high, forcing %d\"),\n+\t\t\t(1U << OE_Z_DELTA_BITS) - 1);\n+\t\tcache_max_small_delta_size = (1U << OE_Z_DELTA_BITS) - 1;\n+\t}\n \n \targv_array_push(&rp, \"pack-objects\");\n \tif (thin) {\ndiff --git a/pack-objects.h b/pack-objects.h\nindex e962dce3c0..9d0391c173 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -6,6 +6,7 @@\n #define OE_DFS_STATE_BITS\t2\n #define OE_DEPTH_BITS\t\t12\n #define OE_IN_PACK_BITS\t\t10\n+#define OE_Z_DELTA_BITS\t\t20\n \n /*\n * State flags for depth-first search used for analyzing delta cycles.\n@@ -77,7 +78,7 @@ struct object_entry {\n \t\t\t\t */\n \tvoid *delta_data;\t/* cached delta (uncompressed) */\n \tunsigned long delta_size;\t/* delta data size (uncompressed) */\n-\tunsigned long z_delta_size;\t/* delta data size (compressed) */\n+\tunsigned z_delta_size:OE_Z_DELTA_BITS;\n \tunsigned type_:TYPE_BITS;\n \tunsigned in_pack_type:TYPE_BITS; /* could be delta */\n \tunsigned type_valid:1;\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344679","messageId":"20180414153513.9902-16-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 15/15] ci: exercise the whole test suite with uncommon code in pack-objects","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:13Z","receivedAt":"2018-04-14T15:35:52Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Some recent optimizations have been added to pack-objects to reduce\nmemory usage and some code paths are split into two: one for common\nuse cases and one for rare ones. Make sure the rare cases are tested\nwith Travis since it requires manual test configuration that is\nunlikely to be done by developers.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n ci/run-build-and-tests.sh | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex 8190a753de..4b04c75b7f 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -11,7 +11,10 @@ make --jobs=2\n make --quiet test\n if test \"$jobname\" = \"linux-gcc\"\n then\n-\tGIT_TEST_SPLIT_INDEX=yes make --quiet test\n+\texport GIT_TEST_SPLIT_INDEX=yes\n+\texport GIT_TEST_FULL_IN_PACK_ARRAY=true\n+\texport GIT_TEST_OE_SIZE=10\n+\tmake --quiet test\n fi\n \n check_unignored_build_artifacts\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344680","messageId":"20180414153513.9902-15-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 14/15] pack-objects: reorder members to shrink struct object_entry","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:12Z","receivedAt":"2018-04-14T15:35:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Previous patches leave lots of holes and padding in this struct. This\npatch reorders the members and shrinks the struct down to 80 bytes\n(from 136 bytes on 64-bit systems, before any field shrinking is done)\nwith 16 bits to spare (and a couple more in in_pack_header_size when\nwe really run out of bits).\n\nThis is the last in a series of memory reduction patches (see\n\"pack-objects: a bit of document about struct object_entry\" for the\nfirst one).\n\nOverall they've reduced repack memory size on linux-2.6.git from\n3.747G to 3.424G, or by around 320M, a decrease of 8.5%. The runtime\nof repack has stayed the same throughout this series. Ævar's testing\non a big monorepo he has access to (bigger than linux-2.6.git) has\nshown a 7.9% reduction, so the overall expected improvement should be\nsomewhere around 8%.\n\nSee 87po42cwql.fsf@evledraar.gmail.com on-list\n(https://public-inbox.org/git/87po42cwql.fsf@evledraar.gmail.com/) for\nmore detailed numbers and a test script used to produce the numbers\ncited above.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n pack-objects.h | 28 +++++++++++++++++++++-------\n 1 file changed, 21 insertions(+), 7 deletions(-)\n\ndiff --git a/pack-objects.h b/pack-objects.h\nindex 1c588184b2..e5456c6c89 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -28,6 +28,10 @@ enum dfs_state {\n };\n \n /*\n+ * The size of struct nearly determines pack-objects's memory\n+ * consumption. This struct is packed tight for that reason. When you\n+ * add or reorder something in this struct, think a bit about this.\n+ *\n * basic object info\n * -----------------\n * idx.oid is filled up before delta searching starts. idx.crc32 is\n@@ -76,34 +80,44 @@ enum dfs_state {\n */\n struct object_entry {\n \tstruct pack_idx_entry idx;\n+\tvoid *delta_data;\t/* cached delta (uncompressed) */\n+\toff_t in_pack_offset;\n+\tuint32_t hash;\t\t\t/* name hint hash */\n \tunsigned size_:OE_SIZE_BITS;\n \tunsigned size_valid:1;\n-\tunsigned in_pack_idx:OE_IN_PACK_BITS;\t/* already in pack */\n-\toff_t in_pack_offset;\n \tuint32_t delta_idx;\t/* delta base object */\n \tuint32_t delta_child_idx; /* deltified objects who bases me */\n \tuint32_t delta_sibling_idx; /* other deltified objects who\n \t\t\t\t * uses the same base as me\n \t\t\t\t */\n-\tvoid *delta_data;\t/* cached delta (uncompressed) */\n \tunsigned delta_size_:OE_DELTA_SIZE_BITS; /* delta data size (uncompressed) */\n \tunsigned delta_size_valid:1;\n+\tunsigned in_pack_idx:OE_IN_PACK_BITS;\t/* already in pack */\n \tunsigned z_delta_size:OE_Z_DELTA_BITS;\n+\tunsigned type_valid:1;\n \tunsigned type_:TYPE_BITS;\n+\tunsigned no_try_delta:1;\n \tunsigned in_pack_type:TYPE_BITS; /* could be delta */\n-\tunsigned type_valid:1;\n-\tuint32_t hash;\t\t\t/* name hint hash */\n-\tunsigned char in_pack_header_size;\n \tunsigned preferred_base:1; /*\n \t\t\t\t * we do not pack this, but is available\n \t\t\t\t * to be used as the base object to delta\n \t\t\t\t * objects against.\n \t\t\t\t */\n-\tunsigned no_try_delta:1;\n \tunsigned tagged:1; /* near the very tip of refs */\n \tunsigned filled:1; /* assigned write-order */\n \tunsigned dfs_state:OE_DFS_STATE_BITS;\n+\tunsigned char in_pack_header_size;\n \tunsigned depth:OE_DEPTH_BITS;\n+\n+\t/*\n+\t * pahole results on 64-bit linux (gcc and clang)\n+\t *\n+\t * size: 80, bit_padding: 20 bits, holes: 8 bits\n+\t *\n+\t * and on 32-bit (gcc)\n+\t *\n+\t * size: 76, bit_padding: 20 bits, holes: 8 bits\n+\t */\n };\n \n struct packing_data {\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344681","messageId":"20180414153513.9902-14-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 13/15] pack-objects: shrink delta_size field in struct object_entry","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:11Z","receivedAt":"2018-04-14T15:35:56Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Allowing a delta size of 64 bits is crazy. Shrink this field down to\n20 bits with one overflow bit.\n\nIf we find an existing delta larger than 1MB, we do not cache\ndelta_size at all and will get the value from oe_size(), potentially\nfrom disk if it's larger than 4GB.\n\nNote, since DELTA_SIZE() is used in try_delta() code, it must be\nthread-safe. Luckily oe_size() does guarantee this so we it is\nthread-safe.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/pack-objects.c | 26 ++++++++++++++++----------\n pack-objects.h | 23 ++++++++++++++++++++++-\n 2 files changed, 38 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex cccd0f8040..88d2bb8153 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -34,10 +34,12 @@\n #define IN_PACK(obj) oe_in_pack(&to_pack, obj)\n #define SIZE(obj) oe_size(&to_pack, obj)\n #define SET_SIZE(obj,size) oe_set_size(&to_pack, obj, size)\n+#define DELTA_SIZE(obj) oe_delta_size(&to_pack, obj)\n #define DELTA(obj) oe_delta(&to_pack, obj)\n #define DELTA_CHILD(obj) oe_delta_child(&to_pack, obj)\n #define DELTA_SIBLING(obj) oe_delta_sibling(&to_pack, obj)\n #define SET_DELTA(obj, val) oe_set_delta(&to_pack, obj, val)\n+#define SET_DELTA_SIZE(obj, val) oe_set_delta_size(&to_pack, obj, val)\n #define SET_DELTA_CHILD(obj, val) oe_set_delta_child(&to_pack, obj, val)\n #define SET_DELTA_SIBLING(obj, val) oe_set_delta_sibling(&to_pack, obj, val)\n \n@@ -144,7 +146,7 @@ static void *get_delta(struct object_entry *entry)\n \t\t oid_to_hex(&DELTA(entry)->idx.oid));\n \tdelta_buf = diff_delta(base_buf, base_size,\n \t\t\t buf, size, &delta_size, 0);\n-\tif (!delta_buf || delta_size != entry->delta_size)\n+\tif (!delta_buf || delta_size != DELTA_SIZE(entry))\n \t\tdie(\"delta size changed\");\n \tfree(buf);\n \tfree(base_buf);\n@@ -294,14 +296,14 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent\n \t\tFREE_AND_NULL(entry->delta_data);\n \t\tentry->z_delta_size = 0;\n \t} else if (entry->delta_data) {\n-\t\tsize = entry->delta_size;\n+\t\tsize = DELTA_SIZE(entry);\n \t\tbuf = entry->delta_data;\n \t\tentry->delta_data = NULL;\n \t\ttype = (allow_ofs_delta && DELTA(entry)->idx.offset) ?\n \t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n \t} else {\n \t\tbuf = get_delta(entry);\n-\t\tsize = entry->delta_size;\n+\t\tsize = DELTA_SIZE(entry);\n \t\ttype = (allow_ofs_delta && DELTA(entry)->idx.offset) ?\n \t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n \t}\n@@ -1509,7 +1511,7 @@ static void check_object(struct object_entry *entry)\n \t\t\toe_set_type(entry, entry->in_pack_type);\n \t\t\tSET_SIZE(entry, in_pack_size); /* delta size */\n \t\t\tSET_DELTA(entry, base_entry);\n-\t\t\tentry->delta_size = in_pack_size;\n+\t\t\tSET_DELTA_SIZE(entry, in_pack_size);\n \t\t\tentry->delta_sibling_idx = base_entry->delta_child_idx;\n \t\t\tSET_DELTA_CHILD(base_entry, entry);\n \t\t\tunuse_pack(&w_curs);\n@@ -1937,7 +1939,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \t\tmax_size = trg_size/2 - 20;\n \t\tref_depth = 1;\n \t} else {\n-\t\tmax_size = trg_entry->delta_size;\n+\t\tmax_size = DELTA_SIZE(trg_entry);\n \t\tref_depth = trg->depth;\n \t}\n \tmax_size = (uint64_t)max_size * (max_depth - src->depth) /\n@@ -2006,10 +2008,14 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \tdelta_buf = create_delta(src->index, trg->data, trg_size, &delta_size, max_size);\n \tif (!delta_buf)\n \t\treturn 0;\n+\tif (delta_size >= (1U << OE_DELTA_SIZE_BITS)) {\n+\t\tfree(delta_buf);\n+\t\treturn 0;\n+\t}\n \n \tif (DELTA(trg_entry)) {\n \t\t/* Prefer only shallower same-sized deltas. */\n-\t\tif (delta_size == trg_entry->delta_size &&\n+\t\tif (delta_size == DELTA_SIZE(trg_entry) &&\n \t\t src->depth + 1 >= trg->depth) {\n \t\t\tfree(delta_buf);\n \t\t\treturn 0;\n@@ -2024,7 +2030,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \tfree(trg_entry->delta_data);\n \tcache_lock();\n \tif (trg_entry->delta_data) {\n-\t\tdelta_cache_size -= trg_entry->delta_size;\n+\t\tdelta_cache_size -= DELTA_SIZE(trg_entry);\n \t\ttrg_entry->delta_data = NULL;\n \t}\n \tif (delta_cacheable(src_size, trg_size, delta_size)) {\n@@ -2037,7 +2043,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \t}\n \n \tSET_DELTA(trg_entry, src_entry);\n-\ttrg_entry->delta_size = delta_size;\n+\tSET_DELTA_SIZE(trg_entry, delta_size);\n \ttrg->depth = src->depth + 1;\n \n \treturn 1;\n@@ -2160,11 +2166,11 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,\n \t\tif (entry->delta_data && !pack_to_stdout) {\n \t\t\tunsigned long size;\n \n-\t\t\tsize = do_compress(&entry->delta_data, entry->delta_size);\n+\t\t\tsize = do_compress(&entry->delta_data, DELTA_SIZE(entry));\n \t\t\tif (size < (1U << OE_Z_DELTA_BITS)) {\n \t\t\t\tentry->z_delta_size = size;\n \t\t\t\tcache_lock();\n-\t\t\t\tdelta_cache_size -= entry->delta_size;\n+\t\t\t\tdelta_cache_size -= DELTA_SIZE(entry);\n \t\t\t\tdelta_cache_size += entry->z_delta_size;\n \t\t\t\tcache_unlock();\n \t\t\t} else {\ndiff --git a/pack-objects.h b/pack-objects.h\nindex ee2c7ab382..1c588184b2 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -12,6 +12,7 @@\n * above this limit. Don't lower it too much.\n */\n #define OE_SIZE_BITS\t\t31\n+#define OE_DELTA_SIZE_BITS\t20\n \n /*\n * State flags for depth-first search used for analyzing delta cycles.\n@@ -85,7 +86,8 @@ struct object_entry {\n \t\t\t\t * uses the same base as me\n \t\t\t\t */\n \tvoid *delta_data;\t/* cached delta (uncompressed) */\n-\tunsigned long delta_size;\t/* delta data size (uncompressed) */\n+\tunsigned delta_size_:OE_DELTA_SIZE_BITS; /* delta data size (uncompressed) */\n+\tunsigned delta_size_valid:1;\n \tunsigned z_delta_size:OE_Z_DELTA_BITS;\n \tunsigned type_:TYPE_BITS;\n \tunsigned in_pack_type:TYPE_BITS; /* could be delta */\n@@ -309,4 +311,23 @@ static inline void oe_set_size(struct packing_data *pack,\n \t}\n }\n \n+static inline unsigned long oe_delta_size(struct packing_data *pack,\n+\t\t\t\t\t const struct object_entry *e)\n+{\n+\tif (e->delta_size_valid)\n+\t\treturn e->delta_size_;\n+\treturn oe_size(pack, e);\n+}\n+\n+static inline void oe_set_delta_size(struct packing_data *pack,\n+\t\t\t\t struct object_entry *e,\n+\t\t\t\t unsigned long size)\n+{\n+\te->delta_size_ = size;\n+\te->delta_size_valid = e->delta_size_ == size;\n+\tif (!e->delta_size_valid && size != oe_size(pack, e))\n+\t\tBUG(\"this can only happen in check_object() \"\n+\t\t \"where delta size is the same as entry size\");\n+}\n+\n #endif\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344682","messageId":"20180414153513.9902-11-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 10/15] pack-objects: don't check size when the object is bad","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:08Z","receivedAt":"2018-04-14T15:36:00Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"sha1_object_info() in check_objects() may fail to locate an object in\nthe pack and return type OBJ_BAD. In that case, it will likely leave\nthe \"size\" field untouched. We delay error handling until later in\nprepare_pack() though. Until then, do not touch \"size\" field.\n\nThis field should contain the default value zero, but we can't say\nsha1_object_info() cannot damage it. This becomes more important later\nwhen the object size may have to be retrieved back from the\n(non-existing) pack.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/pack-objects.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 211bb1ad0e..e75693176e 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1742,7 +1742,7 @@ static void get_object_details(void)\n \tfor (i = 0; i < to_pack.nr_objects; i++) {\n \t\tstruct object_entry *entry = sorted_by_offset[i];\n \t\tcheck_object(entry);\n-\t\tif (big_file_threshold < entry->size)\n+\t\tif (entry->type_valid && big_file_threshold < entry->size)\n \t\t\tentry->no_try_delta = 1;\n \t}\n \n@@ -2453,7 +2453,7 @@ static void prepare_pack(int window, int depth)\n \t\t\t */\n \t\t\tcontinue;\n \n-\t\tif (entry->size < 50)\n+\t\tif (!entry->type_valid || entry->size < 50)\n \t\t\tcontinue;\n \n \t\tif (entry->no_try_delta)\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344683","messageId":"20180414153513.9902-12-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 11/15] pack-objects: clarify the use of object_entry::size","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:09Z","receivedAt":"2018-04-14T15:36:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"While this field most of the time contains the canonical object size,\nthere is one case it does not: when we have found that the base object\nof the delta in question is also to be packed, we will very happily\nreuse the delta by copying it over instead of regenerating the new\ndelta.\n\n\"size\" in this case will record the delta size, not canonical object\nsize. Later on in write_reuse_object(), we reconstruct the delta\nheader and \"size\" is used for this purpose. When this happens, the\n\"type\" field contains a delta type instead of a canonical type.\nHighlight this in the code since it could be tricky to see.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/pack-objects.c | 11 ++++++++---\n pack-objects.h | 4 +++-\n 2 files changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex e75693176e..779f14a45e 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1418,6 +1418,7 @@ static void check_object(struct object_entry *entry)\n \t\toff_t ofs;\n \t\tunsigned char *buf, c;\n \t\tenum object_type type;\n+\t\tunsigned long in_pack_size;\n \n \t\tbuf = use_pack(p, &w_curs, entry->in_pack_offset, &avail);\n \n@@ -1427,7 +1428,7 @@ static void check_object(struct object_entry *entry)\n \t\t */\n \t\tused = unpack_object_header_buffer(buf, avail,\n \t\t\t\t\t\t &type,\n-\t\t\t\t\t\t &entry->size);\n+\t\t\t\t\t\t &in_pack_size);\n \t\tif (used == 0)\n \t\t\tgoto give_up;\n \n@@ -1444,6 +1445,7 @@ static void check_object(struct object_entry *entry)\n \t\tdefault:\n \t\t\t/* Not a delta hence we've already got all we need. */\n \t\t\toe_set_type(entry, entry->in_pack_type);\n+\t\t\tentry->size = in_pack_size;\n \t\t\tentry->in_pack_header_size = used;\n \t\t\tif (oe_type(entry) < OBJ_COMMIT || oe_type(entry) > OBJ_BLOB)\n \t\t\t\tgoto give_up;\n@@ -1500,6 +1502,7 @@ static void check_object(struct object_entry *entry)\n \t\t\t * circular deltas.\n \t\t\t */\n \t\t\toe_set_type(entry, entry->in_pack_type);\n+\t\t\tentry->size = in_pack_size; /* delta size */\n \t\t\tSET_DELTA(entry, base_entry);\n \t\t\tentry->delta_size = entry->size;\n \t\t\tentry->delta_sibling_idx = base_entry->delta_child_idx;\n@@ -1509,13 +1512,15 @@ static void check_object(struct object_entry *entry)\n \t\t}\n \n \t\tif (oe_type(entry)) {\n+\t\t\toff_t delta_pos;\n+\n \t\t\t/*\n \t\t\t * This must be a delta and we already know what the\n \t\t\t * final object type is. Let's extract the actual\n \t\t\t * object size from the delta header.\n \t\t\t */\n-\t\t\tentry->size = get_size_from_delta(p, &w_curs,\n-\t\t\t\t\tentry->in_pack_offset + entry->in_pack_header_size);\n+\t\t\tdelta_pos = entry->in_pack_offset + entry->in_pack_header_size;\n+\t\t\tentry->size = get_size_from_delta(p, &w_curs, delta_pos);\n \t\t\tif (entry->size == 0)\n \t\t\t\tgoto give_up;\n \t\t\tunuse_pack(&w_curs);\ndiff --git a/pack-objects.h b/pack-objects.h\nindex 9d0391c173..e4ea6a350c 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -32,7 +32,9 @@ enum dfs_state {\n *\n * \"size\" is the uncompressed object size. Compressed size of the raw\n * data for an object in a pack is not stored anywhere but is computed\n- * and made available when reverse .idx is made.\n+ * and made available when reverse .idx is made. Note that when a\n+ * delta is reused, \"size\" is the uncompressed _delta_ size, not the\n+ * canonical one after the delta has been applied.\n *\n * \"hash\" contains a path name hash which is used for sorting the\n * delta list and also during delta searching. Once prepare_pack()\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344684","messageId":"20180414153513.9902-7-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 06/15] pack-objects: move in_pack_pos out of struct object_entry","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:04Z","receivedAt":"2018-04-14T15:36:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This field is only need for pack-bitmap, which is an optional\nfeature. Move it to a separate array that is only allocated when\npack-bitmap is used (like objects[], it is not freed, since we need it\nuntil the end of the process)\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/pack-objects.c | 3 ++-\n pack-bitmap-write.c | 8 +++++---\n pack-bitmap.c | 2 +-\n pack-bitmap.h | 4 +++-\n pack-objects.h | 16 +++++++++++++++-\n 5 files changed, 26 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex b231e80f17..d9c89e87cd 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -879,7 +879,8 @@ static void write_pack_file(void)\n \n \t\t\tif (write_bitmap_index) {\n \t\t\t\tbitmap_writer_set_checksum(oid.hash);\n-\t\t\t\tbitmap_writer_build_type_index(written_list, nr_written);\n+\t\t\t\tbitmap_writer_build_type_index(\n+\t\t\t\t\t&to_pack, written_list, nr_written);\n \t\t\t}\n \n \t\t\tfinish_tmp_packfile(&tmpname, pack_tmp_name,\ndiff --git a/pack-bitmap-write.c b/pack-bitmap-write.c\nindex 2df7b3e144..d707fc9ea2 100644\n--- a/pack-bitmap-write.c\n+++ b/pack-bitmap-write.c\n@@ -48,7 +48,8 @@ void bitmap_writer_show_progress(int show)\n /**\n * Build the initial type index for the packfile\n */\n-void bitmap_writer_build_type_index(struct pack_idx_entry **index,\n+void bitmap_writer_build_type_index(struct packing_data *to_pack,\n+\t\t\t\t struct pack_idx_entry **index,\n \t\t\t\t uint32_t index_nr)\n {\n \tuint32_t i;\n@@ -57,12 +58,13 @@ void bitmap_writer_build_type_index(struct pack_idx_entry **index,\n \twriter.trees = ewah_new();\n \twriter.blobs = ewah_new();\n \twriter.tags = ewah_new();\n+\tALLOC_ARRAY(to_pack->in_pack_pos, to_pack->nr_objects);\n \n \tfor (i = 0; i < index_nr; ++i) {\n \t\tstruct object_entry *entry = (struct object_entry *)index[i];\n \t\tenum object_type real_type;\n \n-\t\tentry->in_pack_pos = i;\n+\t\toe_set_in_pack_pos(to_pack, entry, i);\n \n \t\tswitch (oe_type(entry)) {\n \t\tcase OBJ_COMMIT:\n@@ -146,7 +148,7 @@ static uint32_t find_object_pos(const unsigned char *sha1)\n \t\t\t\"(object %s is missing)\", sha1_to_hex(sha1));\n \t}\n \n-\treturn entry->in_pack_pos;\n+\treturn oe_in_pack_pos(writer.to_pack, entry);\n }\n \n static void show_object(struct object *object, const char *name, void *data)\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 3f2dab340f..c9e90d1bb5 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -1033,7 +1033,7 @@ int rebuild_existing_bitmaps(struct packing_data *mapping,\n \t\toe = packlist_find(mapping, sha1, NULL);\n \n \t\tif (oe)\n-\t\t\treposition[i] = oe->in_pack_pos + 1;\n+\t\t\treposition[i] = oe_in_pack_pos(mapping, oe) + 1;\n \t}\n \n \trebuild = bitmap_new();\ndiff --git a/pack-bitmap.h b/pack-bitmap.h\nindex 3742a00e14..5ded2f139a 100644\n--- a/pack-bitmap.h\n+++ b/pack-bitmap.h\n@@ -44,7 +44,9 @@ int rebuild_existing_bitmaps(struct packing_data *mapping, khash_sha1 *reused_bi\n \n void bitmap_writer_show_progress(int show);\n void bitmap_writer_set_checksum(unsigned char *sha1);\n-void bitmap_writer_build_type_index(struct pack_idx_entry **index, uint32_t index_nr);\n+void bitmap_writer_build_type_index(struct packing_data *to_pack,\n+\t\t\t\t struct pack_idx_entry **index,\n+\t\t\t\t uint32_t index_nr);\n void bitmap_writer_reuse_bitmaps(struct packing_data *to_pack);\n void bitmap_writer_select_commits(struct commit **indexed_commits,\n \t\tunsigned int indexed_commits_nr, int max_bitmaps);\ndiff --git a/pack-objects.h b/pack-objects.h\nindex cdce1648de..71ea992c3c 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -79,7 +79,6 @@ struct object_entry {\n \tunsigned in_pack_type:TYPE_BITS; /* could be delta */\n \tunsigned type_valid:1;\n \tuint32_t hash;\t\t\t/* name hint hash */\n-\tunsigned int in_pack_pos;\n \tunsigned char in_pack_header_size;\n \tunsigned preferred_base:1; /*\n \t\t\t\t * we do not pack this, but is available\n@@ -99,6 +98,8 @@ struct packing_data {\n \n \tint32_t *index;\n \tuint32_t index_size;\n+\n+\tunsigned int *in_pack_pos;\n };\n \n struct object_entry *packlist_alloc(struct packing_data *pdata,\n@@ -144,4 +145,17 @@ static inline void oe_set_type(struct object_entry *e,\n \te->type_ = (unsigned)type;\n }\n \n+static inline unsigned int oe_in_pack_pos(const struct packing_data *pack,\n+\t\t\t\t\t const struct object_entry *e)\n+{\n+\treturn pack->in_pack_pos[e - pack->objects];\n+}\n+\n+static inline void oe_set_in_pack_pos(const struct packing_data *pack,\n+\t\t\t\t const struct object_entry *e,\n+\t\t\t\t unsigned int pos)\n+{\n+\tpack->in_pack_pos[e - pack->objects] = pos;\n+}\n+\n #endif\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344685","messageId":"20180414153513.9902-13-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 12/15] pack-objects: shrink size field in struct object_entry","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:10Z","receivedAt":"2018-04-14T15:36:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"It's very very rare that an uncompressed object is larger than 4GB\n(partly because Git does not handle those large files very well to\nbegin with). Let's optimize it for the common case where object size\nis smaller than this limit.\n\nShrink size field down to 31 bits and one overflow bit. If the size is\ntoo large, we read it back from disk. As noted in the previous patch,\nwe need to return the delta size instead of canonical size when the\nto-be-reused object entry type is a delta instead of a canonical one.\n\nAdd two compare helpers that can take advantage of the overflow\nbit (e.g. if the file is 4GB+, chances are it's already larger than\ncore.bigFileThreshold and there's no point in comparing the actual\nvalue).\n\nAnother note about oe_get_size_slow(). This function MUST be thread\nsafe because SIZE() macro is used inside try_delta() which may run in\nparallel. Outside parallel code, no-contention locking should be dirt\ncheap (or insignificant compared to i/o access anyway). To exercise\nthis code, it's best to run the test suite with something like\n\n make test GIT_TEST_OE_SIZE=4\n\nwhich forces this code on all objects larger than 3 bytes.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/pack-objects.c | 105 +++++++++++++++++++++++++++++++----------\n pack-objects.c | 3 ++\n pack-objects.h | 57 +++++++++++++++++++++-\n t/README | 6 +++\n 4 files changed, 145 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 779f14a45e..cccd0f8040 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -32,6 +32,8 @@\n #include \"object-store.h\"\n \n #define IN_PACK(obj) oe_in_pack(&to_pack, obj)\n+#define SIZE(obj) oe_size(&to_pack, obj)\n+#define SET_SIZE(obj,size) oe_set_size(&to_pack, obj, size)\n #define DELTA(obj) oe_delta(&to_pack, obj)\n #define DELTA_CHILD(obj) oe_delta_child(&to_pack, obj)\n #define DELTA_SIBLING(obj) oe_delta_sibling(&to_pack, obj)\n@@ -276,7 +278,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent\n \n \tif (!usable_delta) {\n \t\tif (oe_type(entry) == OBJ_BLOB &&\n-\t\t entry->size > big_file_threshold &&\n+\t\t oe_size_greater_than(&to_pack, entry, big_file_threshold) &&\n \t\t (st = open_istream(&entry->idx.oid, &type, &size, NULL)) != NULL)\n \t\t\tbuf = NULL;\n \t\telse {\n@@ -385,12 +387,13 @@ static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,\n \tunsigned char header[MAX_PACK_OBJECT_HEADER],\n \t\t dheader[MAX_PACK_OBJECT_HEADER];\n \tunsigned hdrlen;\n+\tunsigned long entry_size = SIZE(entry);\n \n \tif (DELTA(entry))\n \t\ttype = (allow_ofs_delta && DELTA(entry)->idx.offset) ?\n \t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n \thdrlen = encode_in_pack_object_header(header, sizeof(header),\n-\t\t\t\t\t type, entry->size);\n+\t\t\t\t\t type, entry_size);\n \n \toffset = entry->in_pack_offset;\n \trevidx = find_pack_revindex(p, offset);\n@@ -407,7 +410,7 @@ static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,\n \tdatalen -= entry->in_pack_header_size;\n \n \tif (!pack_to_stdout && p->index_version == 1 &&\n-\t check_pack_inflate(p, &w_curs, offset, datalen, entry->size)) {\n+\t check_pack_inflate(p, &w_curs, offset, datalen, entry_size)) {\n \t\terror(\"corrupt packed object for %s\",\n \t\t oid_to_hex(&entry->idx.oid));\n \t\tunuse_pack(&w_curs);\n@@ -1408,6 +1411,8 @@ static void cleanup_preferred_base(void)\n \n static void check_object(struct object_entry *entry)\n {\n+\tunsigned long canonical_size;\n+\n \tif (IN_PACK(entry)) {\n \t\tstruct packed_git *p = IN_PACK(entry);\n \t\tstruct pack_window *w_curs = NULL;\n@@ -1445,7 +1450,7 @@ static void check_object(struct object_entry *entry)\n \t\tdefault:\n \t\t\t/* Not a delta hence we've already got all we need. */\n \t\t\toe_set_type(entry, entry->in_pack_type);\n-\t\t\tentry->size = in_pack_size;\n+\t\t\tSET_SIZE(entry, in_pack_size);\n \t\t\tentry->in_pack_header_size = used;\n \t\t\tif (oe_type(entry) < OBJ_COMMIT || oe_type(entry) > OBJ_BLOB)\n \t\t\t\tgoto give_up;\n@@ -1502,9 +1507,9 @@ static void check_object(struct object_entry *entry)\n \t\t\t * circular deltas.\n \t\t\t */\n \t\t\toe_set_type(entry, entry->in_pack_type);\n-\t\t\tentry->size = in_pack_size; /* delta size */\n+\t\t\tSET_SIZE(entry, in_pack_size); /* delta size */\n \t\t\tSET_DELTA(entry, base_entry);\n-\t\t\tentry->delta_size = entry->size;\n+\t\t\tentry->delta_size = in_pack_size;\n \t\t\tentry->delta_sibling_idx = base_entry->delta_child_idx;\n \t\t\tSET_DELTA_CHILD(base_entry, entry);\n \t\t\tunuse_pack(&w_curs);\n@@ -1520,9 +1525,10 @@ static void check_object(struct object_entry *entry)\n \t\t\t * object size from the delta header.\n \t\t\t */\n \t\t\tdelta_pos = entry->in_pack_offset + entry->in_pack_header_size;\n-\t\t\tentry->size = get_size_from_delta(p, &w_curs, delta_pos);\n-\t\t\tif (entry->size == 0)\n+\t\t\tcanonical_size = get_size_from_delta(p, &w_curs, delta_pos);\n+\t\t\tif (canonical_size == 0)\n \t\t\t\tgoto give_up;\n+\t\t\tSET_SIZE(entry, canonical_size);\n \t\t\tunuse_pack(&w_curs);\n \t\t\treturn;\n \t\t}\n@@ -1536,13 +1542,17 @@ static void check_object(struct object_entry *entry)\n \t\tunuse_pack(&w_curs);\n \t}\n \n-\toe_set_type(entry, oid_object_info(&entry->idx.oid, &entry->size));\n-\t/*\n-\t * The error condition is checked in prepare_pack(). This is\n-\t * to permit a missing preferred base object to be ignored\n-\t * as a preferred base. Doing so can result in a larger\n-\t * pack file, but the transfer will still take place.\n-\t */\n+\toe_set_type(entry, oid_object_info(&entry->idx.oid, &canonical_size));\n+\tif (entry->type_valid) {\n+\t\tSET_SIZE(entry, canonical_size);\n+\t} else {\n+\t\t/*\n+\t\t * Bad object type is checked in prepare_pack(). This is\n+\t\t * to permit a missing preferred base object to be ignored\n+\t\t * as a preferred base. Doing so can result in a larger\n+\t\t * pack file, but the transfer will still take place.\n+\t\t */\n+\t}\n }\n \n static int pack_offset_sort(const void *_a, const void *_b)\n@@ -1582,6 +1592,7 @@ static void drop_reused_delta(struct object_entry *entry)\n \tunsigned *idx = &to_pack.objects[entry->delta_idx - 1].delta_child_idx;\n \tstruct object_info oi = OBJECT_INFO_INIT;\n \tenum object_type type;\n+\tunsigned long size;\n \n \twhile (*idx) {\n \t\tstruct object_entry *oe = &to_pack.objects[*idx - 1];\n@@ -1594,7 +1605,7 @@ static void drop_reused_delta(struct object_entry *entry)\n \tSET_DELTA(entry, NULL);\n \tentry->depth = 0;\n \n-\toi.sizep = &entry->size;\n+\toi.sizep = &size;\n \toi.typep = &type;\n \tif (packed_object_info(IN_PACK(entry), entry->in_pack_offset, &oi) < 0) {\n \t\t/*\n@@ -1603,11 +1614,11 @@ static void drop_reused_delta(struct object_entry *entry)\n \t\t * And if that fails, the error will be recorded in oe_type(entry)\n \t\t * and dealt with in prepare_pack().\n \t\t */\n-\t\toe_set_type(entry, oid_object_info(&entry->idx.oid,\n-\t\t\t\t\t\t &entry->size));\n+\t\toe_set_type(entry, oid_object_info(&entry->idx.oid, &size));\n \t} else {\n \t\toe_set_type(entry, type);\n \t}\n+\tSET_SIZE(entry, size);\n }\n \n /*\n@@ -1747,7 +1758,8 @@ static void get_object_details(void)\n \tfor (i = 0; i < to_pack.nr_objects; i++) {\n \t\tstruct object_entry *entry = sorted_by_offset[i];\n \t\tcheck_object(entry);\n-\t\tif (entry->type_valid && big_file_threshold < entry->size)\n+\t\tif (entry->type_valid &&\n+\t\t oe_size_greater_than(&to_pack, entry, big_file_threshold))\n \t\t\tentry->no_try_delta = 1;\n \t}\n \n@@ -1776,6 +1788,8 @@ static int type_size_sort(const void *_a, const void *_b)\n \tconst struct object_entry *b = *(struct object_entry **)_b;\n \tenum object_type a_type = oe_type(a);\n \tenum object_type b_type = oe_type(b);\n+\tunsigned long a_size = SIZE(a);\n+\tunsigned long b_size = SIZE(b);\n \n \tif (a_type > b_type)\n \t\treturn -1;\n@@ -1789,9 +1803,9 @@ static int type_size_sort(const void *_a, const void *_b)\n \t\treturn -1;\n \tif (a->preferred_base < b->preferred_base)\n \t\treturn 1;\n-\tif (a->size > b->size)\n+\tif (a_size > b_size)\n \t\treturn -1;\n-\tif (a->size < b->size)\n+\tif (a_size < b_size)\n \t\treturn 1;\n \treturn a < b ? -1 : (a > b); /* newest first */\n }\n@@ -1844,6 +1858,46 @@ static pthread_mutex_t progress_mutex;\n \n #endif\n \n+/*\n+ * Return the size of the object without doing any delta\n+ * reconstruction (so non-deltas are true object sizes, but deltas\n+ * return the size of the delta data).\n+ */\n+unsigned long oe_get_size_slow(struct packing_data *pack,\n+\t\t\t const struct object_entry *e)\n+{\n+\tstruct packed_git *p;\n+\tstruct pack_window *w_curs;\n+\tunsigned char *buf;\n+\tenum object_type type;\n+\tunsigned long used, avail, size;\n+\n+\tif (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) {\n+\t\tread_lock();\n+\t\tif (oid_object_info(&e->idx.oid, &size) < 0)\n+\t\t\tdie(_(\"unable to get size of %s\"),\n+\t\t\t oid_to_hex(&e->idx.oid));\n+\t\tread_unlock();\n+\t\treturn size;\n+\t}\n+\n+\tp = oe_in_pack(pack, e);\n+\tif (!p)\n+\t\tBUG(\"when e->type is a delta, it must belong to a pack\");\n+\n+\tread_lock();\n+\tw_curs = NULL;\n+\tbuf = use_pack(p, &w_curs, e->in_pack_offset, &avail);\n+\tused = unpack_object_header_buffer(buf, avail, &type, &size);\n+\tif (used == 0)\n+\t\tdie(_(\"unable to parse object header of %s\"),\n+\t\t oid_to_hex(&e->idx.oid));\n+\n+\tunuse_pack(&w_curs);\n+\tread_unlock();\n+\treturn size;\n+}\n+\n static int try_delta(struct unpacked *trg, struct unpacked *src,\n \t\t unsigned max_depth, unsigned long *mem_usage)\n {\n@@ -1878,7 +1932,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \t\treturn 0;\n \n \t/* Now some size filtering heuristics. */\n-\ttrg_size = trg_entry->size;\n+\ttrg_size = SIZE(trg_entry);\n \tif (!DELTA(trg_entry)) {\n \t\tmax_size = trg_size/2 - 20;\n \t\tref_depth = 1;\n@@ -1890,7 +1944,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \t\t\t\t\t\t(max_depth - ref_depth + 1);\n \tif (max_size == 0)\n \t\treturn 0;\n-\tsrc_size = src_entry->size;\n+\tsrc_size = SIZE(src_entry);\n \tsizediff = src_size < trg_size ? trg_size - src_size : 0;\n \tif (sizediff >= max_size)\n \t\treturn 0;\n@@ -2008,7 +2062,7 @@ static unsigned long free_unpacked(struct unpacked *n)\n \tfree_delta_index(n->index);\n \tn->index = NULL;\n \tif (n->data) {\n-\t\tfreed_mem += n->entry->size;\n+\t\tfreed_mem += SIZE(n->entry);\n \t\tFREE_AND_NULL(n->data);\n \t}\n \tn->entry = NULL;\n@@ -2458,7 +2512,8 @@ static void prepare_pack(int window, int depth)\n \t\t\t */\n \t\t\tcontinue;\n \n-\t\tif (!entry->type_valid || entry->size < 50)\n+\t\tif (!entry->type_valid ||\n+\t\t oe_size_less_than(&to_pack, entry, 50))\n \t\t\tcontinue;\n \n \t\tif (entry->no_try_delta)\ndiff --git a/pack-objects.c b/pack-objects.c\nindex 08cfe68b6b..e0c4600568 100644\n--- a/pack-objects.c\n+++ b/pack-objects.c\n@@ -143,6 +143,9 @@ void prepare_packing_data(struct packing_data *pdata)\n \t} else {\n \t\tprepare_in_pack_by_idx(pdata);\n \t}\n+\n+\tpdata->oe_size_limit = git_env_ulong(\"GIT_TEST_OE_SIZE\",\n+\t\t\t\t\t 1U << OE_SIZE_BITS);\n }\n \n struct object_entry *packlist_alloc(struct packing_data *pdata,\ndiff --git a/pack-objects.h b/pack-objects.h\nindex e4ea6a350c..ee2c7ab382 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -7,6 +7,11 @@\n #define OE_DEPTH_BITS\t\t12\n #define OE_IN_PACK_BITS\t\t10\n #define OE_Z_DELTA_BITS\t\t20\n+/*\n+ * Note that oe_set_size() becomes expensive when the given size is\n+ * above this limit. Don't lower it too much.\n+ */\n+#define OE_SIZE_BITS\t\t31\n \n /*\n * State flags for depth-first search used for analyzing delta cycles.\n@@ -70,7 +75,8 @@ enum dfs_state {\n */\n struct object_entry {\n \tstruct pack_idx_entry idx;\n-\tunsigned long size;\t/* uncompressed size */\n+\tunsigned size_:OE_SIZE_BITS;\n+\tunsigned size_valid:1;\n \tunsigned in_pack_idx:OE_IN_PACK_BITS;\t/* already in pack */\n \toff_t in_pack_offset;\n \tuint32_t delta_idx;\t/* delta base object */\n@@ -115,6 +121,8 @@ struct packing_data {\n \t */\n \tstruct packed_git **in_pack_by_idx;\n \tstruct packed_git **in_pack;\n+\n+\tuintmax_t oe_size_limit;\n };\n \n void prepare_packing_data(struct packing_data *pdata);\n@@ -254,4 +262,51 @@ static inline void oe_set_delta_sibling(struct packing_data *pack,\n \t\te->delta_sibling_idx = 0;\n }\n \n+unsigned long oe_get_size_slow(struct packing_data *pack,\n+\t\t\t const struct object_entry *e);\n+static inline unsigned long oe_size(struct packing_data *pack,\n+\t\t\t\t const struct object_entry *e)\n+{\n+\tif (e->size_valid)\n+\t\treturn e->size_;\n+\n+\treturn oe_get_size_slow(pack, e);\n+}\n+\n+static inline int oe_size_less_than(struct packing_data *pack,\n+\t\t\t\t const struct object_entry *lhs,\n+\t\t\t\t unsigned long rhs)\n+{\n+\tif (lhs->size_valid)\n+\t\treturn lhs->size_ < rhs;\n+\tif (rhs < pack->oe_size_limit) /* rhs < 2^x <= lhs ? */\n+\t\treturn 0;\n+\treturn oe_get_size_slow(pack, lhs) < rhs;\n+}\n+\n+static inline int oe_size_greater_than(struct packing_data *pack,\n+\t\t\t\t const struct object_entry *lhs,\n+\t\t\t\t unsigned long rhs)\n+{\n+\tif (lhs->size_valid)\n+\t\treturn lhs->size_ > rhs;\n+\tif (rhs < pack->oe_size_limit) /* rhs < 2^x <= lhs ? */\n+\t\treturn 1;\n+\treturn oe_get_size_slow(pack, lhs) > rhs;\n+}\n+\n+static inline void oe_set_size(struct packing_data *pack,\n+\t\t\t struct object_entry *e,\n+\t\t\t unsigned long size)\n+{\n+\tif (size < pack->oe_size_limit) {\n+\t\te->size_ = size;\n+\t\te->size_valid = 1;\n+\t} else {\n+\t\te->size_valid = 0;\n+\t\tif (oe_get_size_slow(pack, e) != size)\n+\t\t\tBUG(\"'size' is supposed to be the object size!\");\n+\t}\n+}\n+\n #endif\ndiff --git a/t/README b/t/README\nindex a06a85bae8..8373a27fea 100644\n--- a/t/README\n+++ b/t/README\n@@ -309,6 +309,12 @@ pack-objects code path where there are more than 1024 packs even if\n the actual number of packs in repository is below this limit. Accept\n any boolean values that are accepted by git-config.\n \n+GIT_TEST_OE_SIZE= exercises the uncommon pack-objects code path\n+where we do not cache object size in memory and read it from existing\n+packs on demand. This normally only happens when the object size is\n+over 2GB. This variable forces the code path on any object larger than\n+ bytes.\n+\n Naming Tests\n ------------\n \n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344686","messageId":"20180414153513.9902-4-pclouds@gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-1-pclouds@gmail.com","subject":"[PATCH 03/15] pack-objects: turn type and in_pack_type to bitfields","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-04-14T15:35:01Z","receivedAt":"2018-04-14T15:36:17Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"An extra field type_valid is added to carry the equivalent of OBJ_BAD\nin the original \"type\" field. in_pack_type always contains a valid\ntype so we only need 3 bits for it.\n\nA note about accepting OBJ_NONE as \"valid\" type. The function\nread_object_list_from_stdin() can pass this value [1] and it\neventually calls create_object_entry() where current code skip setting\n\"type\" field if the incoming type is zero. This does not have any bad\nside effects because \"type\" field should be memset()'d anyway.\n\nBut since we also need to set type_valid now, skipping oe_set_type()\nleaves type_valid zero/false, which will make oe_type() return\nOBJ_BAD, not OBJ_NONE anymore. Apparently we do care about OBJ_NONE in\nprepare_pack(). This switch from OBJ_NONE to OBJ_BAD may trigger\n\n fatal: unable to get type of object ...\n\nAccepting OBJ_NONE [2] does sound wrong, but this is how it is has\nbeen for a very long time and I haven't time to dig in further.\n\n[1] See 5c49c11686 (pack-objects: better check_object() performances -\n 2007-04-16)\n\n[2] 21666f1aae (convert object type handling from a string to a number\n - 2007-02-26)\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/pack-objects.c | 59 +++++++++++++++++++++++++-----------------\n cache.h | 2 ++\n object.h | 1 -\n pack-bitmap-write.c | 6 ++---\n pack-objects.h | 20 ++++++++++++--\n 5 files changed, 58 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 4bdae5a1d8..88877f1f59 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -266,7 +266,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent\n \tstruct git_istream *st = NULL;\n \n \tif (!usable_delta) {\n-\t\tif (entry->type == OBJ_BLOB &&\n+\t\tif (oe_type(entry) == OBJ_BLOB &&\n \t\t entry->size > big_file_threshold &&\n \t\t (st = open_istream(&entry->idx.oid, &type, &size, NULL)) != NULL)\n \t\t\tbuf = NULL;\n@@ -371,7 +371,7 @@ static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,\n \tstruct pack_window *w_curs = NULL;\n \tstruct revindex_entry *revidx;\n \toff_t offset;\n-\tenum object_type type = entry->type;\n+\tenum object_type type = oe_type(entry);\n \toff_t datalen;\n \tunsigned char header[MAX_PACK_OBJECT_HEADER],\n \t\t dheader[MAX_PACK_OBJECT_HEADER];\n@@ -480,11 +480,12 @@ static off_t write_object(struct hashfile *f,\n \t\tto_reuse = 0;\t/* explicit */\n \telse if (!entry->in_pack)\n \t\tto_reuse = 0;\t/* can't reuse what we don't have */\n-\telse if (entry->type == OBJ_REF_DELTA || entry->type == OBJ_OFS_DELTA)\n+\telse if (oe_type(entry) == OBJ_REF_DELTA ||\n+\t\t oe_type(entry) == OBJ_OFS_DELTA)\n \t\t\t\t/* check_object() decided it for us ... */\n \t\tto_reuse = usable_delta;\n \t\t\t\t/* ... but pack split may override that */\n-\telse if (entry->type != entry->in_pack_type)\n+\telse if (oe_type(entry) != entry->in_pack_type)\n \t\tto_reuse = 0;\t/* pack has delta which is unusable */\n \telse if (entry->delta)\n \t\tto_reuse = 0;\t/* we want to pack afresh */\n@@ -705,8 +706,8 @@ static struct object_entry **compute_write_order(void)\n \t * And then all remaining commits and tags.\n \t */\n \tfor (i = last_untagged; i < to_pack.nr_objects; i++) {\n-\t\tif (objects[i].type != OBJ_COMMIT &&\n-\t\t objects[i].type != OBJ_TAG)\n+\t\tif (oe_type(&objects[i]) != OBJ_COMMIT &&\n+\t\t oe_type(&objects[i]) != OBJ_TAG)\n \t\t\tcontinue;\n \t\tadd_to_write_order(wo, &wo_end, &objects[i]);\n \t}\n@@ -715,7 +716,7 @@ static struct object_entry **compute_write_order(void)\n \t * And then all the trees.\n \t */\n \tfor (i = last_untagged; i < to_pack.nr_objects; i++) {\n-\t\tif (objects[i].type != OBJ_TREE)\n+\t\tif (oe_type(&objects[i]) != OBJ_TREE)\n \t\t\tcontinue;\n \t\tadd_to_write_order(wo, &wo_end, &objects[i]);\n \t}\n@@ -1066,8 +1067,7 @@ static void create_object_entry(const struct object_id *oid,\n \n \tentry = packlist_alloc(&to_pack, oid->hash, index_pos);\n \tentry->hash = hash;\n-\tif (type)\n-\t\tentry->type = type;\n+\toe_set_type(entry, type);\n \tif (exclude)\n \t\tentry->preferred_base = 1;\n \telse\n@@ -1407,6 +1407,7 @@ static void check_object(struct object_entry *entry)\n \t\tunsigned long avail;\n \t\toff_t ofs;\n \t\tunsigned char *buf, c;\n+\t\tenum object_type type;\n \n \t\tbuf = use_pack(p, &w_curs, entry->in_pack_offset, &avail);\n \n@@ -1415,11 +1416,15 @@ static void check_object(struct object_entry *entry)\n \t\t * since non-delta representations could still be reused.\n \t\t */\n \t\tused = unpack_object_header_buffer(buf, avail,\n-\t\t\t\t\t\t &entry->in_pack_type,\n+\t\t\t\t\t\t &type,\n \t\t\t\t\t\t &entry->size);\n \t\tif (used == 0)\n \t\t\tgoto give_up;\n \n+\t\tif (type < 0)\n+\t\t\tBUG(\"invalid type %d\", type);\n+\t\tentry->in_pack_type = type;\n+\n \t\t/*\n \t\t * Determine if this is a delta and if so whether we can\n \t\t * reuse it or not. Otherwise let's find out as cheaply as\n@@ -1428,9 +1433,9 @@ static void check_object(struct object_entry *entry)\n \t\tswitch (entry->in_pack_type) {\n \t\tdefault:\n \t\t\t/* Not a delta hence we've already got all we need. */\n-\t\t\tentry->type = entry->in_pack_type;\n+\t\t\toe_set_type(entry, entry->in_pack_type);\n \t\t\tentry->in_pack_header_size = used;\n-\t\t\tif (entry->type < OBJ_COMMIT || entry->type > OBJ_BLOB)\n+\t\t\tif (oe_type(entry) < OBJ_COMMIT || oe_type(entry) > OBJ_BLOB)\n \t\t\t\tgoto give_up;\n \t\t\tunuse_pack(&w_curs);\n \t\t\treturn;\n@@ -1484,7 +1489,7 @@ static void check_object(struct object_entry *entry)\n \t\t\t * deltify other objects against, in order to avoid\n \t\t\t * circular deltas.\n \t\t\t */\n-\t\t\tentry->type = entry->in_pack_type;\n+\t\t\toe_set_type(entry, entry->in_pack_type);\n \t\t\tentry->delta = base_entry;\n \t\t\tentry->delta_size = entry->size;\n \t\t\tentry->delta_sibling = base_entry->delta_child;\n@@ -1493,7 +1498,7 @@ static void check_object(struct object_entry *entry)\n \t\t\treturn;\n \t\t}\n \n-\t\tif (entry->type) {\n+\t\tif (oe_type(entry)) {\n \t\t\t/*\n \t\t\t * This must be a delta and we already know what the\n \t\t\t * final object type is. Let's extract the actual\n@@ -1516,7 +1521,7 @@ static void check_object(struct object_entry *entry)\n \t\tunuse_pack(&w_curs);\n \t}\n \n-\tentry->type = oid_object_info(&entry->idx.oid, &entry->size);\n+\toe_set_type(entry, oid_object_info(&entry->idx.oid, &entry->size));\n \t/*\n \t * The error condition is checked in prepare_pack(). This is\n \t * to permit a missing preferred base object to be ignored\n@@ -1559,6 +1564,7 @@ static void drop_reused_delta(struct object_entry *entry)\n {\n \tstruct object_entry **p = &entry->delta->delta_child;\n \tstruct object_info oi = OBJECT_INFO_INIT;\n+\tenum object_type type;\n \n \twhile (*p) {\n \t\tif (*p == entry)\n@@ -1570,15 +1576,18 @@ static void drop_reused_delta(struct object_entry *entry)\n \tentry->depth = 0;\n \n \toi.sizep = &entry->size;\n-\toi.typep = &entry->type;\n+\toi.typep = &type;\n \tif (packed_object_info(entry->in_pack, entry->in_pack_offset, &oi) < 0) {\n \t\t/*\n \t\t * We failed to get the info from this pack for some reason;\n \t\t * fall back to sha1_object_info, which may find another copy.\n-\t\t * And if that fails, the error will be recorded in entry->type\n+\t\t * And if that fails, the error will be recorded in oe_type(entry)\n \t\t * and dealt with in prepare_pack().\n \t\t */\n-\t\tentry->type = oid_object_info(&entry->idx.oid, &entry->size);\n+\t\toe_set_type(entry, oid_object_info(&entry->idx.oid,\n+\t\t\t\t\t\t &entry->size));\n+\t} else {\n+\t\toe_set_type(entry, type);\n \t}\n }\n \n@@ -1746,10 +1755,12 @@ static int type_size_sort(const void *_a, const void *_b)\n {\n \tconst struct object_entry *a = *(struct object_entry **)_a;\n \tconst struct object_entry *b = *(struct object_entry **)_b;\n+\tenum object_type a_type = oe_type(a);\n+\tenum object_type b_type = oe_type(b);\n \n-\tif (a->type > b->type)\n+\tif (a_type > b_type)\n \t\treturn -1;\n-\tif (a->type < b->type)\n+\tif (a_type < b_type)\n \t\treturn 1;\n \tif (a->hash > b->hash)\n \t\treturn -1;\n@@ -1825,7 +1836,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \tvoid *delta_buf;\n \n \t/* Don't bother doing diffs between different types */\n-\tif (trg_entry->type != src_entry->type)\n+\tif (oe_type(trg_entry) != oe_type(src_entry))\n \t\treturn -1;\n \n \t/*\n@@ -2429,11 +2440,11 @@ static void prepare_pack(int window, int depth)\n \n \t\tif (!entry->preferred_base) {\n \t\t\tnr_deltas++;\n-\t\t\tif (entry->type < 0)\n+\t\t\tif (oe_type(entry) < 0)\n \t\t\t\tdie(\"unable to get type of object %s\",\n \t\t\t\t oid_to_hex(&entry->idx.oid));\n \t\t} else {\n-\t\t\tif (entry->type < 0) {\n+\t\t\tif (oe_type(entry) < 0) {\n \t\t\t\t/*\n \t\t\t\t * This object is not found, but we\n \t\t\t\t * don't have to include it anyway.\n@@ -2542,7 +2553,7 @@ static void read_object_list_from_stdin(void)\n \t\t\tdie(\"expected object ID, got garbage:\\n %s\", line);\n \n \t\tadd_preferred_base_object(p + 1);\n-\t\tadd_object_entry(&oid, 0, p + 1, 0);\n+\t\tadd_object_entry(&oid, OBJ_NONE, p + 1, 0);\n \t}\n }\n \ndiff --git a/cache.h b/cache.h\nindex bbaf5c349a..543bb42ce8 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -373,6 +373,8 @@ extern void free_name_hash(struct index_state *istate);\n #define read_blob_data_from_cache(path, sz) read_blob_data_from_index(&the_index, (path), (sz))\n #endif\n \n+#define TYPE_BITS 3\n+\n enum object_type {\n \tOBJ_BAD = -1,\n \tOBJ_NONE = 0,\ndiff --git a/object.h b/object.h\nindex f13f85b2a9..3d305fe868 100644\n--- a/object.h\n+++ b/object.h\n@@ -25,7 +25,6 @@ struct object_array {\n \n #define OBJECT_ARRAY_INIT { 0, 0, NULL }\n \n-#define TYPE_BITS 3\n /*\n * object flag allocation:\n * revision.h: 0---------10 26\ndiff --git a/pack-bitmap-write.c b/pack-bitmap-write.c\nindex 41ae27fb19..2df7b3e144 100644\n--- a/pack-bitmap-write.c\n+++ b/pack-bitmap-write.c\n@@ -64,12 +64,12 @@ void bitmap_writer_build_type_index(struct pack_idx_entry **index,\n \n \t\tentry->in_pack_pos = i;\n \n-\t\tswitch (entry->type) {\n+\t\tswitch (oe_type(entry)) {\n \t\tcase OBJ_COMMIT:\n \t\tcase OBJ_TREE:\n \t\tcase OBJ_BLOB:\n \t\tcase OBJ_TAG:\n-\t\t\treal_type = entry->type;\n+\t\t\treal_type = oe_type(entry);\n \t\t\tbreak;\n \n \t\tdefault:\n@@ -97,7 +97,7 @@ void bitmap_writer_build_type_index(struct pack_idx_entry **index,\n \t\tdefault:\n \t\t\tdie(\"Missing type information for %s (%d/%d)\",\n \t\t\t oid_to_hex(&entry->idx.oid), real_type,\n-\t\t\t entry->type);\n+\t\t\t oe_type(entry));\n \t\t}\n \t}\n }\ndiff --git a/pack-objects.h b/pack-objects.h\nindex c0a1f61aac..b4a83a6123 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -59,8 +59,9 @@ struct object_entry {\n \tvoid *delta_data;\t/* cached delta (uncompressed) */\n \tunsigned long delta_size;\t/* delta data size (uncompressed) */\n \tunsigned long z_delta_size;\t/* delta data size (compressed) */\n-\tenum object_type type;\n-\tenum object_type in_pack_type;\t/* could be delta */\n+\tunsigned type_:TYPE_BITS;\n+\tunsigned in_pack_type:TYPE_BITS; /* could be delta */\n+\tunsigned type_valid:1;\n \tuint32_t hash;\t\t\t/* name hint hash */\n \tunsigned int in_pack_pos;\n \tunsigned char in_pack_header_size;\n@@ -123,4 +124,19 @@ static inline uint32_t pack_name_hash(const char *name)\n \treturn hash;\n }\n \n+static inline enum object_type oe_type(const struct object_entry *e)\n+{\n+\treturn e->type_valid ? e->type_ : OBJ_BAD;\n+}\n+\n+static inline void oe_set_type(struct object_entry *e,\n+\t\t\t enum object_type type)\n+{\n+\tif (type >= OBJ_ANY)\n+\t\tBUG(\"OBJ_ANY cannot be set in pack-objects code\");\n+\n+\te->type_valid = type >= OBJ_NONE;\n+\te->type_ = (unsigned)type;\n+}\n+\n #endif\n-- \n2.17.0.367.g5dd2e386c3\n\n"},{"id":"344699","messageId":"877ep9fttx.fsf@evledraar.gmail.com","threadId":"48286","inReplyTo":"20180414153513.9902-2-pclouds@gmail.com","subject":"Re: [PATCH 01/15] read-cache.c: make $GIT_TEST_SPLIT_INDEX boolean","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-04-14T19:53:30Z","receivedAt":"2018-04-14T19:53:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Apr 14 2018, Nguyễn Thái Ngọc Duy wrote:\n\n> While at there, document about this special mode when running the test\n> suite.\n\nThis whole series looks good to me, and I see this patch addressed the\nconfusing test env variable situation I pointed out in v8\n(https://public-inbox.org/git/87d0zkw8r9.fsf@evledraar.gmail.com/),\nthanks!\n"}]}