git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org, Derrick Stolee <derrickstolee@github.com>,
	Jeff King <peff@peff.net>
Subject: Re: [PATCH 6/7] config: enable `pack.writeReverseIndex` by default
Date: Thu, 13 Apr 2023 09:14:04 -0700	[thread overview]
Message-ID: <xmqqttxjyd2r.fsf@gitster.g> (raw)
In-Reply-To: <56a0fc0098e0b0551e01414c8e67c17fb1ba3054.1681166596.git.me@ttaylorr.com> (Taylor Blau's message of "Mon, 10 Apr 2023 18:53:40 -0400")

Taylor Blau <me@ttaylorr.com> writes:

> They are useful in GitHub's infrastructure, where we have seen a
> dramatic increase in performance when writing ".rev" files[1]. In
> particular:
>
>   - an ~80% reduction in the time it takes to serve fetches on a popular
>     repository, Homebrew/homebrew-core.
>
>   - a ~60% reduction in the peak memory usage to serve fetches on that
>     same repository.
>
>   - a collective savings of ~35% in CPU time across all pack-objects
>     invocations serving fetches across all repositories in a single
>     datacenter.
>
> Reverse indexes are also beneficial to end-users as well as forges. For
> example, the time it takes to generate a pack containing the objects for
> the 10 most recent commits in linux.git (representing a typical push) is
> significantly faster when on-disk reverse indexes are available:
> ...
> In the more than two years since e37d0b8730 was merged, Git's
> implementation of on-disk reverse indexes has been thoroughly tested,
> both from users enabling `pack.writeReverseIndexes`, and from GitHub's
> deployment of the feature. The latter has been running without incident
> for more than two years.

What is missing is what extra overhead this adds to "git gc".  Does
it add 5%?  15%?  How big an overhead do we find reasonable?

Other than that, it looks like that the series is very well crafted.
It was somewhat surprising that we still need to add a few new
end-user facing or meant-for-testing knobs for a feature that we
claim is well battle tested.  It is understandable that we benefit
by having knobs to selectively _disable_ a part of the feature now
it will be on by default.

Will queue.  Thanks.


  reply	other threads:[~2023-04-13 16:14 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-10 22:53 [PATCH 0/7] pack-revindex: enable on-disk reverse indexes by default Taylor Blau
2023-04-10 22:53 ` [PATCH 1/7] pack-write.c: plug a leak in stage_tmp_packfiles() Taylor Blau
2023-04-11 13:23   ` Derrick Stolee
2023-04-11 21:25     ` Taylor Blau
2023-04-10 22:53 ` [PATCH 2/7] t5325: mark as leak-free Taylor Blau
2023-04-10 22:53 ` [PATCH 3/7] pack-revindex: make `load_pack_revindex` take a repository Taylor Blau
2023-04-11 13:45   ` Derrick Stolee
2023-04-11 21:30     ` Taylor Blau
2023-04-12 17:33       ` Derrick Stolee
2023-04-10 22:53 ` [PATCH 4/7] pack-revindex: introduce GIT_TEST_REV_INDEX_DIE_ON_DISK Taylor Blau
2023-04-10 22:53 ` [PATCH 5/7] pack-revindex: introduce `pack.readReverseIndex` Taylor Blau
2023-04-10 22:53 ` [PATCH 6/7] config: enable `pack.writeReverseIndex` by default Taylor Blau
2023-04-13 16:14   ` Junio C Hamano [this message]
2023-04-10 22:53 ` [PATCH 7/7] t: invert `GIT_TEST_WRITE_REV_INDEX` Taylor Blau
2023-04-11 13:51   ` Derrick Stolee
2023-04-11 21:33     ` Taylor Blau
2023-04-12 17:37       ` Derrick Stolee
2023-04-11 13:54 ` [PATCH 0/7] pack-revindex: enable on-disk reverse indexes by default Derrick Stolee
2023-04-11 21:40   ` Taylor Blau
2023-04-12 17:39     ` Derrick Stolee
2023-04-12 22:20 ` [PATCH v2 " Taylor Blau
2023-04-12 22:20   ` [PATCH v2 1/7] pack-write.c: plug a leak in stage_tmp_packfiles() Taylor Blau
2023-04-12 22:20   ` [PATCH v2 2/7] t5325: mark as leak-free Taylor Blau
2023-04-12 22:20   ` [PATCH v2 3/7] pack-revindex: make `load_pack_revindex` take a repository Taylor Blau
2023-04-12 22:20   ` [PATCH v2 4/7] pack-revindex: introduce GIT_TEST_REV_INDEX_DIE_ON_DISK Taylor Blau
2023-04-12 22:20   ` [PATCH v2 5/7] pack-revindex: introduce `pack.readReverseIndex` Taylor Blau
2023-04-12 22:20   ` [PATCH v2 6/7] config: enable `pack.writeReverseIndex` by default Taylor Blau
2023-04-12 22:20   ` [PATCH v2 7/7] t: invert `GIT_TEST_WRITE_REV_INDEX` Taylor Blau
2023-04-13 13:40   ` [PATCH v2 0/7] pack-revindex: enable on-disk reverse indexes by default Derrick Stolee

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

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

  git send-email \
    --in-reply-to=xmqqttxjyd2r.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).