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

2 messages from 2014-02-12 to 2014-02-12. Participants: David Kastrup, Junio C Hamano.
Thread: https://gitlist.dev/t/35851

## David Kastrup, 2014-02-12 14:27

Subject: [PATCH] blame.c: prepare_lines should not call xrealloc for every line
Message-ID: <1392215244-26785-1-git-send-email-dak@gnu.org>
URL: https://gitlist.dev/e/1392215244-26785-1-git-send-email-dak%40gnu.org

```
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@gnu.org>
---

Since there was no feedback after the last defense/explanation of the
coding choices, the code rewritten by this patch was much more awful,
and the kind of style requests (fixed already in the last iteration)
are not actually heeded by the core developers themselves, I have no
idea whether this patch will be dropped just like the last one.

As opposed to the last try, this incorporates a suggestion from Jeff
to change sizeof(type) to sizeof(expression) which is not helping much
since the types of lineno and sb->lineno still need to be changed in
sync.

It also fiddles cosmetically with the code layout of the loops.

builtin/blame.c | 46 +++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index e44a6bb..1aefedf 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1772,25 +1772,41 @@ static int prepare_lines(struct scoreboard *sb)
 {
 	const char *buf = sb->final_buf;
 	unsigned long len = sb->final_buf_size;
-	int num = 0, incomplete = 0, bol = 1;
+	const char *end = buf + len;
+	const char *p;
+	int *lineno;
+	int num = 0, incomplete = 0;
 
-	if (len && buf[len-1] != '\n')
-		incomplete++; /* incomplete line at the end */
-	while (len--) {
-		if (bol) {
-			sb->lineno = xrealloc(sb->lineno,
-					      sizeof(int *) * (num + 1));
-			sb->lineno[num] = buf - sb->final_buf;
-			bol = 0;
-		}
-		if (*buf++ == '\n') {
+	for (p = buf;;) {
+		p = memchr(p, '\n', end - p);
+		if (p) {
+			p++;
 			num++;
-			bol = 1;
+			continue;
 		}
+		break;
 	}
-	sb->lineno = xrealloc(sb->lineno,
-			      sizeof(int *) * (num + incomplete + 1));
-	sb->lineno[num + incomplete] = buf - sb->final_buf;
+
+	if (len && end[-1] != '\n')
+		incomplete++; /* incomplete line at the end */
+
+	sb->lineno = xmalloc(sizeof(*sb->lineno) * (num + incomplete + 1));
+	lineno = sb->lineno;
+
+	*lineno++ = 0;
+	for (p = buf;;) {
+		p = memchr(p, '\n', end - p);
+		if (p) {
+			p++;
+			*lineno++ = p - buf;
+			continue;
+		}
+		break;
+	}
+
+	if (incomplete)
+		*lineno++ = len;
+
 	sb->num_lines = num + incomplete;
 	return sb->num_lines;
 }
-- 
1.8.3.2

```

## Junio C Hamano, 2014-02-12 19:36

Subject: Re: [PATCH] blame.c: prepare_lines should not call xrealloc for every line
Message-ID: <xmqqvbwkm8c3.fsf@gitster.dls.corp.google.com>
URL: https://gitlist.dev/e/xmqqvbwkm8c3.fsf%40gitster.dls.corp.google.com
In-Reply-To: <1392215244-26785-1-git-send-email-dak@gnu.org>

```
David Kastrup <dak@gnu.org> writes:

> 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@gnu.org>
> ---

I think I took sizeof(int*)->sizeof(int) patch to the 'next' branch
already, which might have to conflict with this clean-up, but it
should be trivial to resolve.

Thanks for resending.  I was busy elsewhere (i.e. "no feedback" does
not mean "silent rejection" nor "silent agreement" at least from
me), and such a resend does help prevent patches fall thru cracks.

> diff --git a/builtin/blame.c b/builtin/blame.c
> index e44a6bb..1aefedf 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1772,25 +1772,41 @@ static int prepare_lines(struct scoreboard *sb)
>  {
>  	const char *buf = sb->final_buf;
>  	unsigned long len = sb->final_buf_size;
> +	const char *end = buf + len;
> +	const char *p;
> +	int *lineno;
> +	int num = 0, incomplete = 0;
>  
> +	for (p = buf;;) {
> +		p = memchr(p, '\n', end - p);
> +		if (p) {
> +			p++;
>  			num++;
> +			continue;
>  		}
> +		break;
>  	}
> +
> +	if (len && end[-1] != '\n')
> +		incomplete++; /* incomplete line at the end */
> +
> +	sb->lineno = xmalloc(sizeof(*sb->lineno) * (num + incomplete + 1));
> +	lineno = sb->lineno;
> +
> +	*lineno++ = 0;
> +	for (p = buf;;) {
> +		p = memchr(p, '\n', end - p);
> +		if (p) {
> +			p++;
> +			*lineno++ = p - buf;
> +			continue;
> +		}
> +		break;
> +	}
> +
> +	if (incomplete)
> +		*lineno++ = len;
> +
>  	sb->num_lines = num + incomplete;
>  	return sb->num_lines;
>  }

```