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

From: "Shawn O. Pearce" <[email-masked]>
Date: Sun, 3 Feb 2008 20:48:50 -0500

Junio C Hamano <[email-masked]> wrote:
> Marco Costalba <[email-masked]> 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 <[email-masked]>
> > ---
> >  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.