Re: [PATCH v5 8/8] sha1_file: refactor has_sha1_file_with_flags
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jul 19, 2017, 12:52 UTC
- Message-ID
- <alpine.DEB.2.21.1.1707191450570.4193@virtualbox>
- In-Reply-To
- <20170718093920.3265c67f@twelve2.svl.corp.google.com>
Hi Jonathan,
On Tue, 18 Jul 2017, Jonathan Tan wrote:
Show 41 quoted lines
> On Tue, 18 Jul 2017 12:30:46 +0200
> Christian Couder <christian.couder@gmail.com> wrote:
>
> > On Thu, Jun 22, 2017 at 2:40 AM, Jonathan Tan <jonathantanmy@google.com> wrote:
> >
> > > diff --git a/sha1_file.c b/sha1_file.c
> > > index bf6b64ec8..778f01d92 100644
> > > --- a/sha1_file.c
> > > +++ b/sha1_file.c
> > > @@ -3494,18 +3494,10 @@ int has_sha1_pack(const unsigned char *sha1)
> > >
> > > int has_sha1_file_with_flags(const unsigned char *sha1, int flags)
> > > {
> > > - struct pack_entry e;
> > > -
> > > if (!startup_info->have_repository)
> > > return 0;
> > > - if (find_pack_entry(sha1, &e))
> > > - return 1;
> > > - if (has_loose_object(sha1))
> > > - return 1;
> > > - if (flags & HAS_SHA1_QUICK)
> > > - return 0;
> > > - reprepare_packed_git();
> > > - return find_pack_entry(sha1, &e);
> > > + return sha1_object_info_extended(sha1, NULL,
> > > + flags | OBJECT_INFO_SKIP_CACHED) >= 0;
> > > }
> >
> > I am not sure if it could affect performance (in one way or another) a
> > lot or not but I just wanted to note that has_loose_object() calls
> > check_and_freshen() which calls access() on loose object files, while
> > sha1_object_info_extended() calls sha1_loose_object_info() which calls
> > stat_sha1_file() which calls lstat() on loose object files.
> >
> > So depending on the relative performance of access() and lstat() there
> > could be a performance impact on repos that have a lot of loose object
> > files.
>
> That is true, but from what little I have read online, they have about
> the same performance.Then your online sources missed out on what we have in compat/mingw.[ch]. I would expect _waccess() (which is used to emulate access()) to be substantially faster than the hoops we jump through to emulate lstat().
Ciao, Dscho