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

2014-03-26 Thread Johannes Sixt
Am 3/19/2014 1:46, schrieb sza...@chromium.org: 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

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

2014-03-21 Thread Karsten Blees
Am 20.03.2014 02:25, schrieb Duy Nguyen: On Thu, Mar 20, 2014 at 4:35 AM, Stefan Zager sza...@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

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

2014-03-21 Thread Karsten Blees
Am 21.03.2014 06:35, schrieb Stefan Zager: On Thu, Mar 20, 2014 at 10:21 PM, Duy Nguyen pclo...@gmail.com 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 sza...@chromium.org wrote: Duy, would you like to re-post your patch

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

2014-03-21 Thread Karsten Blees
Am 20.03.2014 22:56, schrieb Stefan Zager: On Thu, Mar 20, 2014 at 2:35 PM, Karsten Blees karsten.bl...@gmail.com wrote: Am 20.03.2014 17:08, schrieb Stefan Zager: Going forward, there is still a lot of performance that gets left on the table when you rule out threaded file access. There

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

2014-03-20 Thread Karsten Blees
Am 19.03.2014 01:46, schrieb sza...@chromium.org: This adds a Windows implementation of pread. Note that it is NOT safe to intersperse calls to read() and pread() on a file descriptor. This is a bad idea. You're basically fixing the multi-threaded issue twice, while at the same time breaking

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

2014-03-20 Thread Stefan Zager
On Thu, Mar 20, 2014 at 6:54 AM, Karsten Blees karsten.bl...@gmail.com wrote: Am 19.03.2014 01:46, schrieb sza...@chromium.org: This adds a Windows implementation of pread. Note that it is NOT safe to intersperse calls to read() and pread() on a file descriptor. This is a bad idea. You're

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

2014-03-20 Thread Karsten Blees
Am 20.03.2014 17:08, schrieb Stefan Zager: On Thu, Mar 20, 2014 at 6:54 AM, Karsten Blees karsten.bl...@gmail.com wrote: Am 19.03.2014 01:46, schrieb sza...@chromium.org: This adds a Windows implementation of pread. Note that it is NOT safe to intersperse calls to read() and pread() on a

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

2014-03-20 Thread Stefan Zager
On Thu, Mar 20, 2014 at 2:35 PM, Karsten Blees karsten.bl...@gmail.com wrote: Am 20.03.2014 17:08, schrieb Stefan Zager: Going forward, there is still a lot of performance that gets left on the table when you rule out threaded file access. There are not so many calls to read, mmap, and pread

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

2014-03-20 Thread Duy Nguyen
On Fri, Mar 21, 2014 at 4:56 AM, Stefan Zager sza...@chromium.org wrote: Considering all that, Duy's solution of opening separate file descriptors per thread seems to be the best pattern for future multi-threaded work. Does that mean you would endorse the (N threads) * (M pack files)

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

2014-03-20 Thread Duy Nguyen
On Thu, Mar 20, 2014 at 11:08 PM, Stefan Zager sza...@chromium.org 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.

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

2014-03-20 Thread Duy Nguyen
On Fri, Mar 21, 2014 at 08:51:18AM +0700, Duy Nguyen wrote: On Thu, Mar 20, 2014 at 11:08 PM, Stefan Zager sza...@chromium.org 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

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

2014-03-20 Thread Stefan Zager
On Thu, Mar 20, 2014 at 10:21 PM, Duy Nguyen pclo...@gmail.com 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 sza...@chromium.org wrote: Duy, would you like to re-post your patch without the new pread implementation? I

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

2014-03-19 Thread Duy Nguyen
On Wed, Mar 19, 2014 at 7:46 AM, sza...@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

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

2014-03-19 Thread Stefan Zager
On Wed, Mar 19, 2014 at 12:30 AM, Duy Nguyen pclo...@gmail.com wrote: On Wed, Mar 19, 2014 at 7:46 AM, sza...@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

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

2014-03-19 Thread Duy Nguyen
On Wed, Mar 19, 2014 at 2:50 PM, Stefan Zager sza...@chromium.org wrote: On Wed, Mar 19, 2014 at 12:30 AM, Duy Nguyen pclo...@gmail.com wrote: On Wed, Mar 19, 2014 at 7:46 AM, sza...@chromium.org wrote: This adds a Windows implementation of pread. Note that it is NOT safe to intersperse

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

2014-03-19 Thread Stefan Zager
On Wed, Mar 19, 2014 at 3:28 AM, Duy Nguyen pclo...@gmail.com wrote: On Wed, Mar 19, 2014 at 2:50 PM, Stefan Zager sza...@chromium.org wrote: 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

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

2014-03-19 Thread Stefan Zager
On Wed, Mar 19, 2014 at 9:57 AM, Stefan Zager sza...@chromium.org wrote: 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

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

2014-03-19 Thread Junio C Hamano
sza...@chromium.org writes: 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

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

2014-03-19 Thread Junio C Hamano
sza...@chromium.org (Stefan Zager) writes: 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

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

2014-03-19 Thread Duy Nguyen
On Thu, Mar 20, 2014 at 4:35 AM, Stefan Zager sza...@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