Avoid loading commits twice in log with diffs

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@student.ethz.ch> Signed-off-by: Thomas Rast <trast@inf.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Thomas Rast committed Mar 28, 2013 at 09:19 UTC d1b9b76734d2ec31d41ba6e7beddab7c880dff85
1 file changed +10 -3
log-tree.c
+10 -3
@@ -792,11 +792,14 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
792 {
793 int showed_log;
794 struct commit_list *parents;
795 - unsigned const char *sha1 = commit->object.sha1;
795 + unsigned const char *sha1;
796
797 if (!opt->diff && !DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS))
798 return 0;
799
800 + parse_commit(commit);
801 + sha1 = commit->tree->object.sha1;
802 +
803 /* Root commit? */
804 parents = commit->parents;
805 if (!parents) {
@@ -819,7 +822,9 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
822 * parent, showing summary diff of the others
823 * we merged _in_.
824 */
822 - diff_tree_sha1(parents->item->object.sha1, sha1, "", &opt->diffopt);
825 + parse_commit(parents->item);
826 + diff_tree_sha1(parents->item->tree->object.sha1,
827 + sha1, "", &opt->diffopt);
828 log_tree_diff_flush(opt);
829 return !opt->loginfo;
830 }
@@ -832,7 +837,9 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
837 for (;;) {
838 struct commit *parent = parents->item;
839
835 - diff_tree_sha1(parent->object.sha1, sha1, "", &opt->diffopt);
840 + parse_commit(parent);
841 + diff_tree_sha1(parent->tree->object.sha1,
842 + sha1, "", &opt->diffopt);
843 log_tree_diff_flush(opt);
844
845 showed_log |= !opt->loginfo;