patchAdd "--dirstat" for some directory statistics

This adds a new form of overview diffstat output, doing something that I have occasionally ended up doing manually (and badly, because it's actually pretty nasty to do), and that I think is very useful for an project like the kernel that has a fairly deep and well-separated directory structure with semantic meaning.

What I mean by that is that it's often interesting to see exactly which sub-directories are impacted by a patch, and to what degree - even if you don't perhaps care so much about the individual files themselves.

What makes the concept more interesting is that the "impact" is often hierarchical: in the kernel, for example, something could either have a very localized impact to "fs/ext3/" and then it's interesting to see that such a patch changes mostly that subdirectory, but you could have another patch that changes some generic VFS-layer issue which affects _many_ subdirectories that are all under "fs/", but none - or perhaps just a couple of them - of the individual filesystems are interesting in themselves.

So what commonly happens is that you may have big changes in a specific sub-subdirectory, but still also significant separate changes to the subdirectory leading up to that - maybe you have significant VFS-level changes, but *also* changes under that VFS layer in the NFS-specific directories, for example. In that case, you do want the low-level parts that are significant to show up, but then the insignificant ones should show up as under the more generic top-level directory.

This patch shows all of that with "--dirstat". The output can be either something simple like

        commit 81772fe...
        Author: Thomas Gleixner <tglx@linutronix.de>
        Date:   Sun Feb 10 23:57:36 2008 +0100

            x86: remove over noisy debug printk

            pageattr-test.c contains a noisy debug printk that people reported.
            The condition under which it prints (randomly tapping into a mem_map[]
            hole and not being able to c_p_a() there) is valid behavior and not
            interesting to report.

            Remove it.

            Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
            Acked-by: Ingo Molnar <mingo@elte.hu>
            Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

         100.0% arch/x86/mm/

or something much more complex like

        commit e231c2e...
        Author: David Howells <dhowells@redhat.com>
        Date:   Thu Feb 7 00:15:26 2008 -0800

            Convert ERR_PTR(PTR_ERR(p)) instances to ERR_CAST(p)

	  20.5% crypto/
	   7.6% fs/afs/
	   7.6% fs/fuse/
	   7.6% fs/gfs2/
	   5.1% fs/jffs2/
	   5.1% fs/nfs/
	   5.1% fs/nfsd/
	   7.6% fs/reiserfs/
	  15.3% fs/
	   7.6% net/rxrpc/
	  10.2% security/keys/

where that latter example is an example of significant work in some individual fs/*/ subdirectories (like the patches to reiserfs accounting for 7.6% of the whole), but then discounting those individual filesystems, there's also 15.3% other "random" things that weren't worth reporting on their oen left over under fs/ in general (either in that directory itself, or in subdirectories of fs/ that didn't have enough changes to be reported individually).

I'd like to stress that the "15.3% fs/" mentioned above is the stuff that is under fs/ but that was _not_ significant enough to report on its own. So the above does _not_ mean that 15.3% of the work was under fs/ per se, because that 15.3% does *not* include the already-reported 7.6% of afs, 7.6% of fuse etc.

If you want to enable "cumulative" directory statistics, you can use the "--cumulative" flag, which adds up percentages recursively even when they have been already reported for a sub-directory. That cumulative output is disabled if *all* of the changes in one subdirectory come from a deeper subdirectory, to avoid repeating subdirectories all the way to the root.

For an example of the cumulative reporting, the above commit becomes

	commit e231c2e...
	Author: David Howells <dhowells@redhat.com>
	Date:   Thu Feb 7 00:15:26 2008 -0800

	    Convert ERR_PTR(PTR_ERR(p)) instances to ERR_CAST(p)

	  20.5% crypto/
	   7.6% fs/afs/
	   7.6% fs/fuse/
	   7.6% fs/gfs2/
	   5.1% fs/jffs2/
	   5.1% fs/nfs/
	   5.1% fs/nfsd/
	   7.6% fs/reiserfs/
	  61.5% fs/
	   7.6% net/rxrpc/
	  10.2% security/keys/

in which the commit percentages now obviously add up to much more than 100%: now the changes that were already reported for the sub-directories under fs/ are then cumulatively included in the whole percentage of fs/ (ie now shows 61.5% as opposed to the 15.3% without the cumulative reporting).

The default reporting limit has been arbitrarily set at 3%, which seems to be a pretty good cut-off, but you can specify the cut-off manually by giving it as an option parameter (eg "--dirstat=5" makes the cut-off be at 5% instead)

NOTE! The percentages are purely about the total lines added and removed, not anything smarter (or dumber) than that. Also note that you should not generally expect things to add up to 100%: not only does it round down, we don't report leftover scraps (they add up to the top-level change count, but we don't even bother reporting that, it only reports subdirectories).

Quite frankly, as a top-level manager this is really convenient for me, but it's going to be very boring for git itself since there are few subdirectories. Also, don't expect things to make tons of sense if you combine this with "-M" and there are cross-directory renames etc.

But even for git itself, you can get some fun statistics. Try out

        git log --dirstat

and see the occasional mentions of things like Documentation/, git-gui/, gitweb/ and gitk-git/. Or try out something like

        git diff --dirstat v1.5.0..v1.5.4

which does kind of git an overview that shows *something*. But in general, the output is more exciting for big projects with deeper structure, and doing a

        git diff --dirstat v2.6.24..v2.6.25-rc1

on the kernel is what I actually wrote this for!

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

This is a re-send of all my changes rolled up into one single patch. One of the reasons is that the "report percentages in tenths of percent" simply changed the examples in the original commit message. Another is that I actually made the "--cumulative" flag much more useful in practice by making it avoid reporting the subdirectories where all the percentages came from just a single already-reported sub-subdirectory.

The --cumulative thing still has the odd property of having all the percentages add up to more than 100%, but quite often it is now what you really want, and you can, for example, get output like this:

	  27.5% security/selinux/include/
	 100.0% security/selinux/

which means that everything is in "security/selinux", but within that directory we also show that 27.5% was within the include subdirectory. Note how it now no longer gives a "100.0% security/" report, since that would be purely redundant information.

Another example of this new semantic:

	  23.8% include/asm-sparc/
	  76.1% include/asm-sparc64/
	 100.0% include/

here, "100% include/" _is_ reported, because while the 100% did come from the already reported subdirectories, it didn't come from a _single_ one, so it basically means that you don't ever have to mentally add up things, and we only avoid reporting upper-level directory statistics when there really is no point to it.

With this change, "--cumulative" suddenly makes tons of sense to me. For example, for the 2.6.24..2.6.25-rc1 changes in the kernel (with a 2% cutoff), it now gives:

 - "git diff --dirstat=2 --cumulative v2.6.24..v2.6.25-rc1" output:

	   2.4% Documentation/
	   3.2% arch/arm/
	   2.0% arch/cris/
	   2.3% arch/powerpc/boot/
	   2.3% arch/powerpc/configs/
	   6.9% arch/powerpc/
	   3.2% arch/sh/
	   2.3% arch/x86/kernel/
	   6.9% arch/x86/
	  31.3% arch/
	   2.0% drivers/char/
	   2.0% drivers/infiniband/
	   2.9% drivers/media/video/
	   3.7% drivers/media/
	   2.4% drivers/net/wireless/iwlwifi/
	   7.3% drivers/net/wireless/
	  14.3% drivers/net/
	   2.8% drivers/scsi/
	  37.2% drivers/
	   3.5% fs/
	   2.0% include/asm-x86/
	  10.3% include/
	   6.0% net/
	   3.7% sound/

and now you can see at a glance that 31.3 was in arch/, and 37.2% was in drivers/. So this is a case where --cumulative actually makes sense (but it probably still doesn't make sense if you do a "git log --dirstat" and look at just individual commits that tend to be more about a single change).

Anyway, it's a bit larger now, but my explanations and blathering is still much bigger than the patch itself ;)

 diff.c |   90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 diff.h |    3 ++
 2 files changed, 92 insertions(+), 1 deletions(-)

On Tue, 12 Feb 2008, Linus Torvalds wrote:

Side note: this latter one is actually a good example of two different issues.

First off, it's an example of where --cumulative really does make sense, since it reports:

	  10.0% Documentation/
	   4.9% contrib/
	   5.7% git-gui/lib/
	  13.9% git-gui/macosx/
	   8.7% git-gui/po/
	  31.4% git-gui/
	   4.6% gitk-git/
	  13.1% t/

ie it makes it clear that almost a third of changes between 1.5.0 and 1.5.4 was in git-gui/.

But secondly, it's an example of how the whole --dirstat is totally buggered in certain circumstances, because the above is simply not *true*: yes, a lot of the changes were actually due to git-gui, and all those *.po files, but the git-gui/macosx/ count is ridiculously off.

Why? That directory has a binary file, which doesn't have a "line count", so --dirstat actually ends up using the byte-count instead there, which inflates the macosx numbers a lot (same is true of git-gui/lib/, just to a smaller degree).

I don't think that's necessarily a bug, but it's certainly misleading. Does it matter? Not to me, because the kernel generally doesn't have those kinds of issues (no binary blobs that ever really change), but I did want to point out that there's room for perhaps improving things. Maybe we could add a byte count in general to the diffstat data structures and always just count bytes changed rather than lines changed. Or maybe we should consider binary files to have something like "32 bytes per virtual line" or something.

So _I_ don't care, but if somebody does, maybe they want to do something about it. I thought I'd point it out since I just noticed.

					Linus

Linus Torvalds <torvalds@linux-foundation.org> writes:

You can drop the "line count" idea and instead count the "amount of damage" done to the preimage, just like we do in the rename similarity computation.

The attached patch is just an outline. There may need special cases for unmerged paths.

Comparison with your examples is a bit interesting:

    (non cumulative)            (linus, non cumulative)
      18.6% crypto/              20.5% crypto/             
       7.7% fs/afs/               7.6% fs/afs/             
      10.9% fs/fuse/              7.6% fs/fuse/            
       7.4% fs/gfs2/              7.6% fs/gfs2/            
       5.3% fs/jffs2/             5.1% fs/jffs2/       
       4.9% fs/nfs/               5.1% fs/nfs/             
       4.5% fs/nfsd/              5.1% fs/nfsd/            
       7.4% fs/reiserfs/          7.6% fs/reiserfs/    
      14.4% fs/                  15.3% fs/         
       7.4% net/rxrpc/            7.6% net/rxrpc/      
      10.8% security/keys/       10.2% security/keys/  

So it does not change much for text-only project. But the big binary file difference is now in the noise:

    (git)                       (linus, git)                 
      12.4% Documentation/       10.0% Documentation/        
       7.3% contrib/              4.9% contrib/              
       6.0% git-gui/lib/          5.7% git-gui/lib/  
      12.9% git-gui/po/          13.9% git-gui/macosx/       
      23.0% git-gui/              8.7% git-gui/po/   
       7.3% gitk-git/            31.4% git-gui/              
      14.1% t/                    4.6% gitk-git/     
                                 13.1% t/                

git-gui/lib is about 220k while macosx is 30k, when counted with "find $there -type f | xargs cat | wc -c", so this reflects the reality much better.

I also suspect --cumulative should imply --dirstat but that is a different patch if ever.

 diff.c |   80 ++++++++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 61 insertions(+), 19 deletions(-)

On Tue, 12 Feb 2008, Junio C Hamano wrote:

Ouch. I like the concept and the result, but an not a huge fan of the the implementation. The problem with this is that it makes the whole damage computation something separate from the earlier phases.

So it actually seems to add a lot of cost. Right now there is basically zero added cost to just adding a "--dirstat" to one of my common operations, which is

	git diff -M --stat --summary

(that's what I do on merges). 

Wouldn't it be nicer to merge it with one of the stages we did earlier by simply saving the data. If not the --stat phase, then the -M phase?

			Linus

Linus Torvalds <torvalds@linux-foundation.org> writes:

We could add one ulong to diff_filepair struct to record "damage", and populate it inside diffcore_rename().

Obviously:

 - You would need to diffcore_count_changes() in separate phase
   if you do not use -M;

 - Even if you use -M, we do not run diffcore_count_changes()
   for unrenamed paths during diffcore_rename(), so you would
   need to do that separately.

By the way, if you run "git diff --summary --dirstat -M" (that is, without --stat), we will not have to run diffstat at all. Instead, we only run diffcore_count_changes(), which can be reused for renamed paths if we keep it in diff_filepair struct.

Linus Torvalds wrote:

[...]

[...]

May I suggest this instead so to get rid of the cumulative option?

  	   7.6% fs/afs
  	   7.6% fs/fuse
  	   7.6% fs/gfs2
  	   5.1% fs/jffs2
  	   5.1% fs/nfs
  	   5.1% fs/nfsd
  	   7.6% fs/reiserfs
  	  15.3% fs/
  	  61.5% fs

A trailing slash would mean "no recursive, only this directory" and no trailing slash means well the opposite :-)

Abdel.

Hi,

On Wed, 13 Feb 2008, Abdelrazak Younes wrote:

My 2cents: I find that highly counterintuitive (yes, I am a frequent user of "du").

Ciao, Dscho

El 13/2/2008, a las 8:55, Abdelrazak Younes escribió:

I think that's getting a little _too_ subtle.

Cheers, Wincent