RE: [PATCH 0/5] A series of performance enhancements in the memihash and name-cache area

From: Jeff Hostetler <jeff.hostetler@microsoft.com>
Date: Mar 2, 2017, 21:18 UTC
Message-ID: <MWHPR03MB29581B0EDDEDCA7D51EC396F8A280@MWHPR03MB2958.namprd03.prod.outlook.com>
In-Reply-To: <xmqqfuivbcz0.fsf@gitster.mtv.corp.google.com>

Sorry,  $DAYJOB got in the way (again).

This is still on my short-list of things to take care of. I should have something for you next week.

Thanks again, Jeff

-----Original Message-----
From: Junio C Hamano [mailto:gitster@pobox.com] 
Sent: Thursday, March 2, 2017 4:12 PM
To: Jeff Hostetler <git@jeffhostetler.com>
Cc: Jeff King <peff@peff.net>; Johannes Schindelin <johannes.schindelin@gmx.de>; git@vger.kernel.org; Jeff Hostetler <Jeff.Hostetler@microsoft.com>
Subject: Re: [PATCH 0/5] A series of performance enhancements in the memihash and name-cache area

Jeff Hostetler <git@jeffhostetler.com> writes:

Show 24 quoted lines

> On 2/14/2017 5:03 PM, Jeff King wrote:
>> On Tue, Feb 14, 2017 at 12:31:46PM +0100, Johannes Schindelin wrote:
>>
>>> On Windows, calls to memihash() and maintaining the istate.name_hash and
>>> istate.dir_hash HashMaps take significant time on very large
>>> repositories. This series of changes reduces the overall time taken for
>>> various operations by reducing the number calls to memihash(), moving
>>> some of them into multi-threaded code, and etc.
>>>
>>> Note: one commenter in https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgit-for-windows%2Fgit%2Fpull%2F964&data=02%7C01%7CJeff.Hostetler%40microsoft.com%7C1d493f3031f74657f29308d461b0be80%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636240859121403139&sdata=16RQH1%2BrDonsanClb3%2Fwue7pcy9l7cUq9lDJenqCgbE%3D&reserved=0
>>> pointed out that memihash() only handles ASCII correctly. That is true.
>>> And fixing this is outside the purview of this patch series.
>> Out of curiosity, do you have numbers? Bonus points if the speedup can
>> be shown via a t/perf script.
>>
>> We have a read-cache perf-test already, but I suspect you'd want
>> something more like "git status" or "ls-files -o" that calls into
>> read_directory().
>
> I have some informal numbers in a spreadsheet.  I was seeing
> a 8-9% speed up on a status on my gigantic repo.
>
> I'll try to put together a before/after perf-test to better
> demonstrate this.

Ping? I do not think there is anything wrong with the changes from correctness point of view, but as the series is about performance, it somewhat feels pointless to merge to 'next' without mentioning the actual numbers.

It might be sufficient to mention the rough numbers in the log messages, if additions to t/perf/ are too cumbersome to arrange.