[PATCH v2] Avoid loading commits twice in log with diffs

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

 



If you run a log with diffs (such as -p, --raw, --stat etc.) the
current code ends up loading many objects twice.  For example, for
'log -3000 -p' my instrumentation said the objects loaded more than
once are distributed as follows:

  2008 blob
  2103 commit
  2678 tree

Fixing blobs and trees will be harder, because those are really used
within the diff engine and need some form of caching.

However, fixing the commits is easy at least at the band-aid level.
They are triggered by log_tree_diff() invoking diff_tree_sha1() on
commits, which duly loads the specified object to dereference it to a
tree.  Since log_tree_diff() knows that it works with commits and they
must have trees, we can simply pass through the trees.

We add some parse_commit() calls.  The ones for the parents are
required; we do not know at this stage if they have been looked at.
The one for the commit itself is pure paranoia, but has about the same
cost as an assertion on commit->object.parsed.

This has a quite dramatic effect on log --raw, though only a
negligible impact on log -p:

Test                      this tree         HEAD
--------------------------------------------------------------------
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

Signed-off-by: Thomas Rast <trast@xxxxxxxxxxxxxxx>
Signed-off-by: Thomas Rast <trast@xxxxxxxxxxx>
---

Adjusted for the concern that the commit might not be parsed yet.  I
think it's now more paranoid than the original code, since we cannot
look at commit->parents without parsing.  But it's really an almost
free check.


 log-tree.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 5dc45c4..8a34332 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -792,11 +792,14 @@ 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;
 
 	if (!opt->diff && !DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS))
 		return 0;
 
+	parse_commit(commit);
+	sha1 = commit->tree->object.sha1;
+
 	/* Root commit? */
 	parents = commit->parents;
 	if (!parents) {
@@ -819,7 +822,9 @@ 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;
 		}
@@ -832,7 +837,9 @@ 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;
-- 
1.8.2.351.g867a5da

--
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]