Re: [PATCH 1/6] add open_nofollow() helper

From: Jeff King <peff@peff.net>
Date: Feb 16, 2021, 16:19 UTC
Message-ID: <YCvwjBm3Jpmi+8Dv@coredump.intra.peff.net>
In-Reply-To: <YCvuy80k/P8qQ1h7@nand.local>

On Tue, Feb 16, 2021 at 11:11:55AM -0500, Taylor Blau wrote:

Show 31 quoted lines

> On Tue, Feb 16, 2021 at 11:07:39AM -0500, Taylor Blau wrote:
> > On Tue, Feb 16, 2021 at 11:02:23AM -0500, Jeff King wrote:
> > > Can you double-check your initial timings?
> >
> > Aha, I forgot to update the input to the second check-attr tests after
> > putting .gitattributes files everywhere.
> >
> > Rerunning with O_NOFOLLOW, the initial timings look something like
> > 128.8ms and 183.7ms before/after.
> 
> I should add that "before" refers to a clean checkout, and "after"
> refers to the state after running 'find * -type d | ... | xargs touch'.
> Both of those numbers are with the O_NOFOLLOW branch.
> 
> If I repeat the test after applying:
> 
> diff --git a/wrapper.c b/wrapper.c
> index 563ad590df..90f603e749 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -681,7 +681,7 @@ int is_empty_or_missing_file(const char *filename)
> 
>  int open_nofollow(const char *path, int flags)
>  {
> -#ifdef O_NOFOLLOW
> +#if 0
>         return open(path, flags | O_NOFOLLOW);
>  #else
>         struct stat st;
> 
> Then those numbers go from 155.9ms to 197.2ms.

OK, thanks. So to lay out all cases:

             one file | many files
            +--------------------
O_NOFOLLOW  | 128.8ms | 183.7ms
lstat+open  | 155.9ms | 197.2ms

The jump for both from "one" to "many" files for both rows is expected; there's more work to do. The jump from 183 to 197 in the "many" column is what I was wanting to measure (how expensive is the fallback code), and is around the order of magnitude I'd expect (and IMHO probably acceptable).

But the jump in the "one file" column between the two code paths is really confusing. Those are making the same number of syscalls. It's possible that lstat() is way more expensive than open(), but in that case I'd expect to see a similarly large jump in the "many" column. Weird.

-Peff