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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 31, 2016, 17:55 UTC
Message-ID: <xmqqwpgotncb.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161031135601.7immbp44wn7uksvs@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 21 quoted lines

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

Let's keep doubting. I prefer one-step-at-a-time approach to things anyway, and what I plan in the near term are:

 * use the "open() with O_NOATIME|O_CLOEXEC, gradually losing the
   bits during fallback" approach in the ls/git-open-cloexec topic,
   in order to help ls/filter-process topic be part of the upcoming
   release;

 * simplify the logic to the "open(2) with O_CLOEXEC, set O_NOATIME
   with fcntl(2)" in jc/git-open-cloexec~1 after 2.11 ships;

 * cook "drop the latter half of setting O_NOATIME" which is at the
   tip of jc/git-open-cloexec in 'next', and while Linus is looking
   the other way ^W^W^W^W^W^W^W after people had chance to complain
   with numbers, merge it to a future release iff it still looked OK
   to drop O_NOATIME thing.