From: Junio C Hamano <gitster@pobox.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: git@vger.kernel.org
Subject: [RFH] index_name_exists() conversion
Date: Thu, 21 Feb 2008 01:07:20 -0800 [thread overview]
Message-ID: <7vy79evfsn.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <7v7igywvnj.fsf@gitster.siamese.dyndns.org> (Junio C. Hamano's message of "Thu, 21 Feb 2008 00:39:28 -0800")
While (hopefully) I have your attention, I have another question
about the recent optimization.
commit cf558704fb68514a820e3666968967c900e0fd29
Author: Linus Torvalds <torvalds@linux-foundation.org>
Create pathname-based hash-table lookup into index
diff --git a/dir.c b/dir.c
index 1b9cc7a..6543105 100644
--- a/dir.c
+++ b/dir.c
@@ -346,7 +346,7 @@ static struct dir_entry *dir_entry_new(cons...
struct dir_entry *dir_add_name(struct dir_struct *dir, const c...
{
- if (cache_name_pos(pathname, len) >= 0)
+ if (cache_name_exists(pathname, len))
return NULL;
ALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);
This used to check if there is a stage#0 entry of that name in
the index. But the name-hash based index look-up does not care
about the stage as far as I can tell.
I haven't managed to come up with a failure case to demonstrate,
but I think this can affect ls-files and clean, potentially in a
bad way. "git add dir/" to resolve unmerged paths under dir/
somewhere should not be affected, as the codepath calls
read_directory() before reading the cache (so the check will
always say "Nope, we do not have such a path in the index yet").
I am not sure what the right solution would be. The motivation
for the patch was to make "does this name exist in the index"
and also "does this exist under an equivalent name" efficient,
but it is asking a question that is not precise enough. Are we
interested in such an entry at any stage, or only at stage#0, or
what?
This could be related to the recent "status segfaults, bisected
to cf558704" issue posted to the list. I dunno.
next prev parent reply other threads:[~2008-02-21 9:08 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-02-21 8:39 [RFH] CE_REMOVE conversion Junio C Hamano
2008-02-21 9:07 ` Junio C Hamano [this message]
2008-02-21 16:15 ` [RFH] index_name_exists() conversion Linus Torvalds
2008-02-21 16:29 ` Johannes Schindelin
2008-02-21 17:00 ` Linus Torvalds
2008-02-21 16:05 ` [RFH] CE_REMOVE conversion Linus Torvalds
2008-02-21 16:43 ` Junio C Hamano
2008-02-21 17:31 ` Linus Torvalds
2008-02-21 20:06 ` Linus Torvalds
2008-02-21 20:08 ` 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=7vy79evfsn.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/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).