From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
dstolee@microsoft.com, git@jeffhostetler.com, peff@peff.net,
johannes.schindelin@gmx.de, jrnieder@gmail.com,
"Linus Torvalds" <torvalds@linux-foundation.org>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [RFC PATCH 0/2] unconditional O(1) SHA-1 abbreviation
Date: Wed, 6 Jun 2018 10:27:17 +0000 [thread overview]
Message-ID: <20180606102719.27145-1-avarab@gmail.com> (raw)
In-Reply-To: <87lgbsz61p.fsf@evledraar.gmail.com>
On Wed, Jun 06 2018, Ævar Arnfjörð Bjarmason wrote:
> On Mon, Jan 08 2018, Derrick Stolee wrote:
>
>> On 1/7/2018 5:42 PM, Ævar Arnfjörð Bjarmason wrote:
>>>
>>> On Sun, Jan 07 2018, Derrick Stolee jotted:
>>>
>>>> git log --oneline --raw --parents
>>>>
>>>> Num Packs | Before MIDX | After MIDX | Rel % | 1 pack %
>>>> ----------+-------------+------------+--------+----------
>>>> 1 | 35.64 s | 35.28 s | -1.0% | -1.0%
>>>> 24 | 90.81 s | 40.06 s | -55.9% | +12.4%
>>>> 127 | 257.97 s | 42.25 s | -83.6% | +18.6%
>>>>
>>>> The last column is the relative difference between the MIDX-enabled repo
>>>> and the single-pack repo. The goal of the MIDX feature is to present the
>>>> ODB as if it was fully repacked, so there is still room for improvement.
>>>>
>>>> Changing the command to
>>>>
>>>> git log --oneline --raw --parents --abbrev=40
>>>>
>>>> has no observable difference (sub 1% change in all cases). This is likely
>>>> due to the repack I used putting commits and trees in a small number of
>>>> packfiles so the MRU cache workes very well. On more naturally-created
>>>> lists of packfiles, there can be up to 20% improvement on this command.
>>>>
>>>> We are using a version of this patch with an upcoming release of GVFS.
>>>> This feature is particularly important in that space since GVFS performs
>>>> a "prefetch" step that downloads a pack of commits and trees on a daily
>>>> basis. These packfiles are placed in an alternate that is shared by all
>>>> enlistments. Some users have 150+ packfiles and the MRU misses and
>>>> abbreviation computations are significant. Now, GVFS manages the MIDX file
>>>> after adding new prefetch packfiles using the following command:
>>>>
>>>> git midx --write --update-head --delete-expired --pack-dir=<alt>
>>>
>>> (Not a critique of this, just a (stupid) question)
>>>
>>> What's the practical use-case for this feature? Since it doesn't help
>>> with --abbrev=40 the speedup is all in the part that ensures we don't
>>> show an ambiguous SHA-1.
>>
>> The point of including the --abbrev=40 is to point out that object
>> lookups do not get slower with the MIDX feature. Using these "git log"
>> options is a good way to balance object lookups and abbreviations with
>> object parsing and diff machinery.[...]
>
> [snip]
>
>> [...]And while the public data shape I shared did not show a
>> difference, our private testing of the Windows repository did show a
>> valuable improvement when isolating to object lookups and ignoring
>> abbreviation calculations.
>
> Replying to this old thread since I see you're prepearing the MIDX for
> submission again and this seemed like the best venue.
>
> Your WIP branch (github.com/git/derrickstolee/midx/upstream) still only
> references the speedups in abbreviation calculations, but here you
> allude to other improvements. It would be very nice to have some summary
> of that in docs / commit messages when you submit this.
>
> I've been meaning to get around to submitting something like I mentioned
> in https://public-inbox.org/git/87efn0bkls.fsf@evledraar.gmail.com/
> i.e. a way to expand the abbrev mode to not check disambiguations, which
> would look something like:
>
> core.abbrev = 20
> core.validateAbbrev = false
>
> Or:
>
> core.abbrev = +2
> core.validateAbbrev = false
>
> So, using the example from the above referenced E-Mail +2 would make
> linux.git emit hashes of 14 characters, without any abbreviation
> checking (just trusting in statistics to work in your favor).
>
> As noted by JS in this thread that wouldn't be acceptable for your
> use-case, but there's plenty of people (including me) who'd appreciate
> the speedup without being a 100% sure we're emitting unambiguous hashes,
> since that trade-off is better than time spent generating another index
> on-disk. So I see it as a complimentary & orthogonal feature.
>
> But with that implemented I wouldn't get any benefit from things that
> use the MIDX that aren't abbreviations, so what are those?
I won't have time to finish this today, but it's already in a shape
that I think is useful for discussion to see what others think.
I still need to make this be supported by --abbrev=* and have
e.g. --abbrev=+2 work. I got as far as this with that:
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 0f9f311a7a..7cc9d3dfe6 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -16,13 +16,23 @@ int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset)
if (!arg) {
v = unset ? 0 : DEFAULT_ABBREV;
} else {
+ const char *origarg = arg;
v = strtol(arg, (char **)&arg, 10);
if (*arg)
return opterror(opt, "expects a numerical value", 0);
- if (v && v < MINIMUM_ABBREV)
+ if (*origarg == '+' || *origarg == '-') {
+ if (v == 0) {
+ return opterror(opt, "relative abbrev must be non-zero", 0);
+ } else {
+ default_abbrev_relative = v;
+ v = -1;
+ }
+ } else if (v && v < MINIMUM_ABBREV) {
v = MINIMUM_ABBREV;
- else if (v > 40)
+ } else if (v > 40) {
v = 40;
+ }
}
*(int *)(opt->value) = v;
return 0;
But e.g. blame would print 40 character SHA-1s on +2, I didn't have
time to dig into why.
This'll also need tests, I haven't added any yet, and finally it's
probably a good idea to split off the core.abbrev=[+-]NUM feature into
a seperate patch from core.validateAbbrev, although with my 2/2 the
two can be used in isolation, or together.
Ævar Arnfjörð Bjarmason (2):
config.c: use braces on multiple conditional arms
sha1-name: add core.validateAbbrev & relative core.abbrev
Documentation/config.txt | 46 ++++++++++++++++++++++++++++++++++++++++
cache.h | 2 ++
config.c | 18 ++++++++++++++--
environment.c | 2 ++
sha1-name.c | 15 +++++++++++++
5 files changed, 81 insertions(+), 2 deletions(-)
--
2.17.0.290.gded63e768a
next prev parent reply other threads:[~2018-06-06 10:27 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-07 18:14 [RFC PATCH 00/18] Multi-pack index (MIDX) Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 01/18] docs: Multi-Pack Index (MIDX) Design Notes Derrick Stolee
2018-01-08 19:32 ` Jonathan Tan
2018-01-08 20:35 ` Derrick Stolee
2018-01-08 22:06 ` Jonathan Tan
2018-01-07 18:14 ` [RFC PATCH 02/18] midx: specify midx file format Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 03/18] midx: create core.midx config setting Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 04/18] midx: write multi-pack indexes for an object list Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 05/18] midx: create midx builtin with --write mode Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 06/18] midx: add t5318-midx.sh test script Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 07/18] midx: teach midx --write to update midx-head Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 08/18] midx: teach git-midx to read midx file details Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 09/18] midx: find details of nth object in midx Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 10/18] midx: use existing midx when writing Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 11/18] midx: teach git-midx to clear midx files Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 12/18] midx: teach git-midx to delete expired files Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 13/18] t5318-midx.h: confirm git actions are stable Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 14/18] midx: load midx files when loading packs Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 15/18] midx: use midx for approximate object count Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 16/18] midx: nth_midxed_object_oid() and bsearch_midx() Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 17/18] sha1_name: use midx for abbreviations Derrick Stolee
2018-01-07 18:14 ` [RFC PATCH 18/18] packfile: use midx for object loads Derrick Stolee
2018-01-07 22:42 ` [RFC PATCH 00/18] Multi-pack index (MIDX) Ævar Arnfjörð Bjarmason
2018-01-08 0:08 ` Derrick Stolee
2018-01-08 10:20 ` Jeff King
2018-01-08 10:27 ` Jeff King
2018-01-08 12:28 ` Ævar Arnfjörð Bjarmason
2018-01-08 13:43 ` Johannes Schindelin
2018-01-09 6:50 ` Jeff King
2018-01-09 13:05 ` Johannes Schindelin
2018-01-09 19:51 ` Stefan Beller
2018-01-09 20:12 ` Junio C Hamano
2018-01-09 20:16 ` Stefan Beller
2018-01-09 21:31 ` Junio C Hamano
2018-01-10 17:05 ` Johannes Schindelin
2018-01-10 10:57 ` Jeff King
2018-01-08 13:43 ` Derrick Stolee
2018-01-09 7:12 ` Jeff King
2018-01-08 11:43 ` Ævar Arnfjörð Bjarmason
2018-06-06 8:13 ` Ævar Arnfjörð Bjarmason
2018-06-06 10:27 ` Ævar Arnfjörð Bjarmason [this message]
2018-06-06 10:27 ` [RFC PATCH 1/2] config.c: use braces on multiple conditional arms Ævar Arnfjörð Bjarmason
2018-06-06 10:27 ` [RFC PATCH 2/2] sha1-name: add core.validateAbbrev & relative core.abbrev Ævar Arnfjörð Bjarmason
2018-06-06 12:04 ` Christian Couder
2018-06-06 11:24 ` [RFC PATCH 00/18] Multi-pack index (MIDX) Derrick Stolee
2018-01-10 18:25 ` Martin Fick
2018-01-10 19:39 ` Derrick Stolee
2018-01-10 21:01 ` Martin Fick
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=20180606102719.27145-1-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=dstolee@microsoft.com \
--cc=git@jeffhostetler.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johannes.schindelin@gmx.de \
--cc=jrnieder@gmail.com \
--cc=peff@peff.net \
--cc=torvalds@linux-foundation.org \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).