git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: Derrick Stolee <stolee@gmail.com>,
	Git Mailing List <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>, Chris Torek <chris.torek@gmail.com>
Subject: Re: [PATCH] midx: use buffered I/O to talk to pack-objects
Date: Tue, 11 Aug 2020 18:08:29 +0200
Message-ID: <3655b3c8-9b60-daec-c3a2-6e3703ec5b3f@web.de> (raw)
In-Reply-To: <90541678-f412-89a1-2ee0-4cae30e26551@gmail.com>

Am 03.08.20 um 14:39 schrieb Derrick Stolee:
> On 8/2/2020 10:38 AM, René Scharfe wrote:
>> Like f0bca72dc77 (send-pack: use buffered I/O to talk to pack-objects,
>> 2016-06-08), significantly reduce the number of system calls and
>> simplify the code for sending object IDs to pack-objects by using
>> stdio's buffering and handling errors after the loop.
>
> Good find. Thanks for doing this important cleanup.
>
> Outside of Chris's other feedback, this looks like an obviously
> correct transformation.

I spent a surprising amount of time trying to find a solution that is
easy to use and allows precise error handling.  But now I get second
thoughts.  The main selling point of buffering is better performance,
which is achieved by reducing the number of system calls.  How much
better actually?

So I get this in my Git repo clone without this patch:

  $ strace --summary-only --trace=write git multi-pack-index repack --no-progress
  % time     seconds  usecs/call     calls    errors syscall
  ------ ----------- ----------- --------- --------- ----------------
  100.00    2.237478           2    921650           write
  ------ ----------- ----------- --------- --------- ----------------
  100.00    2.237478                921650           total

And here's the same with the patch:

  % time     seconds  usecs/call     calls    errors syscall
  ------ ----------- ----------- --------- --------- ----------------
  100.00    0.013293           2      4613           write
  ------ ----------- ----------- --------- --------- ----------------
  100.00    0.013293                  4613           total

Awesome, right?  write(2) calls are down by a factor of almost 200 and
the time spent on them is reduced significantly, as advertised.  Let's
ask hyperfine for a second opinion though.  Without this patch:

  Benchmark #1: git multi-pack-index repack --no-progress
    Time (mean ± σ):      1.652 s ±  0.206 s    [User: 1.383 s, System: 0.317 s]
    Range (min … max):    1.426 s …  1.890 s    10 runs

And the same with this patch applied:

    Time (mean ± σ):      1.635 s ±  0.199 s    [User: 1.363 s, System: 0.204 s]
    Range (min … max):    1.430 s …  1.871 s    10 runs

OK, so system time is down by ca. 50%, but the total duration is
basically unchanged.  It seems strace added quite some overhead to our
measurement above.

Anyway, now I wonder if adding our own buffer on top if the
OS-internal pipe buffer is actually worth it.  The numbers above are
from Debian testing , by the way.  Perhaps buffering still pays off on
operating systems with slower pipes..

René

  reply	other threads:[~2020-08-11 16:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-02 14:38 René Scharfe
2020-08-02 16:11 ` Chris Torek
2020-08-03 18:10   ` Johannes Sixt
2020-08-03 22:27     ` René Scharfe
2020-08-04  4:31       ` René Scharfe
2020-08-04  4:37         ` Junio C Hamano
2020-08-03 12:39 ` Derrick Stolee
2020-08-11 16:08   ` René Scharfe [this message]
2020-08-11 17:14     ` Derrick Stolee
2020-08-12 16:52 ` [PATCH v2] " René Scharfe
2020-08-12 20:28   ` Junio C Hamano
2020-08-12 20:31     ` Junio C Hamano
2020-08-13  9:11       ` Jeff King
2020-08-13  9:06     ` Jeff King

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3655b3c8-9b60-daec-c3a2-6e3703ec5b3f@web.de \
    --to=l.s.r@web.de \
    --cc=chris.torek@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=stolee@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

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git