Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC

[Jeff King <peff@peff.net>]

On Fri, Oct 28, 2016 at 09:13:41AM -0700, Linus Torvalds wrote:

> On Fri, Oct 28, 2016 at 4:11 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > You guys. I mean: You guys! You sure make my life hard. A brief look at
> > mingw.h could have answered your implicit question:
> 
> So here's what you guys should do:
> 
>  - leave O_NOATIME damn well alone. It works. It has worked for 10+
> years. Stop arguing against it, people who do.

For some definition of worked, perhaps.

If you set a probe on touch_atime() in the kernel (which is called for
every attempt to smudge the atime, regardless of mount options, but is
skipped when the descriptor was opened with O_NOATIME), you can see the
impact. Here's a command I picked because it reads a lot of objects (run
on my git.git clone):

  $ perf stat -e probe:touch-atime git log -Sfoo >/dev/null

And the probe:touch_atime counts before (stock git) and after (a patch
to drop O_NOATIME):

  before: 22,235
   after: 22,362

So that's only half a percent difference. And it's on a reasonably messy
clone that is partway to triggering an auto-repack:

  $ git count-objects -v
  count: 6167
  size: 61128
  in-pack: 275773
  packs: 18
  size-pack: 86857
  prune-packable: 25
  garbage: 0
  size-garbage: 0

Running "git gc" drops the probe count to 21,733.

It makes a bigger difference for some commands (it's more like 10% for
git-status). And smaller for others ("git log -p" triggers it over
100,000 times).

One thing missing in that count is how many of those calls would have
resulted in an actual disk write. Looking at strace, most of the
filesystem activity is opening .gitattributes files, and we end up
opening the same ones repeatedly (e.g., t/.gitattributes in git.git).
Multiple hits for a given inode in the same second get coalesced into at
most a single disk write.

So I guess it's possible that it produces a noticeable effect in some
cases, but I'm still somewhat doubtful. And actually repacking your
repository had a greater effect in every case I measured (in addition to
providing other speedups).

Like I said, I'm OK keeping O_NOATIME. It's just not that much code. But
if you really care about the issue of dirtying inodes via atime, you
should look into vastly increasing our use of O_NOATIME. Or possibly
looking at caching more in the attribute code.

-Peff