Re: [PATCH 09/12] receive-pack: use oidset to de-duplicate .have lines

From: Jeff King <peff@peff.net>
Date: Jan 25, 2017, 19:54 UTC
Message-ID: <20170125195425.q4fpvc4ten5mfjgl@sigill.intra.peff.net>
In-Reply-To: <xmqqefzraqbu.fsf@gitster.mtv.corp.google.com>

On Wed, Jan 25, 2017 at 11:32:05AM -0800, Junio C Hamano wrote:

Show 18 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > If you have an alternate object store with a very large
> > number of refs, the peak memory usage of the sha1_array can
> > grow high, even if most of them are duplicates that end up
> > not being printed at all.
> > ...
> > Signed-off-by: Jeff King <peff@peff.net>
> > ---
> >  builtin/receive-pack.c | 26 ++++++++++++--------------
> >  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> Nice.  
> 
> Incidentally, this also shows that the refnames in alternate ref
> namespace will not matter.  We are to only show just one of many
> anyway (and that name is masked and replaced with ".have").  Perhaps
> we want to do 04/12 without refname?

I kept the refnames there as a "plausible" thing that a callback might want. But I have ulterior motives. :)

I have another series which uses alternate refs as part of check_connected(). Since that function calls rev-list, it needs to add something like "rev-list --alternate-refs". Even check_connected() does not care about the refnames, it seems like that rev-list option should have some useful name for each ref. It can make things like "--source" more sensible, instead of just reporting everything as a blank ".have".

I could be persuaded otherwise, though. I don't think we pay a huge for the refnames here. The generating for-each-ref has them either way, and the receiving side only keeps one in memory at a time. So we are really only paying to send them across the pipe and parse them, which doesn't seem extravagant. I didn't actually measure it, though.

-Peff