From: Taylor Blau <me@ttaylorr.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: "SZEDER Gábor" <szeder.dev@gmail.com>, git@vger.kernel.org
Subject: Re: [RFC PATCH] Not computing changed path filter for root commits
Date: Tue, 10 Oct 2023 15:47:22 -0400 [thread overview]
Message-ID: <ZSWqStEpsFU0SmEm@nand.local> (raw)
In-Reply-To: <20231009205925.1915096-1-jonathantanmy@google.com>
On Mon, Oct 09, 2023 at 01:59:25PM -0700, Jonathan Tan wrote:
> Taylor Blau <me@ttaylorr.com> writes:
> > This only happens when we return REV_TREE_NEW from a call to
> > `rev_compare_tree(revs, p, commit, nth_parent)`. But we'll only get
> > REV_TREE_NEW back if
> >
> > repo_get_commit_tree(the_repository, p);
> >
> > returns NULL. But when we call rev_same_tree_as_empty(revs, p) in the
> > REV_TREE_NEW case, we return early as follows:
> >
> > struct tree *t1 = repo_get_commit_tree(revs, p);
> > if (!t1)
> > return 0;
> >
> > So we won't even consult the Bloom filter in that case, since t1 is NULL
> > for the same reason as what caused rev_compare_tree() to return
> > REV_TREE_NEW in the first place.
> >
> > I am still dumbfounded by how we would ever get REV_TREE_NEW in the
> > first place, but if we did, I think we would be OK here.
> >
> > Thanks,
> > Taylor
>
> Ah, good point. Your patch in
> https://lore.kernel.org/git/ZQnmTXUO94%2FQy8mq@nand.local/ looks good to
> me, then.
Oops, I made a mistake in the quoted portion, which is that we could get
REV_TREE_NEW if the tree-diff itself only adds files. This is the
non-trivial case that we get when t1 is non-NULL, and we end up calling
`diff_tree_oid()` which sets the static `tree_difference` variable.
So I think adding an nth_parent field (like you originally
suggested[^1]) makes sense.
Thanks,
Taylor
[^1]: Thanks for being patient with me ;-).
next prev parent reply other threads:[~2023-10-10 19:47 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-11 22:31 [RFC PATCH] Not computing changed path filter for root commits Jonathan Tan
2023-09-15 18:25 ` Junio C Hamano
2023-09-15 20:29 ` SZEDER Gábor
2023-09-19 18:19 ` Taylor Blau
2023-10-02 22:55 ` Jonathan Tan
2023-10-09 17:20 ` Taylor Blau
2023-10-09 17:26 ` Taylor Blau
2023-10-09 20:59 ` Jonathan Tan
2023-10-10 19:47 ` Taylor Blau [this message]
2023-09-19 18:21 ` Taylor Blau
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=ZSWqStEpsFU0SmEm@nand.local \
--to=me@ttaylorr.com \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.com \
--cc=szeder.dev@gmail.com \
/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.