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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> David Kastrup <dak@xxxxxxx> 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 <dak@xxxxxxx>
>
> 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 <dak@xxxxxxx>
> ---
>
>  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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]