All of lore.kernel.org
 help / color / mirror / Atom feed
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.

  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 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.