[prev in list] [next in list] [prev in thread] [next in thread]
List: git
Subject: Re: [PATCH] Enable index-pack threading in msysgit.
From: Duy Nguyen <pclouds () gmail ! com>
Date: 2014-03-19 10:28:27
Message-ID: CACsJy8A7ESSjfHqr96_yYjNsE-A1Sf=8+rmRfGrjML0+fCWTTg () mail ! gmail ! com
[Download RAW message or body]
On Wed, Mar 19, 2014 at 2:50 PM, Stefan Zager <szager@chromium.org> wrote:
> On Wed, Mar 19, 2014 at 12:30 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Wed, Mar 19, 2014 at 7:46 AM, <szager@chromium.org> wrote:
>>> This adds a Windows implementation of pread. Note that it is NOT
>>> safe to intersperse calls to read() and pread() on a file
>>> descriptor. According to the ReadFile spec, using the 'overlapped'
>>> argument should not affect the implicit position pointer of the
>>> descriptor. Experiments have shown that this is, in fact, a lie.
>>
>> If I understand it correctly, new pread() is added because
>> compat/pread.c does not work because of some other read() in between?
>> Where are those read() (I can only see one in index-pack.c, but there
>> could be some hidden read()..)
>
> I *think* it's the call to fixup_pack_header_footer(), but I'm not 100% sure.
>
> I suppose it would be possible to fix the immediate problem just by
> using one fd per thread, without a new pread implementation. But it
> seems better overall to have a pread() implementation that is
> thread-safe as long as read() and pread() aren't interspersed; and
> then convert all existing read() calls to pread(). That would be a
> good follow-up patch...
I still don't understand how compat/pread.c does not work with pack_fd
per thread. I don't have Windows to test, but I forced compat/pread.c
on on Linux with similar pack_fd changes and it worked fine, helgrind
only complained about progress.c.
A pread() implementation that is thread-safe with condition sounds
like an invite for trouble later. And I don't think converting read()
to pread() is a good idea. Platforms that rely on pread() will hit
first because of more use of compat/pread.c. read() seeks while
pread() does not, so we have to audit more code..
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic