Re: [PATCH 03/11] Use new compress helpers in fast-import

From: Shawn O. Pearce <spearce@spearce.org>
Date: Feb 4, 2008, 01:48 UTC
Message-ID: <20080204014849.GG24004@spearce.org>
In-Reply-To: <7v3as9slc6.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:

Show 17 quoted lines

> Marco Costalba <mcostalba@gmail.com> writes:
> 
> > Here is slightly more difficult, in particular
> > a xrealloc() has been substituted with a
> > free() + xmalloc() to keep the code simple.
> >
> > Signed-off-by: Marco Costalba <mcostalba@gmail.com>
> > ---
> >  fast-import.c |   45 +++++++++++++++------------------------------
> >  1 files changed, 15 insertions(+), 30 deletions(-)
> 
> I'll let Shawn comment on this.  The realloc() does not seem to
> be using the contents in the buffer from the previous round, so
> I suspect that a free() followed by an independent alloc() would
> be an improvement when the later call uses much larger buffer
> than the previous one, but would be a waste if the later one
> needs smaller buffer.

Junio is correct, that xrealloc isn't using the contents of the buffer from the last round, which makes any memcpy it might do internally due to movement to a larger buffer an utter waste.

In this new version we are probably always free'ing a buffer of a much smaller size than we are then later allocating (or in the old version xrealloc'ing to) because we are switching from a delta to full content. Its most likely the delta is way smaller, so I'd guess the malloc implementation is mostly going to another buffer. In short, Marco's change will most likely do better.

But this is all academic wanking. We're talking about this xrealloc (or free/xmalloc pair) happening only when we switch packfiles, which in fast-import is usually every 4 GiB of output. That's a *lot* of data to write. Who cares how many extra microseconds we spend to perform this buffer change; we probably hit it only once every 15-30 minutes, depending on how fast your system is able to transfer 4 GiB of data out of the source and into a packfile.

-- 
Shawn.