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

Re: [GSoC] How to protect cached_objects

From
Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Date
May 25, 2019, 14:42 UTC
Message-ID
<CAHd-oW4Y6bN+6SCJNEg+w0GMWQcUdiHmTbQfCpJ5J283xU8X+w@mail.gmail.com>
In-Reply-To
<20190524061352.GB25694@sigill.intra.peff.net>
On Fri, May 24, 2019 at 3:13 AM Jeff King <peff@peff.net> wrote:
Show 35 quoted lines
>
> On Thu, May 23, 2019 at 01:51:47PM -0300, Matheus Tavares Bernardino wrote:
>
> > As one of my first tasks in GSoC, I'm looking to protect the global
> > states at sha1-file.c for future parallelizations. Currently, I'm
> > analyzing how to deal with the cached_objects array, which is a small
> > set of in-memory objects that read_object_file() is able to return
> > although they don't really exist on disk. The only current user of
> > this set is git-blame, which adds a fake commit containing
> > non-committed changes.
> >
> > As it is now, if we start parallelizing blame, cached_objects won't be
> > a problem since it is written to only once, at the beginning, and read
> > from a couple times latter, with no possible race conditions.
> >
> > But should we make these operations thread safe for future uses that
> > could involve potential parallel writes and reads too?
> >
> > If so, we have two options:
> > - Make the array thread local, which would oblige us to replicate data, or
> > - Protect it with locks, which could impact the sequential
> > performance. We could have a macro here, to skip looking on
> > single-threaded use cases. But we don't know, a priori, the number of
> > threads that would want to use the pack access code.
>
> It seems like a lot of the sha1-reading code is 99% read-only, but very
> occasionally will require a write (e.g., refreshing the packed_git list
> when we fail a lookup, or manipulating the set of cached mmap windows).
>
> I think pthreads has read/write locks, where many readers can hold the
> lock simultaneously but a writer blocks readers (and other writers).
> Then in the common case we'd only pay the price to take the lock, and
> not deal with contention. I don't know how expensive it is to take such
> a read lock; it's presumably just a few instructions but implies a
> memory barrier. Maybe it's worth timing?

The pthread_rwlock_t, right? Nice! I didn't know about this type of lock. It will be very handy in other situations as well, thanks for the suggestion.

> -Peff
Previous: Jeff KingNext: Duy Nguyen
Message 3 of 6 in “[GSoC] How to protect cached_objects”
  1. Matheus Tavares BernardinoMay 23, 2019
  2. Jeff KingMay 24, 2019
  3. Matheus Tavares BernardinoMay 25, 2019
  4. Duy NguyenMay 24, 2019
  5. Matheus Tavares BernardinoMay 25, 2019
  6. Duy NguyenMay 26, 2019

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.