On Fri, Oct 13, 2017 at 09:56:36AM -0400, Jeff King wrote: > On Fri, Oct 13, 2017 at 09:55:15AM -0400, Derrick Stolee wrote: > > > > We should be comparing an empty tree and d0/d0/d0/d0 (or however deep > > > your pathspec goes). We should be able to see immediately that the entry > > > is not present between the two and not bother descending. After all, > > > we've set the QUICK flag in init_revisions(). So the real question is > > > why QUICK is not kicking in. > > > > I'm struggling to understand your meaning. We want to walk from root to > > d0/d0/d0/d0, but there is no reason to walk beyond that tree. But maybe > > that's what the QUICK flag is supposed to do. > > Yes, that's exactly what it is for. When we see the first difference we > should say "aha, the caller only wanted to know whether there was a > difference, not what it was" and return immediately. See > diff_can_quit_early(). Hmm. So this patch makes it go fast: diff --git a/revision.c b/revision.c index d167223e69..b52ea4e9d8 100644 --- a/revision.c +++ b/revision.c @@ -409,7 +409,7 @@ static void file_add_remove(struct diff_options *options, int diff = addremove == '+' ? REV_TREE_NEW : REV_TREE_OLD; tree_difference |= diff; - if (tree_difference == REV_TREE_DIFFERENT) + if (tree_difference & REV_TREE_DIFFERENT) DIFF_OPT_SET(options, HAS_CHANGES); } But that essentially makes the conditional a noop (since we know we set either NEW or OLD above and DIFFERENT is the union of those flags). I'm not sure I understand why file_add_remove() would ever want to avoid setting HAS_CHANGES (certainly its companion file_change() always does). This goes back to Junio's dd47aa3133 (try-to-simplify-commit: use diff-tree --quiet machinery., 2007-03-14). Maybe I am missing something, but AFAICT this was always buggy. But since it only affects adds and deletes, maybe nobody noticed? I'm also not sure if it only causes a slowdown, or if this could cause us to erroneously mark something as TREESAME which isn't (I _do_ think people would have noticed that). -Peff
