From 1c57a627bf269f3c83c48ad724cd8b14292502ef Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Fri, 29 Apr 2011 11:36:21 +0200 Subject: [PATCH] New --dirstat=lines mode, doing dirstat analysis based on diffstat This patch adds an alternative implementation of show_dirstat(), called show_dirstat_by_line(), which uses the more expensive diffstat analysis (as opposed to show_dirstat()'s own (relatively inexpensive) analysis) to derive the numbers from which the --dirstat output is computed. The alternative implementation is controlled by the new "lines" parameter to the --dirstat option (or the diff.dirstat config variable). For binary files, the diffstat analysis counts bytes instead of lines, so to prevent binary files from dominating the dirstat results, the byte counts for binary files are divided by 64 before being compared to their textual/line-based counterparts. This is a stupid and ugly - but very cheap - heuristic. In linux-2.6.git, running the three different --dirstat modes: time git diff v2.6.20..v2.6.30 --dirstat=changes > /dev/null vs. time git diff v2.6.20..v2.6.30 --dirstat=lines > /dev/null vs. time git diff v2.6.20..v2.6.30 --dirstat=files > /dev/null yields the following average runtimes on my machine: - "changes" (default): ~6.0 s - "lines": ~9.6 s - "files": ~0.1 s So, as expected, there's a considerable performance hit (~60%) by going through the full diffstat analysis as compared to the default "changes" analysis (obviously, "files" is much faster than both). As such, the "lines" mode is probably only useful if you really need the --dirstat numbers to be consistent with the numbers returned from the other --*stat options. The patch also includes documentation and tests for the new dirstat mode. Improved-by: Junio C Hamano Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano --- Documentation/diff-config.txt | 8 +++ Documentation/diff-options.txt | 8 +++ diff.c | 61 +++++++++++++++++++- diff.h | 1 + t/t4047-diff-dirstat.sh | 100 +++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+), 2 deletions(-) diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt index 228329d4a..1aed79e7d 100644 --- a/Documentation/diff-config.txt +++ b/Documentation/diff-config.txt @@ -23,6 +23,14 @@ diff.dirstat:: the amount of pure code movements within a file. In other words, rearranging lines in a file is not counted as much as other changes. This is the default behavior when no parameter is given. +`lines`;; + Compute the dirstat numbers by doing the regular line-based diff + analysis, and summing the removed/added line counts. (For binary + files, count 64-byte chunks instead, since binary files have no + natural concept of lines). This is a more expensive `--dirstat` + behavior than the `changes` behavior, but it does count rearranged + lines within a file as much as other changes. The resulting output + is consistent with what you get from the other `--*stat` options. `files`;; Compute the dirstat numbers by counting the number of files changed. Each changed file counts equally in the dirstat analysis. This is diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index dc023523e..bddceb06b 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -81,6 +81,14 @@ endif::git-format-patch[] the amount of pure code movements within a file. In other words, rearranging lines in a file is not counted as much as other changes. This is the default behavior when no parameter is given. +`lines`;; + Compute the dirstat numbers by doing the regular line-based diff + analysis, and summing the removed/added line counts. (For binary + files, count 64-byte chunks instead, since binary files have no + natural concept of lines). This is a more expensive `--dirstat` + behavior than the `changes` behavior, but it does count rearranged + lines within a file as much as other changes. The resulting output + is consistent with what you get from the other `--*stat` options. `files`;; Compute the dirstat numbers by counting the number of files changed. Each changed file counts equally in the dirstat analysis. This is diff --git a/diff.c b/diff.c index 70464d9d6..919a16d89 100644 --- a/diff.c +++ b/diff.c @@ -73,9 +73,15 @@ static int parse_dirstat_params(struct diff_options *options, const char *params while (*p) { if (!prefixcmp(p, "changes")) { p += 7; + DIFF_OPT_CLR(options, DIRSTAT_BY_LINE); + DIFF_OPT_CLR(options, DIRSTAT_BY_FILE); + } else if (!prefixcmp(p, "lines")) { + p += 5; + DIFF_OPT_SET(options, DIRSTAT_BY_LINE); DIFF_OPT_CLR(options, DIRSTAT_BY_FILE); } else if (!prefixcmp(p, "files")) { p += 5; + DIFF_OPT_CLR(options, DIRSTAT_BY_LINE); DIFF_OPT_SET(options, DIRSTAT_BY_FILE); } else if (!prefixcmp(p, "noncumulative")) { p += 13; @@ -1671,6 +1677,50 @@ found_damage: gather_dirstat(options, &dir, changed, "", 0); } +static void show_dirstat_by_line(struct diffstat_t *data, struct diff_options *options) +{ + int i; + unsigned long changed; + struct dirstat_dir dir; + + if (data->nr == 0) + return; + + dir.files = NULL; + dir.alloc = 0; + dir.nr = 0; + dir.permille = options->dirstat_permille; + dir.cumulative = DIFF_OPT_TST(options, DIRSTAT_CUMULATIVE); + + changed = 0; + for (i = 0; i < data->nr; i++) { + struct diffstat_file *file = data->files[i]; + unsigned long damage = file->added + file->deleted; + if (file->is_binary) + /* + * binary files counts bytes, not lines. Must find some + * way to normalize binary bytes vs. textual lines. + * The following heuristic assumes that there are 64 + * bytes per "line". + * This is stupid and ugly, but very cheap... + */ + damage = (damage + 63) / 64; + ALLOC_GROW(dir.files, dir.nr + 1, dir.alloc); + dir.files[dir.nr].name = file->name; + dir.files[dir.nr].changed = damage; + changed += damage; + dir.nr++; + } + + /* This can happen even with many files, if everything was renames */ + if (!changed) + return; + + /* Show all directories with more than x% of the changes */ + qsort(dir.files, dir.nr, sizeof(dir.files[0]), dirstat_compare); + gather_dirstat(options, &dir, changed, "", 0); +} + static void free_diffstat_info(struct diffstat_t *diffstat) { int i; @@ -4088,6 +4138,7 @@ void diff_flush(struct diff_options *options) struct diff_queue_struct *q = &diff_queued_diff; int i, output_format = options->output_format; int separator = 0; + int dirstat_by_line = 0; /* * Order: raw, stat, summary, patch @@ -4108,7 +4159,11 @@ void diff_flush(struct diff_options *options) separator++; } - if (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT)) { + if (output_format & DIFF_FORMAT_DIRSTAT && DIFF_OPT_TST(options, DIRSTAT_BY_LINE)) + dirstat_by_line = 1; + + if (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT) || + dirstat_by_line) { struct diffstat_t diffstat; memset(&diffstat, 0, sizeof(struct diffstat_t)); @@ -4123,10 +4178,12 @@ void diff_flush(struct diff_options *options) show_stats(&diffstat, options); if (output_format & DIFF_FORMAT_SHORTSTAT) show_shortstats(&diffstat, options); + if (output_format & DIFF_FORMAT_DIRSTAT) + show_dirstat_by_line(&diffstat, options); free_diffstat_info(&diffstat); separator++; } - if (output_format & DIFF_FORMAT_DIRSTAT) + if ((output_format & DIFF_FORMAT_DIRSTAT) && !dirstat_by_line) show_dirstat(options); if (output_format & DIFF_FORMAT_SUMMARY && !is_summary_empty(q)) { diff --git a/diff.h b/diff.h index 90f491e9e..937a903dc 100644 --- a/diff.h +++ b/diff.h @@ -78,6 +78,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) #define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25) #define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1 << 26) #define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1 << 27) +#define DIFF_OPT_DIRSTAT_BY_LINE (1 << 28) #define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag) #define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag) diff --git a/t/t4047-diff-dirstat.sh b/t/t4047-diff-dirstat.sh index 4b25e1003..b8ad92a35 100755 --- a/t/t4047-diff-dirstat.sh +++ b/t/t4047-diff-dirstat.sh @@ -839,4 +839,104 @@ test_expect_success '--dirstat=files,cumulative,27.09' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +cat <expect_diff_dirstat + 10.6% dst/copy/changed/ + 10.6% dst/copy/rearranged/ + 10.6% dst/copy/unchanged/ + 10.6% dst/move/changed/ + 10.6% dst/move/rearranged/ + 10.6% dst/move/unchanged/ + 10.6% src/move/changed/ + 10.6% src/move/rearranged/ + 10.6% src/move/unchanged/ +EOF + +cat <expect_diff_dirstat_M + 5.2% changed/ + 26.3% dst/copy/changed/ + 26.3% dst/copy/rearranged/ + 26.3% dst/copy/unchanged/ + 5.2% dst/move/changed/ + 5.2% dst/move/rearranged/ + 5.2% rearranged/ +EOF + +cat <expect_diff_dirstat_CC + 16.6% changed/ + 16.6% dst/copy/changed/ + 16.6% dst/copy/rearranged/ + 16.6% dst/move/changed/ + 16.6% dst/move/rearranged/ + 16.6% rearranged/ +EOF + +test_expect_success '--dirstat=lines' ' + git diff --dirstat=lines HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=lines -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=lines -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success 'diff.dirstat=lines' ' + git -c diff.dirstat=lines diff --dirstat HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=lines diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=lines diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +cat <expect_diff_dirstat + 2.1% changed/ + 10.6% dst/copy/changed/ + 10.6% dst/copy/rearranged/ + 10.6% dst/copy/unchanged/ + 10.6% dst/move/changed/ + 10.6% dst/move/rearranged/ + 10.6% dst/move/unchanged/ + 2.1% rearranged/ + 10.6% src/move/changed/ + 10.6% src/move/rearranged/ + 10.6% src/move/unchanged/ +EOF + +cat <expect_diff_dirstat_M + 5.2% changed/ + 26.3% dst/copy/changed/ + 26.3% dst/copy/rearranged/ + 26.3% dst/copy/unchanged/ + 5.2% dst/move/changed/ + 5.2% dst/move/rearranged/ + 5.2% rearranged/ +EOF + +cat <expect_diff_dirstat_CC + 16.6% changed/ + 16.6% dst/copy/changed/ + 16.6% dst/copy/rearranged/ + 16.6% dst/move/changed/ + 16.6% dst/move/rearranged/ + 16.6% rearranged/ +EOF + +test_expect_success '--dirstat=lines,0' ' + git diff --dirstat=lines,0 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=lines,0 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=lines,0 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success 'diff.dirstat=0,lines' ' + git -c diff.dirstat=0,lines diff --dirstat HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=0,lines diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=0,lines diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + test_done -- 2.26.2