* [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; 4+ 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] 4+ 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; 4+ 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] 4+ 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; 4+ 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] 4+ messages in thread
* [PATCH 0/2] Two janitorial patches for builtin/blame.c @ 2014-01-19 18:57 David Kastrup 2014-01-19 18:57 ` [PATCH 2/2] Eliminate same_suspect function in builtin/blame.c David Kastrup 0 siblings, 1 reply; 4+ messages in thread From: David Kastrup @ 2014-01-19 18:57 UTC (permalink / raw) To: git; +Cc: David Kastrup This is more a warmup than anything else: I'm actually doing a quite more involved rewrite of git-blame right now. But it's been a long time since I sent patches for Git, so I'm starting out with something reasonably uncontroversial. Patch 1 is a no-brainer: maintaining reverse links is not worth the trouble if you are not going to use them. Now one can be "conservative" and say "but git-blame is awfully inefficient and maybe we'll need them in a more efficient version". I can answer this with "no": the kind of stuff that would make things more efficient does not require backlinks. Patch 2 is a bit more tricky: basically its contention is that I understand some implications of the code better than its author appeared to do. Which is somewhat optimistic. Since my followup work depends on my understanding this correctly, it's better to make sure of that by handing in a nicely isolated patch for review. It's conceivable that my understanding of the commit->util cache is not fully satisfactory. I don't use it in my followup work anyway, but it still would be nice to get this code path cleaned out in advance. I don't expect measurable performance improvements from these two patches: their main purpose is to get some cruft out of the way so that the heavy-duty patches actually dealing with the performance sinks will be a bit more focused. And of course, getting the ball rolling again. 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] 4+ messages in thread
* [PATCH 2/2] Eliminate same_suspect function in builtin/blame.c 2014-01-19 18:57 [PATCH 0/2] Two janitorial patches for builtin/blame.c David Kastrup @ 2014-01-19 18:57 ` David Kastrup 0 siblings, 0 replies; 4+ messages in thread From: David Kastrup @ 2014-01-19 18:57 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. --- 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] 4+ messages in thread
end of thread, other threads:[~2014-01-22 0:20 UTC | newest] Thread overview: 4+ 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 2/2] Eliminate same_suspect function in builtin/blame.c 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).