Re: [GSoC Patch 0/3] Move generation, graph_pos to a slab

From: Jakub Narębski <jnareb@gmail.com>
Date: Jun 8, 2020, 15:21 UTC
Message-ID: <85o8pttwft.fsf@gmail.com>
In-Reply-To: <20200608083615.GD8232@szeder.dev>

SZEDER Gábor <szeder.dev@gmail.com> writes:

> On Mon, Jun 08, 2020 at 11:18:27AM +0530, Abhishek Kumar wrote:
>> On Sun, Jun 07, 2020 at 09:53:47PM +0200, SZEDER Gábor wrote:
>>> On Thu, Jun 04, 2020 at 10:22:27AM -0400, Derrick Stolee wrote:
>>>> On 6/4/2020 3:27 AM, Abhishek Kumar wrote:

Show 31 quoted lines

>>>>> The struct commit is used in many contexts. However, members generation
>>>>> and graph_pos are only used for commit-graph related operations and
>>>>> otherwise waste memory.
>>>>> 
>>>>> This wastage would have been more pronounced as transistion to
>>>>> generation number v2, which uses 64-bit generation number instead of
>>>>> current 32-bits.
>>>> 
>>>> Thanks! This is an important step, and will already improve
>>>> performance in subtle ways.
>>> 
>>> While the reduced memory footprint of each commit object might improve
>>> performance, accessing graph position and generation numbers in a
>>> commit-slab is more expensive than direct field accesses in 'struct
>>> commit' instances.  Consequently, these patches increase the runtime
>>> of 'git merge-base --is-ancestor HEAD~50000 HEAD' in the linux
>>> repository from 0.630s to 0.940s. 
>> 
>> Thank you for checking performance. Performance penalty was something we
>> had discussed here [1]. 
>> 
>> Caching the commit slab results in local variables helped wonderfully in v2 [2].
>> For example, the runtime of 'git merge-base --is-ancestor HEAD~50000 HEAD'
>> in the linux repository increased from 0.762 to 0.767s. Since this is a
>> change of <1%, it is *no longer* a performance regression in my opinion.
>>
>> [1]: https://lore.kernel.org/git/9a15c7ba-8b55-099a-3c59-b5e7ff6124f6@gmail.com/
>> [2]: https://lore.kernel.org/git/20200607193237.699335-5-abhishekkumar8222@gmail.com/
>
> Interesting, I measured 0.870s with v2, still a notable increase from
> 0.630s [a change of +38%].

I wonder what might be the cause for this difference. Is it difference in hardware (faster memory, larger CPU cache?), difference in operating system, or difference in position of HEAD?

On one hand it is large relative difference. On the other hand it is almost unnoticeable absolute difference of 0.25s.

I also wonder how the performance changes (with moving commit-graph data to the slab) for commands that do not use this data, like e.g.:

  $ git -o core.commitGraph=false merge-base --is-ancestor HEAD~50000 HEAD

or

  $ git gc

Sidenote: I think the performance changes should be mentioned at least
in the cover letter for the series, if not in commit message(s).

Best,

-- 
Jakub Narębski