From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: gerardu@amazon.com, Jeff King <peff@peff.net>,
Junio C Hamano <gitster@pobox.com>,
Git Mailing List <git@vger.kernel.org>
Subject: Re: [RFC PATCH] index-pack: improve performance on NFS
Date: Sat, 10 Nov 2018 15:04:35 +0100 [thread overview]
Message-ID: <87a7mh58r0.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <CACsJy8BvD6eSaxu1KyXawMcBCQGMQ5DywQpsycTTjKusioZZuQ@mail.gmail.com>
On Fri, Nov 09 2018, Duy Nguyen wrote:
> On Fri, Nov 9, 2018 at 2:46 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> I'm planning to re-submit mine with some minor changes after the great
>> Documentation/config* move lands.
>>
>> As noted in
>> https://public-inbox.org/git/87bm7clf4o.fsf@evledraar.gmail.com/ and
>> https://public-inbox.org/git/87h8gq5zmc.fsf@evledraar.gmail.com/ I think
>> it's regardless of Jeff's optimization is. O(nothing) is always faster
>> than O(something), particularly (as explained in that E-Mail) on NFS.
>
> Is it really worth adding more code to maintain just to shave a couple
> seconds (or a few percent clone time)?
Yeah I think so, because (in rough priority order):
a) The maintenance burden of carrying core.checkCollisions is trivial,
and it's hard to imagine a scenario where it'll be difficult to
selectively turn off some does_this_collide() function.
b) I think I need to worry more about a meteorite colliding with the
datacenter than the threat this check is trying to guard against.
c) I think we should just turn it off by default on SHA-1, but don't
expect that argument will carry the day. But I expect even those who
think we still need it will have a hard time making that argument in
the case of SHA-256. So having the codepath to disable it is helpful.
d) As shown in the linked E-Mails of mine you sometimes pay a 2-3 second
*fixed* cost even for a very small (think ~100-200 objects) push/fetch
that would otherwise take milliseconds with Jeff's version of this
optimization (and not with mine). This can be a hundred/thousands of
percent slowdown.
Is that a big deal in itself in terms of absolute time spent? No. But
I'm also thinking about this from the perspective of getting noise
out of performance metrics. Some of this slowdown is also "user
waiting for the terminal to be usable again" not just some machine
somewhere wasting its own time.
e) As shown in the patch I have this direction as a very beneficial
side-effect makes it much easier to repair corrupt
repositories. Something I'm hoping to pursue even further. I've had
cases where core.checkCollisions=false + stuff on top would have made
repairing a broken repo much easier.
Anyway, I'm in no rush to send my patch. I'm happily using it in
production, but will wait for Jeff's be ready and to land before picking
it up again. Just wanted to do a braindump of the benefits.
next prev parent reply other threads:[~2018-11-10 14:04 UTC|newest]
Thread overview: 99+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-25 18:38 [RFC PATCH] index-pack: improve performance on NFS Jansen, Geert
2018-10-26 0:21 ` Junio C Hamano
2018-10-26 20:38 ` Ævar Arnfjörð Bjarmason
2018-10-27 7:26 ` Junio C Hamano
2018-10-27 9:33 ` Jeff King
2018-10-27 11:22 ` Ævar Arnfjörð Bjarmason
2018-10-28 22:50 ` [PATCH 0/4] index-pack: optionally turn off SHA-1 collision checking Ævar Arnfjörð Bjarmason
2018-10-30 2:49 ` Geert Jansen
2018-10-30 9:04 ` Junio C Hamano
2018-10-30 18:43 ` [PATCH v2 0/3] index-pack: test updates Ævar Arnfjörð Bjarmason
2018-11-13 20:19 ` [PATCH v3] index-pack: add ability to disable SHA-1 collision check Ævar Arnfjörð Bjarmason
2018-11-14 7:09 ` Junio C Hamano
2018-11-14 12:40 ` Ævar Arnfjörð Bjarmason
2018-10-30 18:43 ` [PATCH v2 1/3] pack-objects test: modernize style Ævar Arnfjörð Bjarmason
2018-10-30 18:43 ` [PATCH v2 2/3] pack-objects tests: don't leave test .git corrupt at end Ævar Arnfjörð Bjarmason
2018-10-30 18:43 ` [PATCH v2 3/3] index-pack tests: don't leave test repo dirty " Ævar Arnfjörð Bjarmason
2018-10-28 22:50 ` [PATCH 1/4] pack-objects test: modernize style Ævar Arnfjörð Bjarmason
2018-10-28 22:50 ` [PATCH 2/4] pack-objects tests: don't leave test .git corrupt at end Ævar Arnfjörð Bjarmason
2018-10-28 22:50 ` [PATCH 3/4] index-pack tests: don't leave test repo dirty " Ævar Arnfjörð Bjarmason
2018-10-28 22:50 ` [PATCH 4/4] index-pack: add ability to disable SHA-1 collision check Ævar Arnfjörð Bjarmason
2018-10-29 15:04 ` [RFC PATCH] index-pack: improve performance on NFS Jeff King
2018-10-29 15:09 ` Jeff King
2018-10-29 19:36 ` Ævar Arnfjörð Bjarmason
2018-10-29 23:27 ` Jeff King
2018-11-07 22:55 ` Geert Jansen
2018-11-08 12:02 ` Jeff King
2018-11-08 20:58 ` Geert Jansen
2018-11-08 21:18 ` Jeff King
2018-11-08 21:55 ` Geert Jansen
2018-11-08 22:20 ` Ævar Arnfjörð Bjarmason
2018-11-09 10:11 ` Ævar Arnfjörð Bjarmason
2018-11-12 14:31 ` Jeff King
2018-11-12 14:46 ` [PATCH 0/9] caching loose objects Jeff King
2018-11-12 14:46 ` [PATCH 1/9] fsck: do not reuse child_process structs Jeff King
2018-11-12 15:26 ` Derrick Stolee
2018-11-12 14:47 ` [PATCH 2/9] submodule--helper: prefer strip_suffix() to ends_with() Jeff King
2018-11-12 18:23 ` Stefan Beller
2018-11-12 14:48 ` [PATCH 3/9] rename "alternate_object_database" to "object_directory" Jeff King
2018-11-12 15:30 ` Derrick Stolee
2018-11-12 15:36 ` Jeff King
2018-11-12 19:41 ` Ramsay Jones
2018-11-12 14:48 ` [PATCH 4/9] sha1_file_name(): overwrite buffer instead of appending Jeff King
2018-11-12 15:32 ` Derrick Stolee
2018-11-12 14:49 ` [PATCH 5/9] handle alternates paths the same as the main object dir Jeff King
2018-11-12 15:38 ` Derrick Stolee
2018-11-12 15:46 ` Jeff King
2018-11-12 15:50 ` Derrick Stolee
2018-11-12 14:50 ` [PATCH 6/9] sha1-file: use an object_directory for " Jeff King
2018-11-12 15:48 ` Derrick Stolee
2018-11-12 16:09 ` Jeff King
2018-11-12 19:04 ` Stefan Beller
2018-11-22 17:42 ` Jeff King
2018-11-12 18:48 ` Stefan Beller
2018-11-12 14:50 ` [PATCH 7/9] object-store: provide helpers for loose_objects_cache Jeff King
2018-11-12 19:24 ` René Scharfe
2018-11-12 20:16 ` Jeff King
2018-11-12 14:54 ` [PATCH 8/9] sha1-file: use loose object cache for quick existence check Jeff King
2018-11-12 16:00 ` Derrick Stolee
2018-11-12 16:01 ` Ævar Arnfjörð Bjarmason
2018-11-12 16:21 ` Jeff King
2018-11-12 22:18 ` Ævar Arnfjörð Bjarmason
2018-11-12 22:30 ` Ævar Arnfjörð Bjarmason
2018-11-13 10:02 ` Ævar Arnfjörð Bjarmason
2018-11-14 18:21 ` René Scharfe
2018-12-02 10:52 ` René Scharfe
2018-12-03 22:04 ` Jeff King
2018-12-04 21:45 ` René Scharfe
2018-12-05 4:46 ` Jeff King
2018-12-05 6:02 ` René Scharfe
2018-12-05 6:51 ` Jeff King
2018-12-05 8:15 ` Jeff King
2018-12-05 18:41 ` René Scharfe
2018-12-05 20:17 ` Jeff King
2018-11-12 22:44 ` Geert Jansen
2018-11-27 20:48 ` René Scharfe
2018-12-01 19:49 ` Jeff King
2018-11-12 14:55 ` [PATCH 9/9] fetch-pack: drop custom loose object cache Jeff King
2018-11-12 19:25 ` René Scharfe
2018-11-12 19:32 ` Ævar Arnfjörð Bjarmason
2018-11-12 20:07 ` Jeff King
2018-11-12 20:13 ` René Scharfe
2018-11-12 16:02 ` [PATCH 0/9] caching loose objects Derrick Stolee
2018-11-12 19:10 ` Stefan Beller
2018-11-09 13:43 ` [RFC PATCH] index-pack: improve performance on NFS Ævar Arnfjörð Bjarmason
2018-11-09 16:08 ` Duy Nguyen
2018-11-10 14:04 ` Ævar Arnfjörð Bjarmason [this message]
2018-11-12 14:34 ` Jeff King
2018-11-12 22:58 ` Geert Jansen
2018-10-27 14:04 ` Duy Nguyen
2018-10-29 15:18 ` Jeff King
2018-10-29 0:48 ` Junio C Hamano
2018-10-29 15:20 ` Jeff King
2018-10-29 18:43 ` Ævar Arnfjörð Bjarmason
2018-10-29 21:34 ` Geert Jansen
2018-10-29 21:50 ` Jeff King
2018-10-29 22:21 ` Geert Jansen
2018-10-29 22:27 ` Jeff King
2018-10-29 22:35 ` Stefan Beller
2018-10-29 23:29 ` 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87a7mh58r0.fsf@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=gerardu@amazon.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=pclouds@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.