Re: [PATCH] fetch-pack: always allow fetching of literal SHA1s

From: Jeff King <peff@peff.net>
Date: May 11, 2017, 21:04 UTC
Message-ID: <20170511210418.crsv2lolwoofdv3i@sigill.intra.peff.net>
In-Reply-To: <20170511190354.GA12516@aiede.svl.corp.google.com>

On Thu, May 11, 2017 at 12:03:54PM -0700, Jonathan Nieder wrote:

Show 10 quoted lines

> > But even leaving all the refs/pull stuff aside, allowAnySHA1InWant does
> > seem to increase that confusion, and I don't see a way around it short
> > of never sharing objects between repositories at all. So I think at most
> > we'd do allowReachableSHA1InWant.
> 
> I had guessed you didn't want to do allowReachableSHA1InWant for
> performance reasons.  (I haven't checked to what extent we are already
> taking advantage of bitmaps to avoid a slow reachability check.)  If I
> was wrong and allowReachableSHA1InWant is on the table then it is of
> course even better. :)

Performance is definitely a consideration for resolving sha1's via the website, because those can be any object, and you have to look in all the trees. Traversing all the commits in linux.git is ~5s of CPU. Doing the same for the whole object graph is more like ~45s.

Our experiments did involve bitmaps, but there are some corner cases where we might not have good bitmap coverage of certain refs (I think this is something that could be improved; the commit selection in the current bitmap writer is fairly naive).

But I think upload-pack's reachability check only handles commits anyway. Adding 5s of CPU to a request isn't great, but it's pretty easy to convince Git to use 5s of CPU already. And this would only kick in when the client asked for a non-tip ref anyway, and in general I'd expect requested sha1's to be near the ref tips.

I'll open a discussion internally about enabling it and measuring the performance.

-Peff