{"thread":{"id":"12062","subject":"[PATCH updated] Add \"--dirstat\" for some directory statistics","startedAt":"2008-02-12T21:26:31Z","lastAt":"2008-02-13T14:07:52Z","messageCount":8,"participants":["Linus Torvalds","Junio C Hamano","Abdelrazak Younes","Johannes Schindelin","Wincent Colaiuta"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"68551","messageId":"alpine.LFD.1.00.0802121308360.2920@woody.linux-foundation.org","threadId":"12062","inReplyTo":null,"subject":"[PATCH updated] Add \"--dirstat\" for some directory statistics","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-12T21:26:31Z","receivedAt":"2008-02-12T21:26:31Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nThis adds a new form of overview diffstat output, doing something that I \nhave occasionally ended up doing manually (and badly, because it's \nactually pretty nasty to do), and that I think is very useful for an \nproject like the kernel that has a fairly deep and well-separated \ndirectory structure with semantic meaning.\n\nWhat I mean by that is that it's often interesting to see exactly which \nsub-directories are impacted by a patch, and to what degree - even if you \ndon't perhaps care so much about the individual files themselves.\n\nWhat makes the concept more interesting is that the \"impact\" is often \nhierarchical: in the kernel, for example, something could either have a \nvery localized impact to \"fs/ext3/\" and then it's interesting to see that \nsuch a patch changes mostly that subdirectory, but you could have another \npatch that changes some generic VFS-layer issue which affects _many_ \nsubdirectories that are all under \"fs/\", but none - or perhaps just a \ncouple of them - of the individual filesystems are interesting in \nthemselves.\n\nSo what commonly happens is that you may have big changes in a specific\nsub-subdirectory, but still also significant separate changes to the\nsubdirectory leading up to that - maybe you have significant VFS-level\nchanges, but *also* changes under that VFS layer in the NFS-specific\ndirectories, for example. In that case, you do want the low-level parts\nthat are significant to show up, but then the insignificant ones should\nshow up as under the more generic top-level directory.\n\nThis patch shows all of that with \"--dirstat\". The output can be either\nsomething simple like\n\n commit 81772fe...\n Author: Thomas Gleixner \n Date: Sun Feb 10 23:57:36 2008 +0100\n\n x86: remove over noisy debug printk\n\n pageattr-test.c contains a noisy debug printk that people reported.\n The condition under which it prints (randomly tapping into a mem_map[]\n hole and not being able to c_p_a() there) is valid behavior and not\n interesting to report.\n\n Remove it.\n\n Signed-off-by: Thomas Gleixner \n Acked-by: Ingo Molnar \n Signed-off-by: Linus Torvalds \n\n 100.0% arch/x86/mm/\n\nor something much more complex like\n\n commit e231c2e...\n Author: David Howells \n Date: Thu Feb 7 00:15:26 2008 -0800\n\n Convert ERR_PTR(PTR_ERR(p)) instances to ERR_CAST(p)\n\n\t 20.5% crypto/\n\t 7.6% fs/afs/\n\t 7.6% fs/fuse/\n\t 7.6% fs/gfs2/\n\t 5.1% fs/jffs2/\n\t 5.1% fs/nfs/\n\t 5.1% fs/nfsd/\n\t 7.6% fs/reiserfs/\n\t 15.3% fs/\n\t 7.6% net/rxrpc/\n\t 10.2% security/keys/\n\nwhere that latter example is an example of significant work in some \nindividual fs/*/ subdirectories (like the patches to reiserfs accounting \nfor 7.6% of the whole), but then discounting those individual filesystems, \nthere's also 15.3% other \"random\" things that weren't worth reporting on \ntheir oen left over under fs/ in general (either in that directory itself, \nor in subdirectories of fs/ that didn't have enough changes to be reported \nindividually).\n\nI'd like to stress that the \"15.3% fs/\" mentioned above is the stuff that \nis under fs/ but that was _not_ significant enough to report on its own. \nSo the above does _not_ mean that 15.3% of the work was under fs/ per se, \nbecause that 15.3% does *not* include the already-reported 7.6% of afs, \n7.6% of fuse etc.\n\nIf you want to enable \"cumulative\" directory statistics, you can use the\n\"--cumulative\" flag, which adds up percentages recursively even when\nthey have been already reported for a sub-directory. That cumulative\noutput is disabled if *all* of the changes in one subdirectory come from\na deeper subdirectory, to avoid repeating subdirectories all the way to\nthe root.\n\nFor an example of the cumulative reporting, the above commit becomes\n\n\tcommit e231c2e...\n\tAuthor: David Howells \n\tDate: Thu Feb 7 00:15:26 2008 -0800\n\n\t Convert ERR_PTR(PTR_ERR(p)) instances to ERR_CAST(p)\n\n\t 20.5% crypto/\n\t 7.6% fs/afs/\n\t 7.6% fs/fuse/\n\t 7.6% fs/gfs2/\n\t 5.1% fs/jffs2/\n\t 5.1% fs/nfs/\n\t 5.1% fs/nfsd/\n\t 7.6% fs/reiserfs/\n\t 61.5% fs/\n\t 7.6% net/rxrpc/\n\t 10.2% security/keys/\n\nin which the commit percentages now obviously add up to much more than \n100%: now the changes that were already reported for the sub-directories \nunder fs/ are then cumulatively included in the whole percentage of fs/ \n(ie now shows 61.5% as opposed to the 15.3% without the cumulative \nreporting).\n\nThe default reporting limit has been arbitrarily set at 3%, which seems\nto be a pretty good cut-off, but you can specify the cut-off manually by\ngiving it as an option parameter (eg \"--dirstat=5\" makes the cut-off be\nat 5% instead)\n\nNOTE! The percentages are purely about the total lines added and removed,\nnot anything smarter (or dumber) than that. Also note that you should not\ngenerally expect things to add up to 100%: not only does it round down, we\ndon't report leftover scraps (they add up to the top-level change count,\nbut we don't even bother reporting that, it only reports subdirectories).\n\nQuite frankly, as a top-level manager this is really convenient for me,\nbut it's going to be very boring for git itself since there are few\nsubdirectories. Also, don't expect things to make tons of sense if you\ncombine this with \"-M\" and there are cross-directory renames etc.\n\nBut even for git itself, you can get some fun statistics. Try out\n\n git log --dirstat\n\nand see the occasional mentions of things like Documentation/, git-gui/,\ngitweb/ and gitk-git/. Or try out something like\n\n git diff --dirstat v1.5.0..v1.5.4\n\nwhich does kind of git an overview that shows *something*. But in general,\nthe output is more exciting for big projects with deeper structure, and\ndoing a\n\n git diff --dirstat v2.6.24..v2.6.25-rc1\n\non the kernel is what I actually wrote this for!\n\nSigned-off-by: Linus Torvalds \n---\n\nThis is a re-send of all my changes rolled up into one single patch. One \nof the reasons is that the \"report percentages in tenths of percent\" \nsimply changed the examples in the original commit message. Another is \nthat I actually made the \"--cumulative\" flag much more useful in practice \nby making it avoid reporting the subdirectories where all the percentages \ncame from just a single already-reported sub-subdirectory.\n\nThe --cumulative thing still has the odd property of having all the \npercentages add up to more than 100%, but quite often it is now what you \nreally want, and you can, for example, get output like this:\n\n\t 27.5% security/selinux/include/\n\t 100.0% security/selinux/\n\nwhich means that everything is in \"security/selinux\", but within that \ndirectory we also show that 27.5% was within the include subdirectory. \nNote how it now no longer gives a \"100.0% security/\" report, since that \nwould be purely redundant information.\n\nAnother example of this new semantic:\n\n\t 23.8% include/asm-sparc/\n\t 76.1% include/asm-sparc64/\n\t 100.0% include/\n\nhere, \"100% include/\" _is_ reported, because while the 100% did come from \nthe already reported subdirectories, it didn't come from a _single_ one, \nso it basically means that you don't ever have to mentally add up things, \nand we only avoid reporting upper-level directory statistics when there \nreally is no point to it.\n\nWith this change, \"--cumulative\" suddenly makes tons of sense to me. For \nexample, for the 2.6.24..2.6.25-rc1 changes in the kernel (with a 2% \ncutoff), it now gives:\n\n - \"git diff --dirstat=2 --cumulative v2.6.24..v2.6.25-rc1\" output:\n\n\t 2.4% Documentation/\n\t 3.2% arch/arm/\n\t 2.0% arch/cris/\n\t 2.3% arch/powerpc/boot/\n\t 2.3% arch/powerpc/configs/\n\t 6.9% arch/powerpc/\n\t 3.2% arch/sh/\n\t 2.3% arch/x86/kernel/\n\t 6.9% arch/x86/\n\t 31.3% arch/\n\t 2.0% drivers/char/\n\t 2.0% drivers/infiniband/\n\t 2.9% drivers/media/video/\n\t 3.7% drivers/media/\n\t 2.4% drivers/net/wireless/iwlwifi/\n\t 7.3% drivers/net/wireless/\n\t 14.3% drivers/net/\n\t 2.8% drivers/scsi/\n\t 37.2% drivers/\n\t 3.5% fs/\n\t 2.0% include/asm-x86/\n\t 10.3% include/\n\t 6.0% net/\n\t 3.7% sound/\n\nand now you can see at a glance that 31.3 was in arch/, and 37.2% was in \ndrivers/. So this is a case where --cumulative actually makes sense (but \nit probably still doesn't make sense if you do a \"git log --dirstat\" and \nlook at just individual commits that tend to be more about a single \nchange).\n\nAnyway, it's a bit larger now, but my explanations and blathering is\nstill much bigger than the patch itself ;)\n\n diff.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n diff.h | 3 ++\n 2 files changed, 92 insertions(+), 1 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex cd8bc4d..ad0ff28 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -990,6 +990,85 @@ static void show_numstat(struct diffstat_t* data, struct diff_options *options)\n \t}\n }\n \n+struct diffstat_dir {\n+\tstruct diffstat_file **files;\n+\tint nr, percent, cumulative;\n+};\n+\n+static long gather_dirstat(struct diffstat_dir *dir, unsigned long changed, const char *base, int baselen)\n+{\n+\tunsigned long this_dir = 0;\n+\tunsigned int sources = 0;\n+\n+\twhile (dir->nr) {\n+\t\tstruct diffstat_file *f = *dir->files;\n+\t\tint namelen = strlen(f->name);\n+\t\tunsigned long this;\n+\t\tchar *slash;\n+\n+\t\tif (namelen < baselen)\n+\t\t\tbreak;\n+\t\tif (memcmp(f->name, base, baselen))\n+\t\t\tbreak;\n+\t\tslash = strchr(f->name + baselen, '/');\n+\t\tif (slash) {\n+\t\t\tint newbaselen = slash + 1 - f->name;\n+\t\t\tthis = gather_dirstat(dir, changed, f->name, newbaselen);\n+\t\t\tsources++;\n+\t\t} else {\n+\t\t\tthis = f->added + f->deleted;\n+\t\t\tdir->files++;\n+\t\t\tdir->nr--;\n+\t\t\tsources += 2;\n+\t\t}\n+\t\tthis_dir += this;\n+\t}\n+\n+\t/*\n+\t * We don't report dirstat's for \n+\t * - the top level\n+\t * - or cases where everything came from a single directory\n+\t * under this directory (sources == 1).\n+\t */\n+\tif (baselen && sources != 1) {\n+\t\tint permille = this_dir * 1000 / changed;\n+\t\tif (permille) {\n+\t\t\tint percent = permille / 10;\n+\t\t\tif (percent >= dir->percent) {\n+\t\t\t\tprintf(\"%4d.%01d%% %.*s\\n\", percent, permille % 10, baselen, base);\n+\t\t\t\tif (!dir->cumulative)\n+\t\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn this_dir;\n+}\n+\n+static void show_dirstat(struct diffstat_t *data, struct diff_options *options)\n+{\n+\tint i;\n+\tunsigned long changed;\n+\tstruct diffstat_dir dir;\n+\n+\t/* Calculate total changes */\n+\tchanged = 0;\n+\tfor (i = 0; i < data->nr; i++) {\n+\t\tchanged += data->files[i]->added;\n+\t\tchanged += data->files[i]->deleted;\n+\t}\n+\n+\t/* This can happen even with many files, if everything was renames */\n+\tif (!changed)\n+\t\treturn;\n+\n+\t/* Show all directories with more than x% of the changes */\n+\tdir.files = data->files;\n+\tdir.nr = data->nr;\n+\tdir.percent = options->dirstat_percent;\n+\tdir.cumulative = options->output_format & DIFF_FORMAT_CUMULATIVE;\n+\tgather_dirstat(&dir, changed, \"\", 0);\n+}\n+\n static void free_diffstat_info(struct diffstat_t *diffstat)\n {\n \tint i;\n@@ -2058,6 +2137,7 @@ void diff_setup(struct diff_options *options)\n \toptions->line_termination = '\\n';\n \toptions->break_opt = -1;\n \toptions->rename_limit = -1;\n+\toptions->dirstat_percent = 3;\n \toptions->context = 3;\n \toptions->msg_sep = \"\";\n \n@@ -2099,6 +2179,7 @@ int diff_setup_done(struct diff_options *options)\n \t\t\t\t\t DIFF_FORMAT_NUMSTAT |\n \t\t\t\t\t DIFF_FORMAT_DIFFSTAT |\n \t\t\t\t\t DIFF_FORMAT_SHORTSTAT |\n+\t\t\t\t\t DIFF_FORMAT_DIRSTAT |\n \t\t\t\t\t DIFF_FORMAT_SUMMARY |\n \t\t\t\t\t DIFF_FORMAT_PATCH);\n \n@@ -2110,6 +2191,7 @@ int diff_setup_done(struct diff_options *options)\n \t\t\t\t DIFF_FORMAT_NUMSTAT |\n \t\t\t\t DIFF_FORMAT_DIFFSTAT |\n \t\t\t\t DIFF_FORMAT_SHORTSTAT |\n+\t\t\t\t DIFF_FORMAT_DIRSTAT |\n \t\t\t\t DIFF_FORMAT_SUMMARY |\n \t\t\t\t DIFF_FORMAT_CHECKDIFF))\n \t\tDIFF_OPT_SET(options, RECURSIVE);\n@@ -2220,6 +2302,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\toptions->output_format |= DIFF_FORMAT_NUMSTAT;\n \telse if (!strcmp(arg, \"--shortstat\"))\n \t\toptions->output_format |= DIFF_FORMAT_SHORTSTAT;\n+\telse if (opt_arg(arg, 'X', \"dirstat\", &options->dirstat_percent))\n+\t\toptions->output_format |= DIFF_FORMAT_DIRSTAT;\n+\telse if (!strcmp(arg, \"--cumulative\"))\n+\t\toptions->output_format |= DIFF_FORMAT_CUMULATIVE;\n \telse if (!strcmp(arg, \"--check\"))\n \t\toptions->output_format |= DIFF_FORMAT_CHECKDIFF;\n \telse if (!strcmp(arg, \"--summary\"))\n@@ -2938,7 +3024,7 @@ void diff_flush(struct diff_options *options)\n \t\tseparator++;\n \t}\n \n-\tif (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT)) {\n+\tif (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT|DIFF_FORMAT_DIRSTAT)) {\n \t\tstruct diffstat_t diffstat;\n \n \t\tmemset(&diffstat, 0, sizeof(struct diffstat_t));\n@@ -2948,6 +3034,8 @@ void diff_flush(struct diff_options *options)\n \t\t\tif (check_pair_status(p))\n \t\t\t\tdiff_flush_stat(p, options, &diffstat);\n \t\t}\n+\t\tif (output_format & DIFF_FORMAT_DIRSTAT)\n+\t\t\tshow_dirstat(&diffstat, options);\n \t\tif (output_format & DIFF_FORMAT_NUMSTAT)\n \t\t\tshow_numstat(&diffstat, options);\n \t\tif (output_format & DIFF_FORMAT_DIFFSTAT)\ndiff --git a/diff.h b/diff.h\nindex 073d5cb..8c6bb54 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -30,6 +30,8 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,\n #define DIFF_FORMAT_SUMMARY\t0x0008\n #define DIFF_FORMAT_PATCH\t0x0010\n #define DIFF_FORMAT_SHORTSTAT\t0x0020\n+#define DIFF_FORMAT_DIRSTAT\t0x0040\n+#define DIFF_FORMAT_CUMULATIVE\t0x0080\n \n /* These override all above */\n #define DIFF_FORMAT_NAME\t0x0100\n@@ -80,6 +82,7 @@ struct diff_options {\n \tint pickaxe_opts;\n \tint rename_score;\n \tint rename_limit;\n+\tint dirstat_percent;\n \tint setup;\n \tint abbrev;\n \tconst char *msg_sep;\n"},{"id":"68558","messageId":"alpine.LFD.1.00.0802121346560.2920@woody.linux-foundation.org","threadId":"12062","inReplyTo":"alpine.LFD.1.00.0802121308360.2920@woody.linux-foundation.org","subject":"Re: [PATCH updated] Add \"--dirstat\" for some directory statistics","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-12T21:56:13Z","receivedAt":"2008-02-12T21:56:13Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 12 Feb 2008, Linus Torvalds wrote:\n> \n> But even for git itself, you can get some fun statistics. Try out\n> \n> git log --dirstat\n> \n> and see the occasional mentions of things like Documentation/, git-gui/,\n> gitweb/ and gitk-git/. Or try out something like\n> \n> git diff --dirstat v1.5.0..v1.5.4\n\nSide note: this latter one is actually a good example of two \ndifferent issues.\n\nFirst off, it's an example of where --cumulative really does make sense, \nsince it reports:\n\n\t 10.0% Documentation/\n\t 4.9% contrib/\n\t 5.7% git-gui/lib/\n\t 13.9% git-gui/macosx/\n\t 8.7% git-gui/po/\n\t 31.4% git-gui/\n\t 4.6% gitk-git/\n\t 13.1% t/\n\nie it makes it clear that almost a third of changes between 1.5.0 and \n1.5.4 was in git-gui/.\n\nBut secondly, it's an example of how the whole --dirstat is totally \nbuggered in certain circumstances, because the above is simply not *true*: \nyes, a lot of the changes were actually due to git-gui, and all those *.po \nfiles, but the git-gui/macosx/ count is ridiculously off.\n\nWhy? That directory has a binary file, which doesn't have a \"line count\", \nso --dirstat actually ends up using the byte-count instead there, which \ninflates the macosx numbers a lot (same is true of git-gui/lib/, just to a \nsmaller degree).\n\nI don't think that's necessarily a bug, but it's certainly misleading. \nDoes it matter? Not to me, because the kernel generally doesn't have those \nkinds of issues (no binary blobs that ever really change), but I did want \nto point out that there's room for perhaps improving things. Maybe we \ncould add a byte count in general to the diffstat data structures and \nalways just count bytes changed rather than lines changed. Or maybe we \nshould consider binary files to have something like \"32 bytes per virtual \nline\" or something.\n\nSo _I_ don't care, but if somebody does, maybe they want to do something \nabout it. I thought I'd point it out since I just noticed.\n\n\t\t\t\t\tLinus\n"},{"id":"68581","messageId":"7v1w7h1xs1.fsf@gitster.siamese.dyndns.org","threadId":"12062","inReplyTo":"alpine.LFD.1.00.0802121346560.2920@woody.linux-foundation.org","subject":"Re: [PATCH updated] Add \"--dirstat\" for some directory statistics","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-13T00:54:22Z","receivedAt":"2008-02-13T00:54:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Why? That directory has a binary file, which doesn't have a \"line count\", \n> so --dirstat actually ends up using the byte-count instead there, which \n> inflates the macosx numbers a lot (same is true of git-gui/lib/, just to a \n> smaller degree).\n>\n> I don't think that's necessarily a bug, but it's certainly misleading. \n> Does it matter? Not to me, because the kernel generally doesn't have those \n> kinds of issues (no binary blobs that ever really change), but I did want \n> to point out that there's room for perhaps improving things. Maybe we \n> could add a byte count in general to the diffstat data structures and \n> always just count bytes changed rather than lines changed. Or maybe we \n> should consider binary files to have something like \"32 bytes per virtual \n> line\" or something.\n\nYou can drop the \"line count\" idea and instead count the \"amount\nof damage\" done to the preimage, just like we do in the rename\nsimilarity computation.\n\nThe attached patch is just an outline. There may need special\ncases for unmerged paths.\n\nComparison with your examples is a bit interesting:\n\n (non cumulative) (linus, non cumulative)\n 18.6% crypto/ 20.5% crypto/ \n 7.7% fs/afs/ 7.6% fs/afs/ \n 10.9% fs/fuse/ 7.6% fs/fuse/ \n 7.4% fs/gfs2/ 7.6% fs/gfs2/ \n 5.3% fs/jffs2/ 5.1% fs/jffs2/ \n 4.9% fs/nfs/ 5.1% fs/nfs/ \n 4.5% fs/nfsd/ 5.1% fs/nfsd/ \n 7.4% fs/reiserfs/ 7.6% fs/reiserfs/ \n 14.4% fs/ 15.3% fs/ \n 7.4% net/rxrpc/ 7.6% net/rxrpc/ \n 10.8% security/keys/ 10.2% security/keys/ \n\nSo it does not change much for text-only project. But the big\nbinary file difference is now in the noise:\n\n (git) (linus, git) \n 12.4% Documentation/ 10.0% Documentation/ \n 7.3% contrib/ 4.9% contrib/ \n 6.0% git-gui/lib/ 5.7% git-gui/lib/ \n 12.9% git-gui/po/ 13.9% git-gui/macosx/ \n 23.0% git-gui/ 8.7% git-gui/po/ \n 7.3% gitk-git/ 31.4% git-gui/ \n 14.1% t/ 4.6% gitk-git/ \n 13.1% t/ \n\ngit-gui/lib is about 220k while macosx is 30k, when counted with\n\"find $there -type f | xargs cat | wc -c\", so this reflects the\nreality much better.\n\nI also suspect --cumulative should imply --dirstat but that is a\ndifferent patch if ever.\n\n diff.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++---------------\n 1 files changed, 61 insertions(+), 19 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex dd374d4..8ad04ac 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -990,18 +990,23 @@ static void show_numstat(struct diffstat_t* data, struct diff_options *options)\n \t}\n }\n \n-struct diffstat_dir {\n-\tstruct diffstat_file **files;\n-\tint nr, percent, cumulative;\n+struct dirstat_file {\n+\tconst char *name;\n+\tunsigned long changed;\n };\n \n-static long gather_dirstat(struct diffstat_dir *dir, unsigned long changed, const char *base, int baselen)\n+struct dirstat_dir {\n+\tstruct dirstat_file *files;\n+\tint alloc, nr, percent, cumulative;\n+};\n+\n+static long gather_dirstat(struct dirstat_dir *dir, unsigned long changed, const char *base, int baselen)\n {\n \tunsigned long this_dir = 0;\n \tunsigned int sources = 0;\n \n \twhile (dir->nr) {\n-\t\tstruct diffstat_file *f = *dir->files;\n+\t\tstruct dirstat_file *f = dir->files;\n \t\tint namelen = strlen(f->name);\n \t\tunsigned long this;\n \t\tchar *slash;\n@@ -1016,7 +1021,7 @@ static long gather_dirstat(struct diffstat_dir *dir, unsigned long changed, cons\n \t\t\tthis = gather_dirstat(dir, changed, f->name, newbaselen);\n \t\t\tsources++;\n \t\t} else {\n-\t\t\tthis = f->added + f->deleted;\n+\t\t\tthis = f->changed;\n \t\t\tdir->files++;\n \t\t\tdir->nr--;\n \t\t\tsources += 2;\n@@ -1044,17 +1049,58 @@ static long gather_dirstat(struct diffstat_dir *dir, unsigned long changed, cons\n \treturn this_dir;\n }\n \n-static void show_dirstat(struct diffstat_t *data, struct diff_options *options)\n+static void show_dirstat(struct diff_options *options)\n {\n \tint i;\n \tunsigned long changed;\n-\tstruct diffstat_dir dir;\n+\tstruct dirstat_dir dir;\n+\tstruct diff_queue_struct *q = &diff_queued_diff;\n+\n+\tdir.files = NULL;\n+\tdir.alloc = 0;\n+\tdir.nr = 0;\n+\tdir.percent = options->dirstat_percent;\n+\tdir.cumulative = options->output_format & DIFF_FORMAT_CUMULATIVE;\n \n-\t/* Calculate total changes */\n \tchanged = 0;\n-\tfor (i = 0; i < data->nr; i++) {\n-\t\tchanged += data->files[i]->added;\n-\t\tchanged += data->files[i]->deleted;\n+\tfor (i = 0; i < q->nr; i++) {\n+\t\tstruct diff_filepair *p = q->queue[i];\n+\t\tconst char *name;\n+\t\tunsigned long copied, added, damage;\n+\n+\t\tname = p->one->path ? p->one->path : p->two->path;\n+\n+\t\tif (DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two)) {\n+\t\t\tdiff_populate_filespec(p->one, 0);\n+\t\t\tdiff_populate_filespec(p->two, 0);\n+\t\t\tdiffcore_count_changes(p->one, p->two, NULL, NULL, 0,\n+\t\t\t\t\t &copied, &added);\n+\t\t\tdiff_free_filespec_data(p->one);\n+\t\t\tdiff_free_filespec_data(p->two);\n+\t\t} else if (DIFF_FILE_VALID(p->one)) {\n+\t\t\tdiff_populate_filespec(p->one, 1);\n+\t\t\tcopied = added = 0;\n+\t\t\tdiff_free_filespec_data(p->one);\n+\t\t} else if (DIFF_FILE_VALID(p->two)) {\n+\t\t\tdiff_populate_filespec(p->two, 1);\n+\t\t\tcopied = 0;\n+\t\t\tadded = p->two->size;\n+\t\t\tdiff_free_filespec_data(p->two);\n+\t\t} else\n+\t\t\tcontinue;\n+\n+\t\t/*\n+\t\t * Original minus copied is the removed material,\n+\t\t * added is the new material. They are both damages\n+\t\t * made to the preimage.\n+\t\t */\n+\t\tdamage = (p->one->size - copied) + added;\n+\n+\t\tALLOC_GROW(dir.files, dir.nr + 1, dir.alloc);\n+\t\tdir.files[dir.nr].name = name;\n+\t\tdir.files[dir.nr].changed = damage;\n+\t\tchanged += damage;\n+\t\tdir.nr++;\n \t}\n \n \t/* This can happen even with many files, if everything was renames */\n@@ -1062,10 +1108,6 @@ static void show_dirstat(struct diffstat_t *data, struct diff_options *options)\n \t\treturn;\n \n \t/* Show all directories with more than x% of the changes */\n-\tdir.files = data->files;\n-\tdir.nr = data->nr;\n-\tdir.percent = options->dirstat_percent;\n-\tdir.cumulative = options->output_format & DIFF_FORMAT_CUMULATIVE;\n \tgather_dirstat(&dir, changed, \"\", 0);\n }\n \n@@ -3024,7 +3066,7 @@ void diff_flush(struct diff_options *options)\n \t\tseparator++;\n \t}\n \n-\tif (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT|DIFF_FORMAT_DIRSTAT)) {\n+\tif (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT)) {\n \t\tstruct diffstat_t diffstat;\n \n \t\tmemset(&diffstat, 0, sizeof(struct diffstat_t));\n@@ -3034,8 +3076,6 @@ void diff_flush(struct diff_options *options)\n \t\t\tif (check_pair_status(p))\n \t\t\t\tdiff_flush_stat(p, options, &diffstat);\n \t\t}\n-\t\tif (output_format & DIFF_FORMAT_DIRSTAT)\n-\t\t\tshow_dirstat(&diffstat, options);\n \t\tif (output_format & DIFF_FORMAT_NUMSTAT)\n \t\t\tshow_numstat(&diffstat, options);\n \t\tif (output_format & DIFF_FORMAT_DIFFSTAT)\n@@ -3045,6 +3085,8 @@ void diff_flush(struct diff_options *options)\n \t\tfree_diffstat_info(&diffstat);\n \t\tseparator++;\n \t}\n+\tif (output_format & DIFF_FORMAT_DIRSTAT)\n+\t\tshow_dirstat(options);\n \n \tif (output_format & DIFF_FORMAT_SUMMARY && !is_summary_empty(q)) {\n \t\tfor (i = 0; i < q->nr; i++)\n"},{"id":"68586","messageId":"alpine.LFD.1.00.0802121701310.2920@woody.linux-foundation.org","threadId":"12062","inReplyTo":"7v1w7h1xs1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH updated] Add \"--dirstat\" for some directory statistics","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-13T01:10:04Z","receivedAt":"2008-02-13T01:10:04Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 12 Feb 2008, Junio C Hamano wrote:\n> \n> You can drop the \"line count\" idea and instead count the \"amount\n> of damage\" done to the preimage, just like we do in the rename\n> similarity computation.\n> \n> The attached patch is just an outline. There may need special\n> cases for unmerged paths.\n\nOuch. I like the concept and the result, but an not a huge fan of the the \nimplementation. The problem with this is that it makes the whole damage \ncomputation something separate from the earlier phases.\n\nSo it actually seems to add a lot of cost. Right now there is basically \nzero added cost to just adding a \"--dirstat\" to one of my common \noperations, which is\n\n\tgit diff -M --stat --summary\n\n(that's what I do on merges). \n\nWouldn't it be nicer to merge it with one of the stages we did earlier by \nsimply saving the data. If not the --stat phase, then the -M phase? \n\n\t\t\tLinus\n"},{"id":"68588","messageId":"7vbq6lzluy.fsf@gitster.siamese.dyndns.org","threadId":"12062","inReplyTo":"alpine.LFD.1.00.0802121701310.2920@woody.linux-foundation.org","subject":"Re: [PATCH updated] Add \"--dirstat\" for some directory statistics","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-13T01:27:49Z","receivedAt":"2008-02-13T01:27:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> On Tue, 12 Feb 2008, Junio C Hamano wrote:\n>> \n>> You can drop the \"line count\" idea and instead count the \"amount\n>> of damage\" done to the preimage, just like we do in the rename\n>> similarity computation.\n>> \n>> The attached patch is just an outline. There may need special\n>> cases for unmerged paths.\n>\n> Ouch. I like the concept and the result, but an not a huge fan of the the \n> implementation. The problem with this is that it makes the whole damage \n> computation something separate from the earlier phases.\n>\n> So it actually seems to add a lot of cost. Right now there is basically \n> zero added cost to just adding a \"--dirstat\" to one of my common \n> operations, which is\n>\n> \tgit diff -M --stat --summary\n>\n> (that's what I do on merges). \n>\n> Wouldn't it be nicer to merge it with one of the stages we did earlier by \n> simply saving the data. If not the --stat phase, then the -M phase? \n\nWe could add one ulong to diff_filepair struct to record\n\"damage\", and populate it inside diffcore_rename().\n\nObviously:\n\n - You would need to diffcore_count_changes() in separate phase\n if you do not use -M;\n\n - Even if you use -M, we do not run diffcore_count_changes()\n for unrenamed paths during diffcore_rename(), so you would\n need to do that separately.\n\nBy the way, if you run \"git diff --summary --dirstat -M\" (that\nis, without --stat), we will not have to run diffstat at all.\nInstead, we only run diffcore_count_changes(), which can be\nreused for renamed paths if we keep it in diff_filepair struct.\n"},{"id":"68645","messageId":"fou7pu$dpq$1@ger.gmane.org","threadId":"12062","inReplyTo":"alpine.LFD.1.00.0802121308360.2920@woody.linux-foundation.org","subject":"Re: [PATCH updated] Add \"--dirstat\" for some directory statistics","fromName":"Abdelrazak Younes","fromEmail":"younes@lyx.org","sentAt":"2008-02-13T07:55:37Z","receivedAt":"2008-02-13T07:55:37Z","isPatch":true,"sender":{"key":"younes@lyx.org","avatar":null},"body":"Linus Torvalds wrote:\n> \t 7.6% fs/afs/\n> \t 7.6% fs/fuse/\n> \t 7.6% fs/gfs2/\n> \t 5.1% fs/jffs2/\n> \t 5.1% fs/nfs/\n> \t 5.1% fs/nfsd/\n> \t 7.6% fs/reiserfs/\n> \t 15.3% fs/\n[...]\n> For an example of the cumulative reporting, the above commit becomes\n[...]\n> \t 7.6% fs/afs/\n> \t 7.6% fs/fuse/\n> \t 7.6% fs/gfs2/\n> \t 5.1% fs/jffs2/\n> \t 5.1% fs/nfs/\n> \t 5.1% fs/nfsd/\n> \t 7.6% fs/reiserfs/\n> \t 61.5% fs/\n\n\nMay I suggest this instead so to get rid of the cumulative option?\n\n \t 7.6% fs/afs\n \t 7.6% fs/fuse\n \t 7.6% fs/gfs2\n \t 5.1% fs/jffs2\n \t 5.1% fs/nfs\n \t 5.1% fs/nfsd\n \t 7.6% fs/reiserfs\n \t 15.3% fs/\n \t 61.5% fs\n\nA trailing slash would mean \"no recursive, only this directory\" and no \ntrailing slash means well the opposite :-)\n\nAbdel.\n"},{"id":"68651","messageId":"alpine.LSU.1.00.0802131347490.30505@racer.site","threadId":"12062","inReplyTo":"fou7pu$dpq$1@ger.gmane.org","subject":"Re: [PATCH updated] Add \"--dirstat\" for some directory statistics","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-13T13:48:22Z","receivedAt":"2008-02-13T13:48:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 13 Feb 2008, Abdelrazak Younes wrote:\n\n> Linus Torvalds wrote:\n> > \t 7.6% fs/afs/\n> > \t 7.6% fs/fuse/\n> > \t 7.6% fs/gfs2/\n> > \t 5.1% fs/jffs2/\n> > \t 5.1% fs/nfs/\n> > \t 5.1% fs/nfsd/\n> > \t 7.6% fs/reiserfs/\n> > \t 15.3% fs/\n> [...]\n> > For an example of the cumulative reporting, the above commit becomes\n> [...]\n> > \t 7.6% fs/afs/\n> > \t 7.6% fs/fuse/\n> > \t 7.6% fs/gfs2/\n> > \t 5.1% fs/jffs2/\n> > \t 5.1% fs/nfs/\n> > \t 5.1% fs/nfsd/\n> > \t 7.6% fs/reiserfs/\n> > \t 61.5% fs/\n> \n> \n> May I suggest this instead so to get rid of the cumulative option?\n> \n> \t 7.6% fs/afs\n> \t 7.6% fs/fuse\n> \t 7.6% fs/gfs2\n> \t 5.1% fs/jffs2\n> \t 5.1% fs/nfs\n> \t 5.1% fs/nfsd\n> \t 7.6% fs/reiserfs\n> \t 15.3% fs/\n> \t 61.5% fs\n> \n> A trailing slash would mean \"no recursive, only this directory\" and no\n> trailing slash means well the opposite :-)\n\nMy 2cents: I find that highly counterintuitive (yes, I am a frequent user \nof \"du\").\n\nCiao,\nDscho\n"},{"id":"68653","messageId":"06C59F5E-422E-4510-9D8B-9FAA1D8C4F45@wincent.com","threadId":"12062","inReplyTo":"fou7pu$dpq$1@ger.gmane.org","subject":"Re: [PATCH updated] Add \"--dirstat\" for some directory statistics","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2008-02-13T14:07:52Z","receivedAt":"2008-02-13T14:07:52Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 13/2/2008, a las 8:55, Abdelrazak Younes escribió:\n\n> May I suggest this instead so to get rid of the cumulative option?\n>\n> \t 7.6% fs/afs\n> \t 7.6% fs/fuse\n> \t 7.6% fs/gfs2\n> \t 5.1% fs/jffs2\n> \t 5.1% fs/nfs\n> \t 5.1% fs/nfsd\n> \t 7.6% fs/reiserfs\n> \t 15.3% fs/\n> \t 61.5% fs\n>\n> A trailing slash would mean \"no recursive, only this directory\" and \n> no trailing slash means well the opposite :-)\n\nI think that's getting a little _too_ subtle.\n\nCheers,\nWincent\n"}]}