Subject: Re: [PATCH] Enable index-pack threading in msysgit.

From: Karsten Blees <[email-masked]>
Date: Fri, 21 Mar 2014 19:55:06 +0100

Am 21.03.2014 06:35, schrieb Stefan Zager:
> On Thu, Mar 20, 2014 at 10:21 PM, Duy Nguyen <[email-masked]> wrote:
>> On Fri, Mar 21, 2014 at 08:51:18AM +0700, Duy Nguyen wrote:
>>> On Thu, Mar 20, 2014 at 11:08 PM, Stefan Zager <[email-masked]> wrote:
>>>> Duy, would you like to re-post your patch without the new pread implementation?
>>>
>>> I will but let me try out the sliding window idea first. My quick
>>> tests on git.git show me we may only need 21k mmap instead of 177k
>>> pread. That hints some potential performance improvement.
>>
>> The patch at the bottom reuses (un)use_pack() instead of pread(). The
>> results on linux-2.6 do not look any different. I guess we can drop
>> the idea.
>>
>> It makes me wonder, though, what's wrong a simple patch like this to
>> make pread in index-pack thread-safe? It does not look any different
>> either from the performance point of view, perhaps because
>> unpack_data() reads small deltas most of the time
> 
> When you serialize disk access in this way, the effect on performance
> is really dependent on the behavior of the OS, as well as the locality
> of the read offsets.  Assuming -- fairly, I think -- that the reads
> will be pretty randomly distributed (i.e., no locality to speak of),
> then your best bet is get as many read operations in flight as
> possible, and let the disk scheduler optimize the seek time.
> 

The read() implementation in MSVCRT.DLL is synchronized anyway, and I strongly suspect that this is also true for ReadFile() (at least for synchronous file handles, i.e. opened without FILE_FLAG_OVERLAPPED). So I guess separate file descriptors would help with parallel IO as well.