Re: [PATCH] list-objects: mark fewer commits as edges for non-shallow clones

[Junio C Hamano <gitster@pobox.com>]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> In commit fbd4a70 (list-objects: mark more commits as edges in
> mark_edges_uninteresting - 2013-08-16), we made --thin much more
> aggressive by reading lots more trees.  This produces much smaller packs
> for shallow clones; however, it causes a significant performance
> regression for non-shallow clones with lots of refs (23.322 seconds vs.
> 4.785 seconds with 22400 refs).  Limit this extra edge-marking to
> shallow clones to avoid poor performance.

This change affects non-clone/fetch uses of object listing depending
on the shallowness of the repository, and does not even care if it
is driven as part of the pack-object codepath, if I am reading it
correctly.  It smells wrong.

The problematic fbd4a70 already had unintended fallout that needed
to be corrected with 200abe74 (list-objects: only look at cmdline
trees with edge_hint, 2014-01-20).  The current code with the fix,
the decision to use the more expensive marking is tied to
"edge_hint". I notice that edge_hint is turned on only if the caller
of rev-list passes the "--objects-edge" option, and currently that
only happens in the pack-objects codepath when "thin" is given.
Perhaps that part should decide if it really wants to do edge_hint
depending on the shallowness of the repository perhaps?

That is, something like this instead?

 builtin/pack-objects.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3f9f5c7..a9ebf56 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2709,7 +2709,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 		usage_with_options(pack_usage, pack_objects_options);
 
 	argv_array_push(&rp, "pack-objects");
-	if (thin) {
+	if (thin && is_repository_shallow()) {
 		use_internal_rev_list = 1;
 		argv_array_push(&rp, "--objects-edge");
 	} else