git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Karsten Blees <karsten.blees@gmail.com>
Date
Mar 21, 2014, 20:01 UTC
Message-ID
<532C9AA9.1010102@gmail.com>
In-Reply-To
<CAHOQ7J-sUt3HGYNE7n=X3ZmV3Q-n+n9hMDAtzLbH3YU8iAqoqA@mail.gmail.com>
Am 20.03.2014 22:56, schrieb Stefan Zager:
Show 23 quoted lines
> On Thu, Mar 20, 2014 at 2:35 PM, Karsten Blees <karsten.blees@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 in the code; it should be possible
>>> to rationalize them and make them thread-safe -- at least, thread-safe
>>> for posix-compliant systems and msysgit, which covers the great
>>> majority of git users, I would hope.
>>>
>>
>> IMO a "mostly" XSI compliant pread (or even the git_pread() emulation) is still better than forbidding the use of read() entirely. Switching from read to pread everywhere requires that all callers have to keep track of the file position, which means a _lot_ of code changes (read/xread/strbuf_read is used in ~70 places throughout git). And how do you plan to deal with platforms that don't have a thread-safe pread (HP, Cygwin)?
>>
>> 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)
> approach to threading checkout and status?  That seems kind of
> crazy-town to me.  Not to mention that pack windows are not shared, so
> this approach to multi-threading can have the side-effect of blowing
> out memory consumption.  We have already had to dial back settings for
> pack.threads and core.deltaBaseCacheLimit, because threaded index-pack
> was causing OOM errors on 32-bit platforms.
> 
Opening more file descriptors doesn't significantly increase the memory footprint, so it shouldn't matter whether the threads read data via shared or private descriptors.
git-status with core.preloadindex is already multithreaded (at least the first part), and AFAIK doesn't read pack files at all.
I'm still not convinced that multi-threaded git-checkout is a good idea. According to my tests this is actually slower than sequential checkout. You'd have to be very careful to only multi-thread the parts that don't do any IO, such as unpacking / undeltifying.
Previous: Duy NguyenNext: Duy Nguyen
Message 13 of 19 in “Enable index-pack threading in msysgit.”
  1. Enable index-pack threading in msysgit.szager@chromium.org, Mar 19, 2014
  2. Duy NguyenMar 19, 2014
  3. Stefan ZagerMar 19, 2014
  4. Duy NguyenMar 19, 2014
  5. Stefan ZagerMar 19, 2014
  6. Stefan ZagerMar 19, 2014
  7. Junio C HamanoMar 19, 2014
  8. Karsten BleesMar 20, 2014
  9. Stefan ZagerMar 20, 2014
  10. Karsten BleesMar 20, 2014
  11. Stefan ZagerMar 20, 2014
  12. Duy NguyenMar 21, 2014
  13. Karsten BleesMar 21, 2014
  14. Duy NguyenMar 21, 2014
  15. Duy NguyenMar 21, 2014
  16. Stefan ZagerMar 21, 2014
  17. Karsten BleesMar 21, 2014
  18. index-pack: work around thread-unsafe pread()Nguyễn Thái Ngọc Duy, Mar 25, 2014
  19. Johannes SixtMar 26, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.