From: Derrick Stolee Date: Wed, 04 Oct 2017 13:06:32 GMT Subject: Re: [PATCH v3 3/5] sha1_name: Unroll len loop in find_unique_abbrev_r Message-ID: In-Reply-To: On 10/4/2017 2:10 AM, Junio C Hamano wrote: > Derrick Stolee writes: > ... >> 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? In PATCH 3, 4, and 5, I used the commit-by-commit diff for the perf numbers, so the "Base Time" for PATCH 4 is the time calculated when PATCH 3 is applied. The table in the [PATCH 0/5] message includes the relative change for all commits. I recalculated the relative change for each patch related to the baseline (PATCH 2). Looking again, it appears I misspoke and PATCH 4 does include a +8% change for a fully-repacked Linux repo relative to PATCH 2. Since PATCH 5 includes an optimization targeted directly at large packfiles, the final performance gain is significant in the fully-packed cases. It is also worth looking at the absolute times for these cases, since the fully-packed case is significantly faster than the multiple-packfile case, so the relative change impacts users less. One final note: the improvement was clearer in test p0008.1 when the test included "sort -R" to shuffle the known OIDs. Providing OIDs in lexicographic order has had a significant effect on the performance, which does not reflect real-world usage. I removed the "sort -R" because it is a GNU-ism, but if there is a good cross-platform alternative I would be happy to replace it. 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: | Repo  | Baseline | Patch 3 | Rel % | Patch 4 | Rel % | Patch 5 | Rel % | |-------|----------|---------|-------|---------|-------|---------|-------| | Git   | 0.09     | 0.06    | -33%  | 0.05    | -44%  | 0.05    | -44%  | | Git   | 0.11     | 0.08    | -27%  | 0.08    | -27%  | 0.08    | -27%  | | Git   | 0.09     | 0.07    | -22%  | 0.06    | -33%  | 0.06    | -33%  | | Linux | 0.13     | 0.32    | 146%  | 0.14    | + 8%  | 0.05    | -62%  | | Linux | 1.13     | 1.12    | - 1%  | 0.94    | -17%  | 0.88    | -22%  | | Linux | 1.08     | 1.05    | - 3%  | 0.86    | -20%  | 0.80    | -26%  | | VSTS  | 0.12     | 0.23    | +92%  | 0.11    | - 8%  | 0.05    | -58%  | | VSTS  | 1.02     | 1.08    | + 6%  | 0.95    | - 7%  | 0.95    | - 7%  | | VSTS  | 2.25     | 2.08    | - 8%  | 1.82    | -19%  | 1.93    | -14%  | (Each repo has three versions, in order: 1 packfile, multiple packfiles, and multiple packfiles and loose objects.) Thanks, -Stolee