Re: [PATCH] pack-objects: document about thread synchronization

From: Jeff King <peff@peff.net>
Date: Aug 2, 2018, 19:39 UTC
Message-ID: <20180802193912.GB29084@sigill.intra.peff.net>
In-Reply-To: <CACsJy8AKHo4g5607GdiuLVBy1746gEEFbsYjB2RLwvQvE24Sgg@mail.gmail.com>

On Sun, Jul 29, 2018 at 05:46:17PM +0200, Duy Nguyen wrote:

Show 10 quoted lines

> tOn Sun, Jul 29, 2018 at 5:36 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> >
> > These extra comments should be make it easier to understand how to use
> > locks in pack-objects delta search code. For reference, see
> 
> Side note. I wonder if we could take progress_lock() less often in
> find_deltas() to reduce contention. Right now we take the lock every
> time we check out one object_entry but we could pull like 16 items out
> of the list per lock. I don't know how much actual contention on this
> lock though so maybe not worth doing.

I doubt it really matters that much, since we hold it for such a small amount of time (basically just a few pointer/integer tweaks). Compared to the heavy lifting of actually loading objects, you're not likely to see a huge amount of contention, since at any given moment most threads will be doing that work (or actually computing deltas).

Of course I could be wrong. If you hit a point where many threads are skipping work (e.g., because they are considering deltas from objects in the same pack, and skip forward without actually doing any work), then they'd be ripping through the find_deltas() loop pretty quickly.

OTOH, in cases like that (and the ultimate case would just be running "git repack -ad" twice in a row), the compression phase seems to go quite quickly, implying we're not spending a lot of time there.

-Peff