From: Karsten Blees <karsten.blees@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Duy Nguyen <pclouds@gmail.com>,
kusmabite@gmail.com, Ramkumar Ramachandra <artagnon@gmail.com>,
Robert Zeh <robert.allan.zeh@gmail.com>,
Junio C Hamano <gitster@pobox.com>,
Git List <git@vger.kernel.org>,
finnag@pvv.org
Subject: Re: inotify to minimize stat() calls
Date: Wed, 13 Feb 2013 21:25:59 +0100 [thread overview]
Message-ID: <511BF6D7.3030404@gmail.com> (raw)
In-Reply-To: <20130213181851.GA5603@sigill.intra.peff.net>
Am 13.02.2013 19:18, schrieb Jeff King:
> Moreover, looking at it again, I
> don't think my patch produces the right behavior: we have a single
> dir_next pointer, even though the same ce_entry may appear under many
> directory hashes. So the cache_entries that has to "dir/foo/" and those
> that hash to "dir/bar/" may get confused, because they will also both be
> found under "dir/", and both try to create a linked list from the
> dir_next pointer.
>
Indeed. In the worst case, this causes an endless loop if ce->dir_next == ce
---8<---
mkdir V
mkdir V/XQANY
mkdir WURZAUP
touch V/XQANY/test
git init
git config core.ignorecase true
git add .
git status
---8<---
Note: "V/", "V/XQANY/" and "WURZAUP/" all have the same hash_name. Although I found those strange values by brute force, hash collisions in 32 bit values are not that uncommon in real life :-)
> Looking at Karsten's patch, it seems like it will not add a cache entry
> if there is one of the same name. But I'm not sure if that is right, as
> the old one might be CE_UNHASHED (or it might get removed later). You
> actually want to be able to find each cache_entry that has a file under
> the directory at the hash of that directory, so you can make sure it is
> still valid.
>
Yes, the patch was just to show potential performance savings, I didn't consider CE_UNHASHED at all.
> I think the best way forward is to actually create a separate hash table
> for the directory lookups. I note that we only care about these entries
> in directory_exists_in_index_icase, which is really about whether
> something is there, versus what exactly is there. So could we maybe get
> by with a separate hash table that stores a count of entries at each
> directory, and increment/decrement the count when we add/remove entries?
>
Alternatively, we could simply create normal cache_entries for the directories that are linked via ce->next, but have a trailing '/' in their name?
Reference counting sounds good to me, at least better than allocating directory entries per cache entry * parent dirs.
next prev parent reply other threads:[~2013-02-13 20:26 UTC|newest]
Thread overview: 88+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-08 21:10 inotify to minimize stat() calls Ramkumar Ramachandra
2013-02-08 22:15 ` Junio C Hamano
2013-02-08 22:45 ` Junio C Hamano
2013-02-09 2:10 ` Duy Nguyen
2013-02-09 2:37 ` Junio C Hamano
2013-02-09 2:56 ` Junio C Hamano
2013-02-09 3:36 ` Robert Zeh
2013-02-09 12:05 ` Ramkumar Ramachandra
2013-02-09 12:11 ` Ramkumar Ramachandra
2013-02-09 12:53 ` Ramkumar Ramachandra
2013-02-09 12:59 ` Duy Nguyen
2013-02-09 17:10 ` Ramkumar Ramachandra
2013-02-09 18:56 ` Ramkumar Ramachandra
2013-02-10 5:24 ` Duy Nguyen
2013-02-10 11:17 ` Duy Nguyen
2013-02-10 11:22 ` Duy Nguyen
2013-02-10 20:16 ` Junio C Hamano
2013-02-11 2:56 ` Duy Nguyen
2013-02-11 11:12 ` Duy Nguyen
2013-03-07 22:16 ` Torsten Bögershausen
2013-03-08 0:04 ` Junio C Hamano
2013-03-08 7:01 ` Torsten Bögershausen
2013-03-08 8:15 ` Junio C Hamano
2013-03-08 9:24 ` Torsten Bögershausen
2013-03-08 10:53 ` Duy Nguyen
2013-03-10 8:23 ` Ramkumar Ramachandra
2013-03-13 12:59 ` [PATCH] status: hint the user about -uno if read_directory takes too long Nguyễn Thái Ngọc Duy
2013-03-13 15:21 ` Torsten Bögershausen
2013-03-13 16:16 ` Junio C Hamano
2013-03-14 10:22 ` Duy Nguyen
2013-03-14 15:05 ` Junio C Hamano
2013-03-15 12:30 ` Duy Nguyen
2013-03-15 15:52 ` Torsten Bögershausen
2013-03-15 15:57 ` Ramkumar Ramachandra
2013-03-15 16:53 ` Junio C Hamano
2013-03-15 17:41 ` Torsten Bögershausen
2013-03-15 20:06 ` Junio C Hamano
2013-03-15 21:14 ` Torsten Bögershausen
2013-03-15 21:59 ` Junio C Hamano
2013-03-16 7:21 ` Torsten Bögershausen
2013-03-17 4:47 ` Junio C Hamano
2013-03-16 1:51 ` Duy Nguyen
2013-02-10 13:26 ` inotify to minimize stat() calls demerphq
2013-02-10 15:35 ` Duy Nguyen
2013-02-14 14:36 ` Magnus Bäck
2013-02-10 16:45 ` Ramkumar Ramachandra
2013-02-11 3:03 ` Duy Nguyen
2013-02-10 16:58 ` Erik Faye-Lund
2013-02-11 3:53 ` Duy Nguyen
2013-02-12 20:48 ` Karsten Blees
2013-02-13 10:06 ` Duy Nguyen
2013-02-13 12:15 ` Duy Nguyen
2013-02-13 18:18 ` Jeff King
2013-02-13 19:47 ` Jeff King
2013-02-13 20:25 ` Karsten Blees [this message]
2013-02-13 22:55 ` Jeff King
2013-02-14 0:48 ` Karsten Blees
2013-02-27 14:45 ` [PATCH] name-hash.c: fix endless loop with core.ignorecase=true Karsten Blees
2013-02-27 16:53 ` Junio C Hamano
2013-02-27 21:52 ` Karsten Blees
2013-02-27 23:57 ` [PATCH v2] " Karsten Blees
2013-02-28 0:27 ` Junio C Hamano
2013-02-19 9:49 ` inotify to minimize stat() calls Ramkumar Ramachandra
2013-02-19 14:25 ` Karsten Blees
2013-02-19 13:16 ` Drew Northup
2013-02-19 13:47 ` Duy Nguyen
2013-02-09 19:35 ` Junio C Hamano
2013-02-10 19:03 ` Robert Zeh
2013-02-10 19:26 ` Martin Fick
2013-02-10 20:18 ` Robert Zeh
2013-02-11 3:21 ` Duy Nguyen
2013-02-11 14:13 ` Robert Zeh
2013-02-19 9:57 ` Ramkumar Ramachandra
2013-04-24 17:20 ` [PATCH] " Robert Zeh
2013-04-24 21:32 ` Duy Nguyen
2013-04-25 19:44 ` Robert Zeh
2013-04-25 21:20 ` Duy Nguyen
2013-04-26 15:35 ` Robert Zeh
2013-04-25 8:18 ` Thomas Rast
2013-04-25 19:37 ` Robert Zeh
2013-04-25 19:59 ` Thomas Rast
2013-04-27 13:51 ` Thomas Rast
2013-04-27 23:56 ` Duy Nguyen
[not found] ` <CAKXa9=r2A7UeBV2s2H3wVGdPkS1zZ9huNJhtvTC-p0S5Ed12xA@mail.gmail.com>
2013-04-30 0:27 ` Duy Nguyen
2013-02-09 11:32 ` Ramkumar Ramachandra
2013-02-14 15:16 ` Ævar Arnfjörð Bjarmason
2013-02-14 16:31 ` Junio C Hamano
2013-02-19 9:40 ` Ramkumar Ramachandra
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=511BF6D7.3030404@gmail.com \
--to=karsten.blees@gmail.com \
--cc=artagnon@gmail.com \
--cc=blees@dcon.de \
--cc=finnag@pvv.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=kusmabite@gmail.com \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
--cc=robert.allan.zeh@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 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).