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

Re: [PATCH v2 05/11] object-store: allow threaded access to object reading

From
Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Date
Nov 14, 2019, 05:57 UTC
Message-ID
<CAHd-oW4PtO2CKpd3HFgrJWmQf3MN+MHt5c7V6OGx33LgU-CrOQ@mail.gmail.com>
In-Reply-To
<20191113052044.GB3547@sigill.intra.peff.net>
Hi, Peff and Jonathan
On Wed, Nov 13, 2019 at 2:20 AM Jeff King <peff@peff.net> wrote:
>
> On Mon, Nov 11, 2019 at 06:54:18PM -0800, Jonathan Tan wrote:
[...]
Show 6 quoted lines
> > My concern is that the presence of these "holes" make object reading
> > non-thread-safe, defeating the purpose of obj_read_mutex. In particular,
> > the section quoted above assumes that the window section returned by
> > use_pack() is still valid throughout the inflation, but that window
> > could have been invalidated by things like an excess of windows open,
> > reprepare_packed_git(), etc.
Thank you for spotting this issue!
Show 10 quoted lines
> > I thought of this for a while but couldn't think of a good solution. If
> > we introduced a reference counting mechanism into Git, that would allow
> > us to hold the window open outside the mutex, but things like
> > reprepare_packed_git() would still be difficult.
>
> I think you could put a reader-writer lock into each window. The code
> here would take the reader lock, and multiple readers could use it at
> the same time. Any time the window needs to be shifted, resized, or
> discarded, that code would take the writer lock, waiting for (and then
> blocking out) any readers.
Great idea, I'll work on that. Thanks!

About the other parallel inflation calls on loose objects at unpack_loose_short_header(), unpack_loose_header_to_strbuf() and unpack_loose_rest(): could they suffer from a similar race problem? FWIU, the inflation input used in these cases comes from map_loose_object(), and it's not referenced outside this scope. So I think there's no risk of one thread munmapping the object file while other is inflating it. Is that right?

> A pthread_rwlock would work, but it would be the first use in Git. I
> think we'd need to find an equivalent for compat/win32/pthread.h.

These[1][2] seems to be the equivalent options on Windows. I'll have to read these docs more carefully, but [2] seems to be more interesting in terms of speed. Also, the extra features of [1] are not really needed for our use case here.

[1]: https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/reader-writer-spin-locks [2]: https://docs.microsoft.com/en-us/windows/win32/sync/slim-reader-writer--srw--locks

Previous: Jeff KingNext: Jeff King
Message 14 of 47 in “grep: re-enable threads when cached, w/ parallel inflation”
  1. Matheus TavaresAug 10, 2019
  2. [GSoC][PATCH 1/4] object-store: add lock to read_object_file_extended()Matheus Tavares, Aug 10, 2019
  3. [GSoC][PATCH 2/4] grep: allow locks to be enabled individuallyMatheus Tavares, Aug 10, 2019
  4. [GSoC][PATCH 3/4] grep: disable grep_read_mutex when possibleMatheus Tavares, Aug 10, 2019
  5. [GSoC][PATCH 4/4] grep: re-enable threads in some non-worktree casesMatheus Tavares, Aug 10, 2019
  6. 00/11 grep: improve threading and fix race conditionsMatheus Tavares, Sep 30, 2019
  7. 01/11 grep: fix race conditions on userdiff callsMatheus Tavares, Sep 30, 2019
  8. 02/11 grep: fix race conditions at grep_submodule()Matheus Tavares, Sep 30, 2019
  9. 03/11 grep: fix racy calls in grep_objects()Matheus Tavares, Sep 30, 2019
  10. 04/11 replace-object: make replace operations thread-safeMatheus Tavares, Sep 30, 2019
  11. 05/11 object-store: allow threaded access to object readingMatheus Tavares, Sep 30, 2019
  12. Jonathan TanNov 12, 2019
  13. Jeff KingNov 13, 2019
  14. Matheus Tavares BernardinoNov 14, 2019
  15. Jeff KingNov 14, 2019
  16. Jonathan TanNov 14, 2019
  17. Jeff KingNov 15, 2019
  18. Matheus Tavares BernardinoDec 19, 2019
  19. Matheus Tavares BernardinoJan 9, 2020
  20. Christian CouderJan 10, 2020
  21. 06/11 grep: replace grep_read_mutex by internal obj read lockMatheus Tavares, Sep 30, 2019
  22. squash! grep: replace grep_read_mutex by internal obj read lockMatheus Tavares, Oct 1, 2019
  23. 07/11 submodule-config: add skip_if_read option to repo_read_gitmodules()Matheus Tavares, Sep 30, 2019
  24. 08/11 grep: allow submodule functions to run in parallelMatheus Tavares, Sep 30, 2019
  25. 09/11 grep: protect packed_git [re-]initializationMatheus Tavares, Sep 30, 2019
  26. 10/11 grep: re-enable threads in non-worktree caseMatheus Tavares, Sep 30, 2019
  27. 11/11 grep: move driver pre-load out of critical sectionMatheus Tavares, Sep 30, 2019
  28. 00/12 grep: improve threading and fix race conditionsMatheus Tavares, Jan 16, 2020
  29. 01/12 grep: fix race conditions on userdiff callsMatheus Tavares, Jan 16, 2020
  30. 02/12 grep: fix race conditions at grep_submodule()Matheus Tavares, Jan 16, 2020
  31. 03/12 grep: fix racy calls in grep_objects()Matheus Tavares, Jan 16, 2020
  32. 04/12 replace-object: make replace operations thread-safeMatheus Tavares, Jan 16, 2020
  33. 05/12 object-store: allow threaded access to object readingMatheus Tavares, Jan 16, 2020
  34. 06/12 grep: replace grep_read_mutex by internal obj read lockMatheus Tavares, Jan 16, 2020
  35. 07/12 submodule-config: add skip_if_read option to repo_read_gitmodules()Matheus Tavares, Jan 16, 2020
  36. 08/12 grep: allow submodule functions to run in parallelMatheus Tavares, Jan 16, 2020
  37. SZEDER GáborJan 29, 2020
  38. Junio C HamanoJan 29, 2020
  39. Junio C HamanoJan 29, 2020
  40. Matheus Tavares BernardinoJan 29, 2020
  41. Philippe BlainJan 30, 2020
  42. 09/12 grep: protect packed_git [re-]initializationMatheus Tavares, Jan 16, 2020
  43. 10/12 grep: re-enable threads in non-worktree caseMatheus Tavares, Jan 16, 2020
  44. 11/12 grep: move driver pre-load out of critical sectionMatheus Tavares, Jan 16, 2020
  45. 12/12 grep: use no. of cores as the default no. of threadsMatheus Tavares, Jan 16, 2020
  46. Victor LeschukJan 16, 2020
  47. Matheus TavaresJan 16, 2020

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.