Re: [PATCH v3 3/5] sha1_name: Unroll len loop in find_unique_abbrev_r

From: Junio C Hamano <gitster@pobox.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: Derrick Stolee <dstolee@microsoft.com>,
	git@vger.kernel.org, git@jeffhostetler.com, sbeller@google.com
Subject: Re: [PATCH v3 3/5] sha1_name: Unroll len loop in find_unique_abbrev_r
Date: Wed, 04 Oct 2017 15:10:24 +0900	[thread overview]
Message-ID: <xmqqpoa3cujz.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <6a3804a4-9728-d122-7fac-f297fc7977d1@gmail.com> (Derrick Stolee's message of "Tue, 3 Oct 2017 07:26:02 -0400")

Derrick Stolee <stolee@gmail.com> writes:

> On 10/3/2017 6:49 AM, Junio C Hamano wrote:
>> Derrick Stolee <dstolee@microsoft.com> writes:
>>
>>> p0008.1: find_unique_abbrev() for existing objects
>>> --------------------------------------------------
>>>
>>> For 10 repeated tests, each checking 100,000 known objects, we find the
>>> following results when running in a Linux VM:
>>>
>>> |       | Pack  | Packed  | Loose  | Base   | New    |         |
>>> | Repo  | Files | Objects | Objects| Time   | Time   | Rel%    |
>>> |-------|-------|---------|--------|--------|--------|---------|
>>> | Git   |     1 |  230078 |      0 | 0.09 s | 0.06 s | - 33.3% |
>>> | Git   |     5 |  230162 |      0 | 0.11 s | 0.08 s | - 27.3% |
>>> | Git   |     4 |  154310 |  75852 | 0.09 s | 0.07 s | - 22.2% |
>>> | Linux |     1 | 5606645 |      0 | 0.12 s | 0.32 s | +146.2% |
>>> | Linux |    24 | 5606645 |      0 | 1.12 s | 1.12 s | -  0.9% |
>>> | Linux |    23 | 5283204 | 323441 | 1.08 s | 1.05 s | -  2.8% |
>>> | VSTS  |     1 | 4355923 |      0 | 0.12 s | 0.23 s | + 91.7% |
>>> | VSTS  |    32 | 4355923 |      0 | 1.02 s | 1.08 s | +  5.9% |
>>> | VSTS  |    31 | 4276829 |  79094 | 2.25 s | 2.08 s | -  7.6% |
>>
>> The above does not look so good, especially in cases where a
>> repository is well maintained by packing into smaller number of
>> packs, we get much worse result?
>
> I understand that this patch on its own does not have good numbers. I
> split the
> patches 3 and 4 specifically to highlight two distinct changes:
>
> Patch 3: Unroll the len loop that may inspect all files multiple times.
> Patch 4: Parse less while disambiguating.
>
> Patch 4 more than makes up for the performance hits in this patch.

Now you confused me even more.  When we read the similar table that
appears in [Patch 4/5], what does the "Base Time" column mean?
Vanilla Git with [Patch 3/5] applied?  Vanillay Git with [Patch 4/5]
alone applied?  Something else?