* [PATCH 1/2] builtin/blame.c: struct blame_entry does not need a prev link
2014-01-19 18:57 [PATCH 0/2] Two janitorial patches for builtin/blame.c David Kastrup
@ 2014-01-19 18:57 ` David Kastrup
2014-01-21 21:54 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: David Kastrup @ 2014-01-19 18:57 UTC (permalink / raw)
To: git; +Cc: David Kastrup
---
builtin/blame.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index e44a6bb..2195595 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -197,7 +197,6 @@ static void drop_origin_blob(struct origin *o)
* scoreboard structure, sorted by the target line number.
*/
struct blame_entry {
- struct blame_entry *prev;
struct blame_entry *next;
/* the first line of this group in the final image;
@@ -282,8 +281,6 @@ static void coalesce(struct scoreboard *sb)
ent->s_lno + ent->num_lines == next->s_lno) {
ent->num_lines += next->num_lines;
ent->next = next->next;
- if (ent->next)
- ent->next->prev = ent;
origin_decref(next->suspect);
free(next);
ent->score = 0;
@@ -534,7 +531,7 @@ static void add_blame_entry(struct scoreboard *sb, struct blame_entry *e)
prev = ent;
/* prev, if not NULL, is the last one that is below e */
- e->prev = prev;
+
if (prev) {
e->next = prev->next;
prev->next = e;
@@ -543,8 +540,6 @@ static void add_blame_entry(struct scoreboard *sb, struct blame_entry *e)
e->next = sb->ent;
sb->ent = e;
}
- if (e->next)
- e->next->prev = e;
}
/*
@@ -555,14 +550,12 @@ static void add_blame_entry(struct scoreboard *sb, struct blame_entry *e)
*/
static void dup_entry(struct blame_entry *dst, struct blame_entry *src)
{
- struct blame_entry *p, *n;
+ struct blame_entry *n;
- p = dst->prev;
n = dst->next;
origin_incref(src->suspect);
origin_decref(dst->suspect);
memcpy(dst, src, sizeof(*src));
- dst->prev = p;
dst->next = n;
dst->score = 0;
}
@@ -2502,8 +2495,6 @@ parse_done:
ent->suspect = o;
ent->s_lno = bottom;
ent->next = next;
- if (next)
- next->prev = ent;
origin_incref(o);
}
origin_decref(o);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] builtin/blame.c: struct blame_entry does not need a prev link
2014-01-19 18:57 ` [PATCH 1/2] builtin/blame.c: struct blame_entry does not need a prev link David Kastrup
@ 2014-01-21 21:54 ` Junio C Hamano
2014-01-21 23:02 ` David Kastrup
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2014-01-21 21:54 UTC (permalink / raw)
To: David Kastrup; +Cc: git
David Kastrup <dak@gnu.org> writes:
> ---
Thanks. At some point during its development I must have thought
that having it as a dual-linked list may make it easier when we have
to split a block into pieces, but it seems that split_overlap() does
not need to look at this information.
Needs sign-off.
> builtin/blame.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index e44a6bb..2195595 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -197,7 +197,6 @@ static void drop_origin_blob(struct origin *o)
> * scoreboard structure, sorted by the target line number.
> */
> struct blame_entry {
> - struct blame_entry *prev;
> struct blame_entry *next;
>
> /* the first line of this group in the final image;
> @@ -282,8 +281,6 @@ static void coalesce(struct scoreboard *sb)
> ent->s_lno + ent->num_lines == next->s_lno) {
> ent->num_lines += next->num_lines;
> ent->next = next->next;
> - if (ent->next)
> - ent->next->prev = ent;
> origin_decref(next->suspect);
> free(next);
> ent->score = 0;
> @@ -534,7 +531,7 @@ static void add_blame_entry(struct scoreboard *sb, struct blame_entry *e)
> prev = ent;
>
> /* prev, if not NULL, is the last one that is below e */
> - e->prev = prev;
> +
> if (prev) {
> e->next = prev->next;
> prev->next = e;
> @@ -543,8 +540,6 @@ static void add_blame_entry(struct scoreboard *sb, struct blame_entry *e)
> e->next = sb->ent;
> sb->ent = e;
> }
> - if (e->next)
> - e->next->prev = e;
> }
>
> /*
> @@ -555,14 +550,12 @@ static void add_blame_entry(struct scoreboard *sb, struct blame_entry *e)
> */
> static void dup_entry(struct blame_entry *dst, struct blame_entry *src)
> {
> - struct blame_entry *p, *n;
> + struct blame_entry *n;
>
> - p = dst->prev;
> n = dst->next;
> origin_incref(src->suspect);
> origin_decref(dst->suspect);
> memcpy(dst, src, sizeof(*src));
> - dst->prev = p;
> dst->next = n;
> dst->score = 0;
> }
> @@ -2502,8 +2495,6 @@ parse_done:
> ent->suspect = o;
> ent->s_lno = bottom;
> ent->next = next;
> - if (next)
> - next->prev = ent;
> origin_incref(o);
> }
> origin_decref(o);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] builtin/blame.c: struct blame_entry does not need a prev link
2014-01-21 21:54 ` Junio C Hamano
@ 2014-01-21 23:02 ` David Kastrup
0 siblings, 0 replies; 6+ messages in thread
From: David Kastrup @ 2014-01-21 23:02 UTC (permalink / raw)
To: git
Junio C Hamano <gitster@pobox.com> writes:
> David Kastrup <dak@gnu.org> writes:
>
>> ---
>
> Thanks. At some point during its development I must have thought
> that having it as a dual-linked list may make it easier when we have
> to split a block into pieces, but it seems that split_overlap() does
> not need to look at this information.
>
> Needs sign-off.
Well, as I said: it's quite possible that the double-linking might be
useful for some particular hypothetical rewrite of the code. It isn't
for the current code, and it's not useful for my own rewrite.
Will be posting a signed-off version presently.
--
David Kastrup
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 0/2] Two mostly janitorial patches for builtin/blame.c
@ 2014-01-22 0:20 David Kastrup
2014-01-22 0:20 ` [PATCH 1/2] builtin/blame.c: struct blame_entry does not need a prev link David Kastrup
2014-01-22 0:20 ` [PATCH 2/2] Eliminate same_suspect function in builtin/blame.c David Kastrup
0 siblings, 2 replies; 6+ messages in thread
From: David Kastrup @ 2014-01-22 0:20 UTC (permalink / raw)
To: git; +Cc: David Kastrup
Same series as sent previously, just signed off this time.
David Kastrup (2):
builtin/blame.c: struct blame_entry does not need a prev link
Eliminate same_suspect function in builtin/blame.c
builtin/blame.c | 38 ++++++++++----------------------------
1 file changed, 10 insertions(+), 28 deletions(-)
--
1.8.3.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] builtin/blame.c: struct blame_entry does not need a prev link
2014-01-22 0:20 [PATCH 0/2] Two mostly janitorial patches for builtin/blame.c David Kastrup
@ 2014-01-22 0:20 ` David Kastrup
2014-01-22 0:20 ` [PATCH 2/2] Eliminate same_suspect function in builtin/blame.c David Kastrup
1 sibling, 0 replies; 6+ messages in thread
From: David Kastrup @ 2014-01-22 0:20 UTC (permalink / raw)
To: git; +Cc: David Kastrup
Signed-off-by: David Kastrup <dak@gnu.org>
---
builtin/blame.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index e44a6bb..2195595 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -197,7 +197,6 @@ static void drop_origin_blob(struct origin *o)
* scoreboard structure, sorted by the target line number.
*/
struct blame_entry {
- struct blame_entry *prev;
struct blame_entry *next;
/* the first line of this group in the final image;
@@ -282,8 +281,6 @@ static void coalesce(struct scoreboard *sb)
ent->s_lno + ent->num_lines == next->s_lno) {
ent->num_lines += next->num_lines;
ent->next = next->next;
- if (ent->next)
- ent->next->prev = ent;
origin_decref(next->suspect);
free(next);
ent->score = 0;
@@ -534,7 +531,7 @@ static void add_blame_entry(struct scoreboard *sb, struct blame_entry *e)
prev = ent;
/* prev, if not NULL, is the last one that is below e */
- e->prev = prev;
+
if (prev) {
e->next = prev->next;
prev->next = e;
@@ -543,8 +540,6 @@ static void add_blame_entry(struct scoreboard *sb, struct blame_entry *e)
e->next = sb->ent;
sb->ent = e;
}
- if (e->next)
- e->next->prev = e;
}
/*
@@ -555,14 +550,12 @@ static void add_blame_entry(struct scoreboard *sb, struct blame_entry *e)
*/
static void dup_entry(struct blame_entry *dst, struct blame_entry *src)
{
- struct blame_entry *p, *n;
+ struct blame_entry *n;
- p = dst->prev;
n = dst->next;
origin_incref(src->suspect);
origin_decref(dst->suspect);
memcpy(dst, src, sizeof(*src));
- dst->prev = p;
dst->next = n;
dst->score = 0;
}
@@ -2502,8 +2495,6 @@ parse_done:
ent->suspect = o;
ent->s_lno = bottom;
ent->next = next;
- if (next)
- next->prev = ent;
origin_incref(o);
}
origin_decref(o);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] Eliminate same_suspect function in builtin/blame.c
2014-01-22 0:20 [PATCH 0/2] Two mostly janitorial patches for builtin/blame.c David Kastrup
2014-01-22 0:20 ` [PATCH 1/2] builtin/blame.c: struct blame_entry does not need a prev link David Kastrup
@ 2014-01-22 0:20 ` David Kastrup
1 sibling, 0 replies; 6+ messages in thread
From: David Kastrup @ 2014-01-22 0:20 UTC (permalink / raw)
To: git; +Cc: David Kastrup
Since the origin pointers are "interned" and reference-counted, comparing
the pointers rather than the content is enough. The only uninterned
origins are cached values kept in commit->util, but same_suspect is not
called on them.
Signed-off-by: David Kastrup <dak@gnu.org>
---
builtin/blame.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 2195595..ead6148 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -255,15 +255,6 @@ struct scoreboard {
int *lineno;
};
-static inline int same_suspect(struct origin *a, struct origin *b)
-{
- if (a == b)
- return 1;
- if (a->commit != b->commit)
- return 0;
- return !strcmp(a->path, b->path);
-}
-
static void sanity_check_refcnt(struct scoreboard *);
/*
@@ -276,7 +267,7 @@ static void coalesce(struct scoreboard *sb)
struct blame_entry *ent, *next;
for (ent = sb->ent; ent && (next = ent->next); ent = next) {
- if (same_suspect(ent->suspect, next->suspect) &&
+ if (ent->suspect == next->suspect &&
ent->guilty == next->guilty &&
ent->s_lno + ent->num_lines == next->s_lno) {
ent->num_lines += next->num_lines;
@@ -735,7 +726,7 @@ static int find_last_in_target(struct scoreboard *sb, struct origin *target)
int last_in_target = -1;
for (e = sb->ent; e; e = e->next) {
- if (e->guilty || !same_suspect(e->suspect, target))
+ if (e->guilty || e->suspect != target)
continue;
if (last_in_target < e->s_lno + e->num_lines)
last_in_target = e->s_lno + e->num_lines;
@@ -755,7 +746,7 @@ static void blame_chunk(struct scoreboard *sb,
struct blame_entry *e;
for (e = sb->ent; e; e = e->next) {
- if (e->guilty || !same_suspect(e->suspect, target))
+ if (e->guilty || e->suspect != target)
continue;
if (same <= e->s_lno)
continue;
@@ -985,7 +976,7 @@ static int find_move_in_parent(struct scoreboard *sb,
while (made_progress) {
made_progress = 0;
for (e = sb->ent; e; e = e->next) {
- if (e->guilty || !same_suspect(e->suspect, target) ||
+ if (e->guilty || e->suspect != target ||
ent_score(sb, e) < blame_move_score)
continue;
find_copy_in_blob(sb, e, parent, split, &file_p);
@@ -1020,14 +1011,14 @@ static struct blame_list *setup_blame_list(struct scoreboard *sb,
for (e = sb->ent, num_ents = 0; e; e = e->next)
if (!e->scanned && !e->guilty &&
- same_suspect(e->suspect, target) &&
+ e->suspect == target &&
min_score < ent_score(sb, e))
num_ents++;
if (num_ents) {
blame_list = xcalloc(num_ents, sizeof(struct blame_list));
for (e = sb->ent, i = 0; e; e = e->next)
if (!e->scanned && !e->guilty &&
- same_suspect(e->suspect, target) &&
+ e->suspect == target &&
min_score < ent_score(sb, e))
blame_list[i++].ent = e;
}
@@ -1171,7 +1162,7 @@ static void pass_whole_blame(struct scoreboard *sb,
origin->file.ptr = NULL;
}
for (e = sb->ent; e; e = e->next) {
- if (!same_suspect(e->suspect, origin))
+ if (e->suspect != origin)
continue;
origin_incref(porigin);
origin_decref(e->suspect);
@@ -1560,7 +1551,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
/* Take responsibility for the remaining entries */
for (ent = sb->ent; ent; ent = ent->next)
- if (same_suspect(ent->suspect, suspect))
+ if (ent->suspect == suspect)
found_guilty_entry(ent);
origin_decref(suspect);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-01-22 0:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-22 0:20 [PATCH 0/2] Two mostly janitorial patches for builtin/blame.c David Kastrup
2014-01-22 0:20 ` [PATCH 1/2] builtin/blame.c: struct blame_entry does not need a prev link David Kastrup
2014-01-22 0:20 ` [PATCH 2/2] Eliminate same_suspect function in builtin/blame.c David Kastrup
-- strict thread matches above, loose matches on Subject: below --
2014-01-19 18:57 [PATCH 0/2] Two janitorial patches for builtin/blame.c David Kastrup
2014-01-19 18:57 ` [PATCH 1/2] builtin/blame.c: struct blame_entry does not need a prev link David Kastrup
2014-01-21 21:54 ` Junio C Hamano
2014-01-21 23:02 ` David Kastrup
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).