Re: [PATCH] Avoid loading commits twice in log with diffs
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:17
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH] Avoid loading commits twice in log with diffs · Thomas Rast <hidden>
- 2016-06-15 · [PATCH] Avoid loading commits twice in log with diffs · Thomas Rast <hidden>
Thomas Rast [off-list ref] writes:
Test with patch before -------------------------------------------------------------------- 4000.2: log --raw -3000 0.50(0.43+0.06) 0.54(0.46+0.06) +7.0%*** 4000.3: log -p -3000 2.34(2.20+0.13) 2.37(2.22+0.13) +1.2% -------------------------------------------------------------------- Significance hints: '.' 0.1 '*' 0.05 '**' 0.01 '***' 0.001
It may be a silly question but what is a significance hint?
quoted hunk
Signed-off-by: Thomas Rast <redacted> --- log-tree.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)diff --git a/log-tree.c b/log-tree.c index eb1a1b4..277a38f 100644 --- a/log-tree.c +++ b/log-tree.c@@ -715,7 +715,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log { int showed_log; struct commit_list *parents; - unsigned const char *sha1 = commit->object.sha1; + unsigned const char *sha1 = commit->tree->object.sha1;
Overall I think this goes in the right direction and I can see why the changes in later two hunks are correct, but I am not sure if we can safely assume that the caller has parsed the incoming commit and we have a good commit->tree here already. Right now, this static function has a single call-site in a public function log_tree_commit(), whose existing callers may all pass an already parsed commit, but I feel somewhat uneasy to do the above without some mechanism in place (either parse it here or in the caller if unparsed, or document that log_tree_commit() must be called with a parsed commit and perhaps add an assert there) to ensure that the invariant is not broken in the future. Thanks.
quoted hunk
if (!opt->diff && !DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)) return 0;@@ -742,7 +742,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log * parent, showing summary diff of the others * we merged _in_. */ - diff_tree_sha1(parents->item->object.sha1, sha1, "", &opt->diffopt); + parse_commit(parents->item); + diff_tree_sha1(parents->item->tree->object.sha1, sha1, "", &opt->diffopt); log_tree_diff_flush(opt); return !opt->loginfo; }@@ -755,7 +756,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log for (;;) { struct commit *parent = parents->item; - diff_tree_sha1(parent->object.sha1, sha1, "", &opt->diffopt); + parse_commit(parent); + diff_tree_sha1(parent->tree->object.sha1, sha1, "", &opt->diffopt); log_tree_diff_flush(opt); showed_log |= !opt->loginfo;