All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>,
	gitster@pobox.com, peff@peff.net, peartben@gmail.com,
	benpeart@microsoft.com
Subject: [PATCH 1/3] revision: unify {tree,blob}_objects in rev_info
Date: Fri, 24 Feb 2017 17:18:36 -0800	[thread overview]
Message-ID: <06a84f8c77924b275606384ead8bb2fd7d75f7b6.1487984670.git.jonathantanmy@google.com> (raw)
In-Reply-To: <cover.1487984670.git.jonathantanmy@google.com>
In-Reply-To: <cover.1487984670.git.jonathantanmy@google.com>

Whenever tree_objects is set to 1 in revision.h's struct rev_info,
blob_objects is likewise set, and vice versa. Combine those two fields
into one.

Some of the existing code does not handle tree_objects being different
from blob_objects properly. For example, "handle_commit" in revision.c
recurses from an UNINTERESTING tree into its subtree if tree_objects ==
1, completely ignoring blob_objects; it probably should still recurse if
tree_objects == 0 and blob_objects == 1 (to mark the blobs), and should
behave differently depending on blob_objects (controlling the
instantiation and marking of blob objects). This commit resolves the
issue by forbidding tree_objects from being different to blob_objects.

It could be argued that in the future, Git might need to distinguish
tree_objects from blob_objects - in particular, a user might want
rev-list to print the trees but not the blobs. However, this results in
a minor performance savings at best in that objects no longer need to be
instantiated (causing memory allocations and hashtable insertions) - no
disk reads are being done for objects whether blob_objects is set or
not.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 bisect.c            |  2 +-
 builtin/rev-list.c  |  6 +++---
 list-objects.c      |  4 ++--
 pack-bitmap-write.c |  3 +--
 pack-bitmap.c       |  3 +--
 reachable.c         |  3 +--
 revision.c          | 16 ++++++----------
 revision.h          |  3 +--
 8 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/bisect.c b/bisect.c
index 8e63c40d2..265b32905 100644
--- a/bisect.c
+++ b/bisect.c
@@ -634,7 +634,7 @@ static void bisect_common(struct rev_info *revs)
 {
 	if (prepare_revision_walk(revs))
 		die("revision walk setup failed");
-	if (revs->tree_objects)
+	if (revs->tree_and_blob_objects)
 		mark_edges_uninteresting(revs, NULL);
 }
 
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 0aa93d589..6c2651b31 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -345,7 +345,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 		revs.commit_format = CMIT_FMT_RAW;
 
 	if ((!revs.commits &&
-	     (!(revs.tag_objects || revs.tree_objects || revs.blob_objects) &&
+	     (!(revs.tag_objects || revs.tree_and_blob_objects) &&
 	      !revs.pending.nr)) ||
 	    revs.diff)
 		usage(rev_list_usage);
@@ -374,7 +374,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 				return 0;
 			}
 		} else if (revs.max_count < 0 &&
-			   revs.tag_objects && revs.tree_objects && revs.blob_objects) {
+			   revs.tag_objects && revs.tree_and_blob_objects) {
 			if (!prepare_bitmap_walk(&revs)) {
 				traverse_bitmap_commit_list(&show_object_fast);
 				return 0;
@@ -384,7 +384,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 
 	if (prepare_revision_walk(&revs))
 		die("revision walk setup failed");
-	if (revs.tree_objects)
+	if (revs.tree_and_blob_objects)
 		mark_edges_uninteresting(&revs, show_edge);
 
 	if (bisect_list) {
diff --git a/list-objects.c b/list-objects.c
index f3ca6aafb..796957105 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -18,7 +18,7 @@ static void process_blob(struct rev_info *revs,
 	struct object *obj = &blob->object;
 	size_t pathlen;
 
-	if (!revs->blob_objects)
+	if (!revs->tree_and_blob_objects)
 		return;
 	if (!obj)
 		die("bad blob object");
@@ -78,7 +78,7 @@ static void process_tree(struct rev_info *revs,
 		all_entries_interesting: entry_not_interesting;
 	int baselen = base->len;
 
-	if (!revs->tree_objects)
+	if (!revs->tree_and_blob_objects)
 		return;
 	if (!obj)
 		die("bad tree object");
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 970559601..5080e276b 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -258,8 +258,7 @@ void bitmap_writer_build(struct packing_data *to_pack)
 
 	init_revisions(&revs, NULL);
 	revs.tag_objects = 1;
-	revs.tree_objects = 1;
-	revs.blob_objects = 1;
+	revs.tree_and_blob_objects = 1;
 	revs.no_walk = 0;
 
 	revs.include_check = should_include;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 39bcc1684..445a24e0d 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -951,8 +951,7 @@ void test_bitmap_walk(struct rev_info *revs)
 		die("Commit %s doesn't have an indexed bitmap", oid_to_hex(&root->oid));
 
 	revs->tag_objects = 1;
-	revs->tree_objects = 1;
-	revs->blob_objects = 1;
+	revs->tree_and_blob_objects = 1;
 
 	result_popcnt = bitmap_popcount(result);
 
diff --git a/reachable.c b/reachable.c
index d0199cace..9a3beb75f 100644
--- a/reachable.c
+++ b/reachable.c
@@ -166,8 +166,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
 	 * in all object types, not just commits.
 	 */
 	revs->tag_objects = 1;
-	revs->blob_objects = 1;
-	revs->tree_objects = 1;
+	revs->tree_and_blob_objects = 1;
 
 	/* Add all refs from the index file */
 	add_index_objects_to_pending(revs, 0);
diff --git a/revision.c b/revision.c
index b37dbec37..5e49d9e0e 100644
--- a/revision.c
+++ b/revision.c
@@ -267,7 +267,7 @@ static struct commit *handle_commit(struct rev_info *revs,
 	 */
 	if (object->type == OBJ_TREE) {
 		struct tree *tree = (struct tree *)object;
-		if (!revs->tree_objects)
+		if (!revs->tree_and_blob_objects)
 			return NULL;
 		if (flags & UNINTERESTING) {
 			mark_tree_contents_uninteresting(tree);
@@ -281,7 +281,7 @@ static struct commit *handle_commit(struct rev_info *revs,
 	 * Blob object? You know the drill by now..
 	 */
 	if (object->type == OBJ_BLOB) {
-		if (!revs->blob_objects)
+		if (!revs->tree_and_blob_objects)
 			return NULL;
 		if (flags & UNINTERESTING)
 			return NULL;
@@ -1817,23 +1817,19 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->limited = 1;
 	} else if (!strcmp(arg, "--objects")) {
 		revs->tag_objects = 1;
-		revs->tree_objects = 1;
-		revs->blob_objects = 1;
+		revs->tree_and_blob_objects = 1;
 	} else if (!strcmp(arg, "--objects-edge")) {
 		revs->tag_objects = 1;
-		revs->tree_objects = 1;
-		revs->blob_objects = 1;
+		revs->tree_and_blob_objects = 1;
 		revs->edge_hint = 1;
 	} else if (!strcmp(arg, "--objects-edge-aggressive")) {
 		revs->tag_objects = 1;
-		revs->tree_objects = 1;
-		revs->blob_objects = 1;
+		revs->tree_and_blob_objects = 1;
 		revs->edge_hint = 1;
 		revs->edge_hint_aggressive = 1;
 	} else if (!strcmp(arg, "--verify-objects")) {
 		revs->tag_objects = 1;
-		revs->tree_objects = 1;
-		revs->blob_objects = 1;
+		revs->tree_and_blob_objects = 1;
 		revs->verify_objects = 1;
 	} else if (!strcmp(arg, "--unpacked")) {
 		revs->unpacked = 1;
diff --git a/revision.h b/revision.h
index 9fac1a607..43cce137f 100644
--- a/revision.h
+++ b/revision.h
@@ -89,8 +89,7 @@ struct rev_info {
 			simplify_merges:1,
 			simplify_by_decoration:1,
 			tag_objects:1,
-			tree_objects:1,
-			blob_objects:1,
+			tree_and_blob_objects:1,
 			verify_objects:1,
 			edge_hint:1,
 			edge_hint_aggressive:1,
-- 
2.11.0.483.g087da7b7c-goog


  reply	other threads:[~2017-02-25  1:19 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-25  1:18 [PATCH 0/3] Test fetch-pack's ability to fetch arbitrary blobs Jonathan Tan
2017-02-25  1:18 ` Jonathan Tan [this message]
2017-02-28 21:42   ` [PATCH 1/3] revision: unify {tree,blob}_objects in rev_info Junio C Hamano
2017-02-28 21:59     ` Jeff King
2017-03-02 18:36       ` Junio C Hamano
2017-02-28 22:06   ` Jeff King
2017-02-25  1:18 ` [PATCH 2/3] revision: exclude trees/blobs given commit Jonathan Tan
2017-02-28 21:44   ` Junio C Hamano
2017-02-28 22:12   ` Jeff King
2017-03-02 19:50     ` [PATCH] t/perf: export variable used in other blocks Jonathan Tan
2017-03-03  6:45       ` Jeff King
2017-03-03  7:14         ` [PATCH] t/perf: use $MODERN_GIT for all repo-copying steps Jeff King
2017-03-03  7:36           ` [PATCH] t/perf: add fallback for pre-bin-wrappers versions of git Jeff King
2017-03-03 18:51         ` [PATCH] t/perf: export variable used in other blocks Junio C Hamano
2017-03-03 22:31           ` Jeff King
2017-02-28 23:12   ` [PATCH 2/3] revision: exclude trees/blobs given commit Junio C Hamano
2017-02-25  1:18 ` [PATCH 3/3] upload-pack: compute blob reachability correctly Jonathan Tan

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=06a84f8c77924b275606384ead8bb2fd7d75f7b6.1487984670.git.jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=benpeart@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peartben@gmail.com \
    --cc=peff@peff.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.