git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: Nicolas Pitre <nico@fluxnic.net>
Cc: git@vger.kernel.org, "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH 12/17] pack-objects: do not add type OBJ_NONE to objects[] in pack v4
Date: Sat, 21 Sep 2013 20:57:58 +0700	[thread overview]
Message-ID: <1379771883-10278-13-git-send-email-pclouds@gmail.com> (raw)
In-Reply-To: <1379771883-10278-1-git-send-email-pclouds@gmail.com>

This is a longer explation of what is noted in the patch. When object
names are received from stdin, we lazily put OBJ_NONE as type to
objects[]. check_object() is called for each entry in objects[] later,
when it checks for ref-delta and ofs-delta for straight copy to the
pack later.

In pack v4, we don't store commits and trees as ref-delta and we need
a way to know those are delta objects in pack v2 sources are actually
commits or trees. We detect that with "type" field in struct
object_entry, which is correctly filled when --revs is passed. Without
--revs, the "type" field would be OBJ_NONE and we would need another
sha1_object_info() or similar call to detect the true object type.

Because we need object type anyway in this code path for building up
ident and path dictionaries, fill correct type as well.

Without this, the condition "pack_version < 4 || entry->type !=
OBJ_TREE" in check_object() is always true when --revs is not used,
and we will encode trees as OBJ_REF_DELTA with the base either
OBJ_TREE or OBJ_PV4_TREE. That kills pv4 tree format advantages.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/pack-objects.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 1e0c2e6..01954cb 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1496,6 +1496,7 @@ static void check_object(struct object_entry *entry)
 			 */
 			if (pack_version < 4 || entry->type != OBJ_TREE)
 				entry->type = entry->in_pack_type;
+			assert(entry->type != OBJ_NONE);
 			entry->delta = base_entry;
 			entry->delta_size = entry->size;
 			entry->delta_sibling = base_entry->delta_child;
@@ -2361,7 +2362,6 @@ static void read_object_list_from_stdin(void)
 			die("expected sha1, got garbage:\n %s", line);
 
 		add_preferred_base_object(line+41);
-		add_object_entry(sha1, 0, line+41, 0);
 
 		if (pack_version == 4) {
 			void *data;
@@ -2370,7 +2370,17 @@ static void read_object_list_from_stdin(void)
 			int (*add_dict_entries)(struct dict_table *, void *, unsigned long);
 			struct dict_table *dict;
 
-			switch (sha1_object_info(sha1, &size)) {
+			type = sha1_object_info(sha1, &size);
+			/*
+			 * In v2, we can afford to keep entry->type ==
+			 * OBJ_NONE and check_object() will fill it
+			 * later. In v4, we need to know the type
+			 * right now, don't waste time looking for
+			 * type again later in check_object() when the
+			 * in-pack type is ref-delta.
+			 */
+			add_object_entry(sha1, type, line+41, 0);
+			switch (type) {
 			case OBJ_COMMIT:
 				add_dict_entries = add_commit_dict_entries;
 				dict = v4.commit_ident_table;
@@ -2389,7 +2399,8 @@ static void read_object_list_from_stdin(void)
 				die("can't process %s object %s",
 				    typename(type), sha1_to_hex(sha1));
 			free(data);
-		}
+		} else
+			add_object_entry(sha1, 0, line+41, 0);
 	}
 }
 
-- 
1.8.2.83.gc99314b

  parent reply	other threads:[~2013-09-21 13:55 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-21 13:57 [PATCH 00/17] np/pack-v4 updates Nguyễn Thái Ngọc Duy
2013-09-21 13:57 ` [PATCH 01/17] fixup! index-pack: record all delta bases in v4 (tree and ref-delta) Nguyễn Thái Ngọc Duy
2013-09-21 13:57 ` [PATCH 02/17] fixup! packv4-parse.c: add tree offset caching Nguyễn Thái Ngọc Duy
2013-09-21 13:57 ` [PATCH 03/17] fixup! pack-objects: support writing pack v4 Nguyễn Thái Ngọc Duy
2013-09-21 13:57 ` [PATCH 04/17] fixup! pack-objects: recognize v4 as pack source Nguyễn Thái Ngọc Duy
2013-09-21 13:57 ` [PATCH 05/17] fixup! index-pack: support completing thin packs v4 Nguyễn Thái Ngọc Duy
2013-09-21 13:57 ` [PATCH 06/17] fixup! pack v4: move packv4-create.c to libgit.a Nguyễn Thái Ngọc Duy
2013-09-21 13:57 ` [PATCH 07/17] fixup! index-pack, pack-objects: allow creating .idx v2 with .pack v4 Nguyễn Thái Ngọc Duy
2013-09-22  1:39   ` Nicolas Pitre
2013-09-21 13:57 ` [PATCH 08/17] fixup! pack v4: code to obtain a SHA1 from a sha1ref Nguyễn Thái Ngọc Duy
2013-09-21 13:57 ` [PATCH 09/17] fixup! pack-objects: add --version to specify written pack version Nguyễn Thái Ngọc Duy
2013-09-21 13:57 ` [PATCH 10/17] test-lib.sh: add --packv4 for running the tests with pack v4 as default Nguyễn Thái Ngọc Duy
2013-09-21 13:57 ` [PATCH 11/17] packv4-parse: accept ref-delta as base of pv4-tree Nguyễn Thái Ngọc Duy
2013-09-21 13:57 ` Nguyễn Thái Ngọc Duy [this message]
2013-09-21 13:57 ` [PATCH 13/17] index-pack: encode appended trees using v4 format in pack v4 Nguyễn Thái Ngọc Duy
2013-09-21 13:58 ` [PATCH 14/17] t5302: disable sealth corruption tests when run with --packv4 Nguyễn Thái Ngọc Duy
2013-09-22  2:13   ` Eric Sunshine
2013-09-21 13:58 ` [PATCH 15/17] t5300: avoid testing ofs-delta " Nguyễn Thái Ngọc Duy
2013-09-21 16:46   ` Nicolas Pitre
2013-09-22  1:48     ` Duy Nguyen
2013-09-22  2:13       ` Nicolas Pitre
2013-09-21 13:58 ` [PATCH 16/17] pack-objects: disable pack size limit feature on pack v4 Nguyễn Thái Ngọc Duy
2013-09-21 13:58 ` [PATCH 17/17] t5303: adapt the tests to run with --packv4 Nguyễn Thái Ngọc Duy
2013-09-21 16:07 ` [PATCH 00/17] np/pack-v4 updates Nicolas Pitre
2013-10-15 21:45   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1379771883-10278-13-git-send-email-pclouds@gmail.com \
    --to=pclouds@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=nico@fluxnic.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).