Subject: Re: [PATCH] blame.c: prepare_lines should not call xrealloc for every line

From: David Kastrup <[email-masked]>
Date: Tue, 04 Feb 2014 22:09:44 +0100

Junio C Hamano <[email-masked]> writes:

> Junio C Hamano <[email-masked]> writes:
>
>> David Kastrup <[email-masked]> writes:
>>
>>> Whitespace error in line 1778.  Should I be reposting?
>>
>> Heh, let me try to clean it up first and then repost for your
>> review.
>>
>> Thanks.
>
> -- >8 --
> From: David Kastrup <[email-masked]>
>
> Making a single preparation run for counting the lines will avoid memory
> fragmentation.  Also, fix the allocated memory size which was wrong
> when sizeof(int *) != sizeof(int), and would have been too small
> for sizeof(int *) < sizeof(int), admittedly unlikely.
>
> Signed-off-by: David Kastrup <[email-masked]>
> ---
>
>  One logic difference from what was posted is that sb->lineno[num]
>  is filled with the length of the entire buffer when the file ends
>  with a complete line.

Where's the difference?  This is exactly what will happen with my code
as well.  I _do_ rely on memchr(whatever, '\n', 0) to return NULL
without looking at any memory for that.  If there is a fear of memchr
not being able to deal with a count of 0, this code needs to be somewhat
more complex.

>  I do not remember if the rest of the logic
>  actually depends on it (I think I use lineno[n+1] - lineno[n] to
>  find the end of line,

Well, you do it about _half_ the time.  The other half, you scan for the
'\n' explicitly.

>  The original code dates back to 2006 when the author of the code
>  was not paid for doing anything for Git but was doing it as a
>  weekend and evening hobby, so it may not be so surprising to find
>  this kind of "what was I thinking when I wrote it" inefficiency in
>  such a code with $0 monetary value ;-)

Oh, _this_ patch is not in the "I want to make money from it" range.  If
that were the case, I should not have bothered at all.  This is just the
"this code offends my sense of aesthetics" class.  It's purely optional
to apply.  It's conceivable that it will make a performance difference
on non-glibc (or what it's called) platforms.

-- 
David Kastrup