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

Re: performance on repack

From
MKMartin Koegler <mkoegler@auto.tuwien.ac.at>
Date
Aug 15, 2007, 17:11 UTC
Message-ID
<20070815171152.GA15155@auto.tuwien.ac.at>
In-Reply-To
<9e4733910708150808x39241071j1a4012f16cd26ef8@mail.gmail.com>
On Wed, Aug 15, 2007 at 11:08:38AM -0400, Jon Smirl wrote:
Show 14 quoted lines
> On 8/15/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> > > Also... read_sha1_file() is currently not thread safe, and threaded
> > > delta searching would requires that its usage be serialized with a
> > > global mutex (not done in this patch which is a bug), or ideally be made
> > > thread aware.
> 
> You can avoid making all the low level calls thread safe by using the
> main thread to get everything into RAM before starting to search for
> the deltas. The worker threads would only deal with things completely
> in memory. You may need to ref count these in-memory objects if they
> are shared between worker threads. For simplicity the in-memory input
> objects should be read only by the threads. The worker threads create
> new structures to hand their results back to the main thread for
> writing to disk.

git-pack-objects knows the order, in which it will use the objects. A seperate thread could pre-read the next object and wait until the main thread starts processing it. After the read is complete, another thread could start computing the delta index.

git-pack-objects currently reads an object (and computes the delta index), if it is really necessary. With the pre-read, unnecessary operations would happen.

Show 5 quoted lines
> Initially I would just ignore very large objects while getting the
> basic code to work. After the basic code is working if a very large
> object is encountered when the main thread is faulting objects in, the
> main thread should just process this object on the spot using the
> existing low memory code.

I expect that the biggest gain will be for big objects, as they require more time to read+unpack the source objects and compute the delta index as well as the delta.

Show 22 quoted lines
> > @@ -1862,10 +1863,12 @@ int pretend_sha1_file(void *buf, unsigned long len, enum object_type type,
> >  void *read_sha1_file(const unsigned char *sha1, enum object_type *type,
> >                      unsigned long *size)
> >  {
> > +       static pthread_mutex_t locky = PTHREAD_MUTEX_INITIALIZER;
> >         unsigned long mapsize;
> >         void *map, *buf;
> >         struct cached_object *co;
> >
> > +       pthread_mutex_lock(&locky);
> >         co = find_cached_object(sha1);
> >         if (co) {
> >                 buf = xmalloc(co->size + 1);
> > @@ -1873,20 +1876,26 @@ void *read_sha1_file(const unsigned char *sha1, enum object_type *type,
> >                 ((char*)buf)[co->size] = 0;
> >                 *type = co->type;
> >                 *size = co->size;
> > +               pthread_mutex_unlock(&locky);
> >                 return buf;
> >         }
> >
> >         buf = read_packed_sha1(sha1, type, size);
Couldn't we release the mutex at this point?

Why do we need to protect from concurrent access, when we are reading a loose object?

Show 18 quoted lines
> > -       if (buf)
> > +       if (buf) {
> > +               pthread_mutex_unlock(&locky);
> >                 return buf;
> > +       }
> >         map = map_sha1_file(sha1, &mapsize);
> >         if (map) {
> >                 buf = unpack_sha1_file(map, mapsize, type, size, sha1);
> >                 munmap(map, mapsize);
> > +               pthread_mutex_unlock(&locky);
> >                 return buf;
> >         }
> >         reprepare_packed_git();
> > -       return read_packed_sha1(sha1, type, size);
> > +       buf = read_packed_sha1(sha1, type, size);
> > +       pthread_mutex_unlock(&locky);
> > +       return buf;
> >  }
mfg Martin Kögler
Previous: Jon SmirlNext: Jon Smirl
Message 17 of 30 in “performance on repack”
  1. Jon SmirlAug 11, 2007
  2. David KastrupAug 11, 2007
  3. Linus TorvaldsAug 11, 2007
  4. Jon SmirlAug 11, 2007
  5. Martin KoeglerAug 12, 2007
  6. Jon SmirlAug 12, 2007
  7. Shawn O. PearceAug 14, 2007
  8. Jon SmirlAug 14, 2007
  9. Shawn O. PearceAug 14, 2007
  10. Jon SmirlAug 14, 2007
  11. Nicolas PitreAug 14, 2007
  12. Nicolas PitreAug 14, 2007
  13. Jon SmirlAug 15, 2007
  14. Nicolas PitreAug 15, 2007
  15. Shawn O. PearceAug 15, 2007
  16. Jon SmirlAug 15, 2007
  17. Martin KoeglerAug 15, 2007
  18. Jon SmirlAug 15, 2007
  19. Nicolas PitreAug 15, 2007
  20. Jon SmirlAug 15, 2007
  21. David KastrupAug 16, 2007
  22. Nicolas PitreAug 16, 2007
  23. Jon SmirlAug 16, 2007
  24. Nicolas PitreAug 16, 2007
  25. Nicolas PitreAug 15, 2007
  26. Nicolas PitreAug 15, 2007
  27. Nicolas PitreAug 30, 2007
  28. Nicolas PitreAug 30, 2007
  29. Jon SmirlAug 30, 2007
  30. Jon SmirlSep 1, 2007

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.