{"thread":{"id":"29594","subject":"git status: small difference between stating whole repository and small subdirectory","startedAt":"2012-02-10T09:42:22Z","lastAt":"2012-04-10T18:00:44Z","messageCount":52,"participants":["Piotr Krukowiecki","Nguyen Thai Ngoc Duy","Thomas Rast","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"184341","messageId":"CAA01Csr8FbvQ8uFvxX8_6i-hysin6JuaifVVC-yoLyoT0N5F4Q@mail.gmail.com","threadId":"29594","inReplyTo":null,"subject":"git status: small difference between stating whole repository and small subdirectory","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2012-02-10T09:42:22Z","receivedAt":"2012-02-10T09:42:22Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"Hi,\n\nI compared stating whole tree vs one small subdirectory, and I\nexpected that for the subdirectory status will be very very fast.\nAfter all, it has only few files to stat. But it's not fast. Why?\n\n\nWith cold cache (echo 3 | sudo tee /proc/sys/vm/drop_caches):\n\n$ time git status > /dev/null\nreal\t0m41.670s\nuser\t0m0.980s\nsys\t0m2.908s\n\n$ time git status -- src/.../somedir > /dev/null\nreal\t0m17.380s\nuser\t0m0.748s\nsys\t0m0.328s\n\n\nWith warm cache:\n\n$ time git status > /dev/null\nreal\t0m0.792s\nuser\t0m0.404s\nsys\t0m0.384s\n\n$ time git status -- src/.../somedir > /dev/null\nreal\t0m0.335s\nuser\t0m0.288s\nsys\t0m0.048s\n\n\nRepository/dir stats:\n\n$ find * -type f | wc -l\n127189\n$ du -shc * | grep total\n2.2G\ttotal\n\n$ find src/.../somedir -type f | wc -l\n55\n$ du -shc src/.../somedir | grep total\n224K\ttotal\n\n\n$ git --version\ngit version 1.7.9.rc0.10.gbeecc\n\n-- \nPiotr Krukowiecki\n"},{"id":"184347","messageId":"CACsJy8DnqYZ5CdZqbebWS4NS85mfwumyao0abeqDxMXrHhDELA@mail.gmail.com","threadId":"29594","inReplyTo":"CAA01Csr8FbvQ8uFvxX8_6i-hysin6JuaifVVC-yoLyoT0N5F4Q@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-10T12:33:41Z","receivedAt":"2012-02-10T12:33:41Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Feb 10, 2012 at 4:42 PM, Piotr Krukowiecki\n wrote:\n> Hi,\n>\n> I compared stating whole tree vs one small subdirectory, and I\n> expected that for the subdirectory status will be very very fast.\n> After all, it has only few files to stat. But it's not fast. Why?\n\nBecause stat'ing is not the only thing git-status does? In order to\nfind out staged changes, unstaged changes and untracked files, it has\nto do the equivalence of \"git diff --cached\", \"git diff\" and \"git\nls-files -o\". I think copy detection is also enabled, which uses more\ncycles.\n\nProfiling it should give you a good idea what parts cost most.\n-- \nDuy\n"},{"id":"184352","messageId":"CAA01Csq+zVhRTjGqy1DJGK7o5wByd9ADsSim214T7Vkxmk+ykQ@mail.gmail.com","threadId":"29594","inReplyTo":"CACsJy8DnqYZ5CdZqbebWS4NS85mfwumyao0abeqDxMXrHhDELA@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2012-02-10T13:46:09Z","receivedAt":"2012-02-10T13:46:09Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Fri, Feb 10, 2012 at 1:33 PM, Nguyen Thai Ngoc Duy wrote:\n> On Fri, Feb 10, 2012 at 4:42 PM, Piotr Krukowiecki\n> wrote:\n>> Hi,\n>>\n>> I compared stating whole tree vs one small subdirectory, and I\n>> expected that for the subdirectory status will be very very fast.\n>> After all, it has only few files to stat. But it's not fast. Why?\n>\n> Because stat'ing is not the only thing git-status does? In order to\n> find out staged changes, unstaged changes and untracked files, it has\n> to do the equivalence of \"git diff --cached\", \"git diff\" and \"git\n> ls-files -o\". I think copy detection is also enabled, which uses more\n> cycles.\n\nI believe copy detection is not done, neither for tracked nor untracked files.\nRename detection is done for tracked files. In this case it should not\nmatter, as there were no changes to the files.\n\nMy point is, that for such small number of small files (55 files and\n223KB), which had no changes at all, the status took a lot of time (17\nseconds) and doing status on whole repository which has more than\n2000x files and 10000x data took only 2x more time.\n\nI don't think that the algorithm scales so well, so my guess is that\n'status' is so inefficient for subdirectories (i.e. the \"git status --\ndir\" filter does not filter as much as it could).\n\n\n> Profiling it should give you a good idea what parts cost most.\n\nI'll try that later.\n\nBTW by stating I meant using \"status\", not that it uses stat() or whatever.\n\n\n-- \nPiotr Krukowiecki\n"},{"id":"184355","messageId":"CACsJy8C05wvQRRQJLxrxYKHjXsgh6RugFexkPUKYGxbQkqiXJA@mail.gmail.com","threadId":"29594","inReplyTo":"CAA01Csq+zVhRTjGqy1DJGK7o5wByd9ADsSim214T7Vkxmk+ykQ@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-10T14:37:21Z","receivedAt":"2012-02-10T14:37:21Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Feb 10, 2012 at 8:46 PM, Piotr Krukowiecki\n wrote:\n> On Fri, Feb 10, 2012 at 1:33 PM, Nguyen Thai Ngoc Duy wrote:\n>> On Fri, Feb 10, 2012 at 4:42 PM, Piotr Krukowiecki\n>> wrote:\n>>> Hi,\n>>>\n>>> I compared stating whole tree vs one small subdirectory, and I\n>>> expected that for the subdirectory status will be very very fast.\n>>> After all, it has only few files to stat. But it's not fast. Why?\n>>\n>> Because stat'ing is not the only thing git-status does? In order to\n>> find out staged changes, unstaged changes and untracked files, it has\n>> to do the equivalence of \"git diff --cached\", \"git diff\" and \"git\n>> ls-files -o\". I think copy detection is also enabled, which uses more\n>> cycles.\n>\n> I believe copy detection is not done, neither for tracked nor untracked files.\n> Rename detection is done for tracked files. In this case it should not\n> matter, as there were no changes to the files.\n>\n> My point is, that for such small number of small files (55 files and\n> 223KB), which had no changes at all, the status took a lot of time (17\n> seconds) and doing status on whole repository which has more than\n> 2000x files and 10000x data took only 2x more time.\n>\n> I don't think that the algorithm scales so well, so my guess is that\n> 'status' is so inefficient for subdirectories (i.e. the \"git status --\n> dir\" filter does not filter as much as it could).\n\nI think the cost is in $GIT_DIR, not the working directory.\n-- \nDuy\n"},{"id":"184358","messageId":"CAA01Csp6_9fP2rg4104UWUXwOxZmUVdQNDAaBe6fRou6agBz6g@mail.gmail.com","threadId":"29594","inReplyTo":"CAA01Csr8FbvQ8uFvxX8_6i-hysin6JuaifVVC-yoLyoT0N5F4Q@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2012-02-10T16:18:55Z","receivedAt":"2012-02-10T16:18:55Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Fri, Feb 10, 2012 at 10:42 AM, Piotr Krukowiecki\n wrote:\n> Hi,\n>\n> I compared stating whole tree vs one small subdirectory, and I\n> expected that for the subdirectory status will be very very fast.\n> After all, it has only few files to stat. But it's not fast. Why?\n>\n>\n> With cold cache (echo 3 | sudo tee /proc/sys/vm/drop_caches):\n>\n> $ time git status    > /dev/null\n> real    0m41.670s\n> user    0m0.980s\n> sys     0m2.908s\n>\n> $ time git status -- src/.../somedir   > /dev/null\n> real    0m17.380s\n> user    0m0.748s\n> sys     0m0.328s\n>\n>\n> With warm cache:\n>\n> $ time git status    > /dev/null\n> real    0m0.792s\n> user    0m0.404s\n> sys     0m0.384s\n>\n> $ time git status -- src/.../somedir   > /dev/null\n> real    0m0.335s\n> user    0m0.288s\n> sys     0m0.048s\n>\n>\n> Repository/dir stats:\n>\n> $ find * -type f | wc -l\n> 127189\n> $ du -shc * | grep total\n> 2.2G    total\n>\n> $ find src/.../somedir -type f | wc -l\n> 55\n> $ du -shc src/.../somedir | grep total\n> 224K    total\n>\n>\n> $ git --version\n> git version 1.7.9.rc0.10.gbeecc\n\nI can't reproduce this behavior at the moment. 'status' on the\ndirectory takes about 1.5s instead of 17s. status on whole repository\ntakes 27s.\nThis is my work repository, so it was changed today.\n\nI'll get back to you when I can reproduce the problem again...\n\n\n-- \nPiotr Krukowiecki\n"},{"id":"184574","messageId":"CAA01CsqjeDR56n=gdLWtDshp8-=8_cSih9vHV3vnUx0wTockYg@mail.gmail.com","threadId":"29594","inReplyTo":"CACsJy8C05wvQRRQJLxrxYKHjXsgh6RugFexkPUKYGxbQkqiXJA@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2012-02-13T16:54:04Z","receivedAt":"2012-02-13T16:54:04Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Fri, Feb 10, 2012 at 3:37 PM, Nguyen Thai Ngoc Duy wrote:\n> I think the cost is in $GIT_DIR, not the working directory.\n\nCould you explain?\n\n\nI got the problem again today. This time I've made a copy of the\nrepository so hopefully I'll able to reproduce the problems.\n\nThis time it's a different repository but the 'status' on a small\nsubdirectory is even more than 2x slower than on the whole repository.\n\nWhole repo:\n$ find * -type f | wc -l\n33021\n$ du -shc * | grep total\n2.1G\ttotal\n\nThe subdir:\n$ find * -type f | wc -l\n17\n$ du -shc * | grep total\n84K\ttotal\n\nAs previously, timing was done with cold cache (echo 3 | sudo tee\n/proc/sys/vm/drop_caches) and executed several times.\n\nThis time I have used recent git (1.7.9.188.g12766) compiled with -pg.\ngit was executed in the subdirectory. Tracked files were not\nchanged/deleted, there was just a couple of small untracked files.\n\nTimings:\n\n$ time git status\nreal\t0m16.595s\nuser\t0m0.680s\nsys\t0m0.616s\n\n$ time git status -- .\nreal\t0m10.030s\nuser\t0m0.464s\nsys\t0m0.184s\n\nYou can find gprof output here:\nhttp://pastebin.com/mhddDUmv - from whole repo status\nhttp://pastebin.com/1LdVn77A - from subdir status\n\n\n\n-- \nPiotr Krukowiecki\n"},{"id":"184649","messageId":"8762f9k5sg.fsf@thomas.inf.ethz.ch","threadId":"29594","inReplyTo":"CAA01Csp6_9fP2rg4104UWUXwOxZmUVdQNDAaBe6fRou6agBz6g@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-02-14T11:34:07Z","receivedAt":"2012-02-14T11:34:07Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Piotr Krukowiecki writes:\n\n> On Fri, Feb 10, 2012 at 10:42 AM, Piotr Krukowiecki\n> wrote:\n>> I compared stating whole tree vs one small subdirectory, and I\n>> expected that for the subdirectory status will be very very fast.\n>> After all, it has only few files to stat. But it's not fast. Why?\n>>\n>>\n>> With cold cache (echo 3 | sudo tee /proc/sys/vm/drop_caches):\n>>\n>> $ time git status    > /dev/null\n>> real    0m41.670s\n>> user    0m0.980s\n>> sys     0m2.908s\n>>\n>> $ time git status -- src/.../somedir   > /dev/null\n>> real    0m17.380s\n>> user    0m0.748s\n>> sys     0m0.328s\n[...]\n> I can't reproduce this behavior at the moment. 'status' on the\n> directory takes about 1.5s instead of 17s. status on whole repository\n> takes 27s.\n> This is my work repository, so it was changed today.\n\nTo me these timings smell like a combination of either a network\nfilesystem or a slow/busy disk, and non-packed repositories. Next time\nthis happens look at 'git count-objects', run 'git gc' and redo the\ntimings.\n\nIf you are indeed on a network filesystem, also look at the\ncore.preloadIndex setting.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"184752","messageId":"CAA01Cso_8=159UDMFUHiYz1X=gYtpbqRO4h3TMw7N=4YMV8YNg@mail.gmail.com","threadId":"29594","inReplyTo":"8762f9k5sg.fsf@thomas.inf.ethz.ch","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2012-02-15T08:57:29Z","receivedAt":"2012-02-15T08:57:29Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Tue, Feb 14, 2012 at 12:34 PM, Thomas Rast wrote:\n> Piotr Krukowiecki writes:\n>\n>> On Fri, Feb 10, 2012 at 10:42 AM, Piotr Krukowiecki\n>> wrote:\n>>> I compared stating whole tree vs one small subdirectory, and I\n>>> expected that for the subdirectory status will be very very fast.\n>>> After all, it has only few files to stat. But it's not fast. Why?\n>>>\n>>>\n>>> With cold cache (echo 3 | sudo tee /proc/sys/vm/drop_caches):\n>>>\n>>> $ time git status    > /dev/null\n>>> real    0m41.670s\n>>> user    0m0.980s\n>>> sys     0m2.908s\n>>>\n>>> $ time git status -- src/.../somedir   > /dev/null\n>>> real    0m17.380s\n>>> user    0m0.748s\n>>> sys     0m0.328s\n> [...]\n>> I can't reproduce this behavior at the moment. 'status' on the\n>> directory takes about 1.5s instead of 17s. status on whole repository\n>> takes 27s.\n>> This is my work repository, so it was changed today.\n>\n> To me these timings smell like a combination of either a network\n> filesystem or a slow/busy disk, and non-packed repositories.  Next time\n> this happens look at 'git count-objects', run 'git gc' and redo the\n> timings.\n>\n> If you are indeed on a network filesystem, also look at the\n> core.preloadIndex setting.\n\nAll is on local disk and system is idle.\n\nIndeed, after gc the times went down:\n10s -> 2.3s (subdirectory)\n17s -> 9.5s (whole repo)\n\n2 seconds is much better and I'd say acceptable for me. But my questions are:\n- why is it so slow with not packed repo?\n- can it be faster without repacking?\n- even with packed repo, the time on small subdirectory is much higher\nthan I'd expect given time on whole repo and subdirectory size - why?\n\n\n$ git count-objects -v\ncount: 5095\nsize: 37084\nin-pack: 755364\npacks: 21\nsize-pack: 2398468\nprune-packable: 0\ngarbage: 0\n\n$ git gc\nCounting objects: 760212, done.\nCompressing objects: 100% (158651/158651), done.\nWriting objects: 100% (760212/760212), done.\nTotal 760212 (delta 535848), reused 736108 (delta 513257)\nChecking connectivity: 760212, done.\n\n$ time git status -- .\nreal\t0m2.503s\nuser\t0m0.160s\nsys\t0m0.096s\n\n$ time git status\nreal\t0m9.663s\nuser\t0m0.232s\nsys\t0m0.556s\n\n\n-- \nPiotr Krukowiecki\n"},{"id":"184755","messageId":"CACsJy8DBG2A91=ppCOC6Hf6aYM2soUXXdsY0GU9hr2xqjj4RYA@mail.gmail.com","threadId":"29594","inReplyTo":"CAA01Cso_8=159UDMFUHiYz1X=gYtpbqRO4h3TMw7N=4YMV8YNg@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-15T11:01:45Z","receivedAt":"2012-02-15T11:01:45Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Feb 15, 2012 at 3:57 PM, Piotr Krukowiecki\n wrote:\n> Indeed, after gc the times went down:\n> 10s -> 2.3s (subdirectory)\n> 17s -> 9.5s (whole repo)\n>\n> 2 seconds is much better and I'd say acceptable for me. But my questions are:\n> - why is it so slow with not packed repo?\n> - can it be faster without repacking?\n\ngc does more than just repacking. If you still have the un-gc'd repo,\nTry these commands one by one, and time \"git status\" after each:\n\n - git pack-refs --all --prune\n - git reflog expire --all\n - git repack -d -l\n - git prune --expire\n - git rerere gc\n\nI'd be more interested in why auto-gc does not kick in (or whther it should).\n\n> - even with packed repo, the time on small subdirectory is much higher\n> than I'd expect given time on whole repo and subdirectory size - why?\n\nHard to say without measuring. I just notice that I missed your mail\nwith profiling results. I will have a look, but just in case, is the\nrepository publicly available?\n-- \nDuy\n"},{"id":"184763","messageId":"CAA01CsoW-32kOtD2g==2FDcYaxfSVD4Xdy1bYG_wJBWDNept4A@mail.gmail.com","threadId":"29594","inReplyTo":"CACsJy8DBG2A91=ppCOC6Hf6aYM2soUXXdsY0GU9hr2xqjj4RYA@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2012-02-15T15:14:38Z","receivedAt":"2012-02-15T15:14:38Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Wed, Feb 15, 2012 at 12:01 PM, Nguyen Thai Ngoc Duy\n wrote:\n> On Wed, Feb 15, 2012 at 3:57 PM, Piotr Krukowiecki\n> wrote:\n>> Indeed, after gc the times went down:\n>> 10s -> 2.3s (subdirectory)\n>> 17s -> 9.5s (whole repo)\n>>\n>> 2 seconds is much better and I'd say acceptable for me. But my questions are:\n>> - why is it so slow with not packed repo?\n>> - can it be faster without repacking?\n>\n> gc does more than just repacking. If you still have the un-gc'd repo,\n> Try these commands one by one, and time \"git status\" after each:\n>\n>  - git pack-refs --all --prune\n>  - git reflog expire --all\n>  - git repack -d -l\n>  - git prune --expire\n>  - git rerere gc\n\nIt will take some time but hopefully I'll have the stats for tomorrow.\n\n\n> I'd be more interested in why auto-gc does not kick in (or whther it should).\n\nI don't have any specific options set, so default values should be used.\n\nI'm using git-svn though, so my workflow looks like this:\n git svn fetch + git svn rebase\n ... git operations like commit, cherry-pick, rebase ...\n git svn dcommit\n\nNot sure if that matters. I remember that I've seen auto-gc being run\nseveral times in the past - I think after svn fetch/rebase.\n\nI'm also using git-new-workdir and have 2 extra workdirs.\n\n\n>> - even with packed repo, the time on small subdirectory is much higher\n>> than I'd expect given time on whole repo and subdirectory size - why?\n>\n> Hard to say without measuring. I just notice that I missed your mail\n> with profiling results. I will have a look, but just in case, is the\n> repository publicly available?\n\nUnfortunately it's not public. I can do some measuring if someone\ntells me what to do.\n\n\n-- \nPiotr Krukowiecki\n"},{"id":"184780","messageId":"20120215190318.GA5992@sigill.intra.peff.net","threadId":"29594","inReplyTo":"CAA01Cso_8=159UDMFUHiYz1X=gYtpbqRO4h3TMw7N=4YMV8YNg@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-15T19:03:18Z","receivedAt":"2012-02-15T19:03:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 15, 2012 at 09:57:29AM +0100, Piotr Krukowiecki wrote:\n\n> All is on local disk and system is idle.\n> \n> Indeed, after gc the times went down:\n> 10s -> 2.3s (subdirectory)\n> 17s -> 9.5s (whole repo)\n> \n> 2 seconds is much better and I'd say acceptable for me. But my questions are:\n\nObviously these answers didn't come from any deep analysis, but are\neducated guesses from me based on previous performance patterns we've\nseen on the list:\n\n> - why is it so slow with not packed repo?\n\nYour numbers show that you're I/O-bound:\n\n>>> $ time git status    > /dev/null\n>>> real    0m41.670s\n>>> user    0m0.980s\n>>> sys     0m2.908s\n>>>\n>>> $ time git status -- src/.../somedir   > /dev/null\n>>> real    0m17.380s\n>>> user    0m0.748s\n>>> sys     0m0.328s\n\nwhich is not surprising, since you said you dropped caches before-hand.\nRepacking probably reduced your disk footprint by a lot, which meant\nless I/O.\n\nI notice that you're still I/O bound even after the repack:\n\n> $ time git status -- .\n> real 0m2.503s\n> user 0m0.160s\n> sys 0m0.096s\n> \n> $ time git status\n> real 0m9.663s\n> user 0m0.232s\n> sys 0m0.556s\n\nDid you drop caches here, too? Usually that would not be the case on a\nwarm cache. If it is, then it sounds like you are short on memory to\nactually hold the directory tree and object db in cache. If not, what do\nthe warm cache numbers look like?\n\n> - can it be faster without repacking?\n\nNot really. You're showing an I/O problem, and repacking is git's way of\nreducing I/O.\n\n> - even with packed repo, the time on small subdirectory is much higher\n> than I'd expect given time on whole repo and subdirectory size - why?\n\nHard to say without profiling. It may be that we reduced the object db\nlookups, saving some time, but still end up stat()ing the whole tree.\nThe optimization to stat only the directories of interest was in 688cd6d\n(status: only touch path we may need to check, 2010-01-14), which went\ninto v1.7.0. What version of git are you using?\n\n-Peff\n"},{"id":"184823","messageId":"CAA01Csrj0FuV7OP3fCbp2QswM0u3Eph8ee6JBUd9TiyRmLZ_9Q@mail.gmail.com","threadId":"29594","inReplyTo":"CAA01CsoW-32kOtD2g==2FDcYaxfSVD4Xdy1bYG_wJBWDNept4A@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2012-02-16T13:22:23Z","receivedAt":"2012-02-16T13:22:23Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Wed, Feb 15, 2012 at 4:14 PM, Piotr Krukowiecki\n wrote:\n> On Wed, Feb 15, 2012 at 12:01 PM, Nguyen Thai Ngoc Duy\n> wrote:\n>> On Wed, Feb 15, 2012 at 3:57 PM, Piotr Krukowiecki\n>> wrote:\n>>> Indeed, after gc the times went down:\n>>> 10s -> 2.3s (subdirectory)\n>>> 17s -> 9.5s (whole repo)\n>>>\n>>> 2 seconds is much better and I'd say acceptable for me. But my questions are:\n>>> - why is it so slow with not packed repo?\n>>> - can it be faster without repacking?\n>>\n>> gc does more than just repacking. If you still have the un-gc'd repo,\n>> Try these commands one by one, and time \"git status\" after each:\n>>\n>>  - git pack-refs --all --prune\n>>  - git reflog expire --all\n>>  - git repack -d -l\n>>  - git prune --expire\n>>  - git rerere gc\n>\n> It will take some time but hopefully I'll have the stats for tomorrow.\n\nHere they are. I did 'status' three times to get reliable results and\nbefore each run have dropped caches. Backed up repository was copied\nbefore each 'status'. Full log is at http://pastebin.com/VmB7J9CJ\n\ngit version 1.7.9.rc0.10.gbeecc\n\nResults after each command:\n\nstatus on whole repo:\n18.5s - after git count-objects -v\n16.0s - after git pack-refs --all --prune\n20.2s - after git reflog expire --all\n13.0s - after git repack -d -l\n16.8s - after git prune --expire now\n19.7s - after git rerere gc\n\nstatus on subdir:\n9.7s - after git count-objects -v\n9.2s - after git pack-refs --all --prune\n9.3s - after git reflog expire --all\n4.4s - after git repack -d -l\n9.2s - after git prune --expire now\n9.0s - after git rerere gc\n\n\n-- \nPiotr Krukowiecki\n"},{"id":"184824","messageId":"CAA01Cso5y23UMguEe0vwOc6kR3-DjuC8-LTMDsMeeOKU4rVGvg@mail.gmail.com","threadId":"29594","inReplyTo":"20120215190318.GA5992@sigill.intra.peff.net","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2012-02-16T13:37:47Z","receivedAt":"2012-02-16T13:37:47Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Wed, Feb 15, 2012 at 8:03 PM, Jeff King wrote:\n> On Wed, Feb 15, 2012 at 09:57:29AM +0100, Piotr Krukowiecki wrote:\n>>\n> I notice that you're still I/O bound even after the repack:\n>\n>> $ time git status  -- .\n>> real    0m2.503s\n>> user    0m0.160s\n>> sys     0m0.096s\n>>\n>> $ time git status\n>> real    0m9.663s\n>> user    0m0.232s\n>> sys     0m0.556s\n>\n> Did you drop caches here, too?\n\nYes I did - with cache the status takes something like 0.1-0.3s on whole repo.\n\n\n>  Usually that would not be the case on a\n> warm cache. If it is, then it sounds like you are short on memory to\n> actually hold the directory tree and object db in cache. If not, what do\n> the warm cache numbers look like?\n\nI've got 4GB of ram and I did not hit the swap when doing last\nperformance tests AFAIK.\nPlease see my previous posts for performance results with warm cache\nand profile results:\n\nhttp://article.gmane.org/gmane.comp.version-control.git/190397\nhttp://article.gmane.org/gmane.comp.version-control.git/190638\n\n\n>> - can it be faster without repacking?\n>\n> Not really. You're showing an I/O problem, and repacking is git's way of\n> reducing I/O.\n\nSo if I understand correctly, the reason is because git must compare\nworkspace files with packed objects - and the problem is\nreading/seeking/searching in the packs?\n\nIs there a way to make packs better? I think most operations are on\nworkdir files - so maybe it'd be possible to tell gc/repack/whatever\nto optimize access to files which I currently have in workdir?\n\n\n>> - even with packed repo, the time on small subdirectory is much higher\n>> than I'd expect given time on whole repo and subdirectory size - why?\n>\n> Hard to say without profiling.  It may be that we reduced the object db\n> lookups, saving some time, but still end up stat()ing the whole tree.\n> The optimization to stat only the directories of interest was in 688cd6d\n> (status: only touch path we may need to check, 2010-01-14), which went\n> into v1.7.0. What version of git are you using?\n\nFor latest tests I've used 1.7.9.rc0.10.gbeecc, for profiling - 1.7.9.188.g12766\n\nIs there anything else I could do?\n\n\n-- \nPiotr Krukowiecki\n"},{"id":"184826","messageId":"87d39eswkx.fsf@thomas.inf.ethz.ch","threadId":"29594","inReplyTo":"CAA01Cso5y23UMguEe0vwOc6kR3-DjuC8-LTMDsMeeOKU4rVGvg@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-02-16T14:05:02Z","receivedAt":"2012-02-16T14:05:02Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Piotr Krukowiecki writes:\n\n> On Wed, Feb 15, 2012 at 8:03 PM, Jeff King wrote:\n>> On Wed, Feb 15, 2012 at 09:57:29AM +0100, Piotr Krukowiecki wrote:\n>>>\n>> I notice that you're still I/O bound even after the repack:\n>>\n>>> $ time git status  -- .\n>>> real    0m2.503s\n>>> user    0m0.160s\n>>> sys     0m0.096s\n>>>\n>>> $ time git status\n>>> real    0m9.663s\n>>> user    0m0.232s\n>>> sys     0m0.556s\n>>\n>> Did you drop caches here, too?\n>\n> Yes I did - with cache the status takes something like 0.1-0.3s on whole repo.\n\nSo umm, I'm not sure that leaves anything to be improved.\n\nI looked at some strace dumps, and limiting the status to a subdirectory\n(in my case, '-- t' in git.git) does omit the lstat()s on uninteresting\nparts of the index-listed files, as well as the getdents() (i.e.,\nreaddir()) for parts of the tree that are not interesting.\n\nBTW, some other parts of git-status's display may be responsible for the\namount of data it pulls from disk. In particular, the \"Your branch is\nahead\" display requires computing the merge-base between HEAD and\n@{upstream}. If your @{upstream} is way ahead/behind, or points at a\ndisjoint chunk of history, this may mean essentially pulling all of the\ninvolved history from disk. If my memory of pack organization serves\nright, the commit objects involved would essentially be spread across\nthe whole pack (corresponding to \"time\") and thus this operation would\nmore or less load the entire pack from disk.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"184830","messageId":"20120216192001.GB4348@sigill.intra.peff.net","threadId":"29594","inReplyTo":"CAA01Cso5y23UMguEe0vwOc6kR3-DjuC8-LTMDsMeeOKU4rVGvg@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-16T19:20:02Z","receivedAt":"2012-02-16T19:20:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 16, 2012 at 02:37:47PM +0100, Piotr Krukowiecki wrote:\n\n> >> $ time git status  -- .\n> >> real    0m2.503s\n> >> user    0m0.160s\n> >> sys     0m0.096s\n> >>\n> >> $ time git status\n> >> real    0m9.663s\n> >> user    0m0.232s\n> >> sys     0m0.556s\n> >\n> > Did you drop caches here, too?\n> \n> Yes I did - with cache the status takes something like 0.1-0.3s on whole repo.\n\nOK, then that makes sense. It's pretty much just I/O on the filesystem\nand on the object db.\n\nYou can break status down a little more to see which is which. Try \"git\nupdate-index --refresh\" to see just how expensive the lstat and index\nhandling is.\n\nAnd then try \"git diff-index HEAD\" for an idea of how expensive it is to\njust read the objects and compare to the index.\n\n> > Not really. You're showing an I/O problem, and repacking is git's way of\n> > reducing I/O.\n> \n> So if I understand correctly, the reason is because git must compare\n> workspace files with packed objects - and the problem is\n> reading/seeking/searching in the packs?\n\nMostly reading (we keep a sorted index and access the packfiles via\nmmap, so we only touch the pages we need). But you're also paying to\nlstat() the directory tree, too. And you're paying to load (probably)\nthe whole index into memory, although it's relatively compact compared\nto the actual file data.\n\n> Is there a way to make packs better? I think most operations are on\n> workdir files - so maybe it'd be possible to tell gc/repack/whatever\n> to optimize access to files which I currently have in workdir?\n\nIt already does optimize for that case. If you can make it even better,\nI'm sure people would be happy to see the numbers.\n\nMostly I think it is just the case that disk I/O is slow, and the\noperation you're asking for has to do a certain amount of it. What kind\nof disk/filesystem are you pulling off of?\n\nIt's not a fuse filesystem by any chance, is it? I have a repo on an\nencfs-mounted filesystem, and the lstat times are absolutely horrific.\n\n-Peff\n"},{"id":"184835","messageId":"7vwr7my1pf.fsf@alter.siamese.dyndns.org","threadId":"29594","inReplyTo":"87d39eswkx.fsf@thomas.inf.ethz.ch","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-16T20:15:24Z","receivedAt":"2012-02-16T20:15:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> ... If my memory of pack organization serves\n> right, the commit objects involved would essentially be spread across\n> the whole pack\n\nNo they are crumped together in a contiguous section in a packfile, so\nthat \"git log\" without any pathspec can go faster without consulting tree\nobjects.\n"},{"id":"184888","messageId":"CAA01CsqfM4=cHBuJ5faPuWHPcHKAeO7fRQdSQzaFLZMdQaVGrA@mail.gmail.com","threadId":"29594","inReplyTo":"87d39eswkx.fsf@thomas.inf.ethz.ch","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2012-02-17T16:55:03Z","receivedAt":"2012-02-17T16:55:03Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Thu, Feb 16, 2012 at 3:05 PM, Thomas Rast wrote:\n> Piotr Krukowiecki writes:\n>\n>> On Wed, Feb 15, 2012 at 8:03 PM, Jeff King wrote:\n>>> On Wed, Feb 15, 2012 at 09:57:29AM +0100, Piotr Krukowiecki wrote:\n>>>>\n>>> I notice that you're still I/O bound even after the repack:\n>>>\n>>>> $ time git status  -- .\n>>>> real    0m2.503s\n>>>> user    0m0.160s\n>>>> sys     0m0.096s\n>>>>\n>>>> $ time git status\n>>>> real    0m9.663s\n>>>> user    0m0.232s\n>>>> sys     0m0.556s\n>>>\n>>> Did you drop caches here, too?\n>>\n>> Yes I did - with cache the status takes something like 0.1-0.3s on whole repo.\n>\n> So umm, I'm not sure that leaves anything to be improved.\n\nBut even with caches time on small directory is only half of time on whole repo:\n0.15s vs 0.07s\n\n\n> I looked at some strace dumps, and limiting the status to a subdirectory\n> (in my case, '-- t' in git.git) does omit the lstat()s on uninteresting\n> parts of the index-listed files, as well as the getdents() (i.e.,\n> readdir()) for parts of the tree that are not interesting.\n\nSame in my case.\n\nI've run strace with -c which shows system calls times. As I\nunderstand the results the time used by lstat() is very small. With\ndropped caches:\n\n% time seconds usecs/call calls errors syscall\n------ ----------- ----------- --------- --------- ----------------\n 90.70 0.065108 3 25361 12 lstat\n 6.78 0.004869 1 6534 getdents\n 1.94 0.001392 0 6964 3238 open\n 0.27 0.000194 0 3726 close\n\n\n> BTW, some other parts of git-status's display may be responsible for the\n> amount of data it pulls from disk.  In particular, the \"Your branch is\n> ahead\" display requires computing the merge-base between HEAD and\n> @{upstream}.  If your @{upstream} is way ahead/behind, or points at a\n> disjoint chunk of history, this may mean essentially pulling all of the\n> involved history from disk.  If my memory of pack organization serves\n> right, the commit objects involved would essentially be spread across\n> the whole pack (corresponding to \"time\") and thus this operation would\n> more or less load the entire pack from disk.\n\nI don't think this is the case - I'm using git-svn and thus have no\nupstream in git meaning.\n\n\n-- \nPiotr Krukowiecki\n"},{"id":"184891","messageId":"CAA01Csq6vSekW=Fa236bB0H3LVtN43Gb2aLMVE+A1wVyUqYJ7A@mail.gmail.com","threadId":"29594","inReplyTo":"20120216192001.GB4348@sigill.intra.peff.net","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2012-02-17T17:19:06Z","receivedAt":"2012-02-17T17:19:06Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Thu, Feb 16, 2012 at 8:20 PM, Jeff King wrote:\n> On Thu, Feb 16, 2012 at 02:37:47PM +0100, Piotr Krukowiecki wrote:\n>\n>> >> $ time git status  -- .\n>> >> real    0m2.503s\n>> >> user    0m0.160s\n>> >> sys     0m0.096s\n>> >>\n>> >> $ time git status\n>> >> real    0m9.663s\n>> >> user    0m0.232s\n>> >> sys     0m0.556s\n>> >\n>> > Did you drop caches here, too?\n>>\n>> Yes I did - with cache the status takes something like 0.1-0.3s on whole repo.\n>\n> OK, then that makes sense. It's pretty much just I/O on the filesystem\n> and on the object db.\n>\n> You can break status down a little more to see which is which. Try \"git\n> update-index --refresh\" to see just how expensive the lstat and index\n> handling is.\n\n\"git update-index --refresh\" with dropped cache took\nreal\t0m3.726s\nuser\t0m0.024s\nsys\t0m0.404s\n\nwhile \"git status\" with dropped cache takes\nreal\t0m13.578s\nuser\t0m0.240s\nsys\t0m0.600s\n\nI'm not sure why it takes more than the 9s reported before - IIRC I\ndid the previous test in single mode under bare shell and this time\nI'm testing under gnome. This or it's the effect of running\nupdate-index :/\nNow status on subdir takes 9.5s. But still the\nnot-much-faster-status-on-subdir rule is true.\n\n\n> And then try \"git diff-index HEAD\" for an idea of how expensive it is to\n> just read the objects and compare to the index.\n\nThe diff-index after dropping cache takes\nreal\t0m14.095s\nuser\t0m0.268s\nsys\t0m0.564s\n\n\n>> > Not really. You're showing an I/O problem, and repacking is git's way of\n>> > reducing I/O.\n>>\n>> So if I understand correctly, the reason is because git must compare\n>> workspace files with packed objects - and the problem is\n>> reading/seeking/searching in the packs?\n>\n> Mostly reading (we keep a sorted index and access the packfiles via\n> mmap, so we only touch the pages we need). But you're also paying to\n> lstat() the directory tree, too. And you're paying to load (probably)\n> the whole index into memory, although it's relatively compact compared\n> to the actual file data.\n\nIf the index is the objects/pack/*.idx files than it's 21MB\n\n\n>> Is there a way to make packs better? I think most operations are on\n>> workdir files - so maybe it'd be possible to tell gc/repack/whatever\n>> to optimize access to files which I currently have in workdir?\n>\n> It already does optimize for that case. If you can make it even better,\n> I'm sure people would be happy to see the numbers.\n\nIf I understand correctly, you only need to compute sha1 on the\nworkdir files and compare it with sha1 files recorded in index/gitdir.\nIt seems that to get the sha1 from index/gitdir I need to read the\npackfiles? Maybe it'd be possible to cache/index it somehow, for\nexample in separate and smaller file?\n\n\n> Mostly I think it is just the case that disk I/O is slow, and the\n> operation you're asking for has to do a certain amount of it. What kind\n> of disk/filesystem are you pulling off of?\n>\n> It's not a fuse filesystem by any chance, is it? I have a repo on an\n> encfs-mounted filesystem, and the lstat times are absolutely horrific.\n\nNo, it's ext4 and the disk Seagate Barracuda 7200.12 500GB, as it\nreads on the cover :)\n\nBut IMO faster disk won't help with this - times will be smaller, but\nyou'll still have to read the same data, so the subdir times will be\njust 2x faster than whole repo, won't it? So maybe in my case it will\ngo down to e.g. 2s on subdir, but for someone with larger repository\nit will still be 10s...\n\n\n-- \nPiotr Krukowiecki\n"},{"id":"184905","messageId":"20120217203755.GA30114@sigill.intra.peff.net","threadId":"29594","inReplyTo":"CAA01Csq6vSekW=Fa236bB0H3LVtN43Gb2aLMVE+A1wVyUqYJ7A@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-17T20:37:55Z","receivedAt":"2012-02-17T20:37:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 17, 2012 at 06:19:06PM +0100, Piotr Krukowiecki wrote:\n\n> \"git update-index --refresh\" with dropped cache took\n> real\t0m3.726s\n> user\t0m0.024s\n> sys\t0m0.404s\n> [...]\n> The diff-index after dropping cache takes\n> real\t0m14.095s\n> user\t0m0.268s\n> sys\t0m0.564s\n\nOK, that suggests to me that the real culprit is the I/O we spend in\naccessing the object db, since that is the main I/O that happens in the\nsecond command but not the first.\n\n> > Mostly reading (we keep a sorted index and access the packfiles via\n> > mmap, so we only touch the pages we need). But you're also paying to\n> > lstat() the directory tree, too. And you're paying to load (probably)\n> > the whole index into memory, although it's relatively compact compared\n> > to the actual file data.\n> \n> If the index is the objects/pack/*.idx files than it's 21MB\n\nYes, that's it. Though we don't necessarily read the whole thing. The\nsorted list of sha1s is only a part of that. And we mmap and\nbinary-search that, so we only have to fault in pages that are actually\nused in our binary search.\n\nHowever, we're faulting in random pages of the index in series, so it\nmay actually have a lot of latency. You can see how expensive the I/O on\nthe index is with something like this:\n\n [whole operation, for reference]\n $ sudo sh -c 'echo 3 >/proc/sys/vm/drop_caches'\n $ time git diff-index HEAD\n real 0m2.636s\n user 0m0.248s\n sys 0m0.392s\n\n [prime the cache with just the index]\n $ sudo sh -c 'echo 3 >/proc/sys/vm/drop_caches'\n $ time cat .git/objects/pack/*.idx >/dev/null\n real 0m0.288s\n user 0m0.000s\n sys 0m0.028s\n $ time git diff-index HEAD\n real 0m2.175s\n user 0m0.272s\n sys 0m0.320s\n\nSo roughly 20% of the I/O time in my case went to faulting in the index.\nYou could pre-fault in the index, which would give the OS a chance to do\nread-ahead caching. You can see that the combined cat and diff-index\ntimes are still lower than the raw diff-index time. You could also do\nthem in parallel, but that will create some additional seeks as the\nthreads fight for the disk, but may be a win in the long run because we\ncan read bigger chunks. You can roughly simulate it by running the \"cat\"\nand the \"diff-index\" above in parallel. I get:\n\n real 0m2.464s\n user 0m0.284s\n sys 0m0.372s\n\nwhich is almost exactly the same as doing them separately (though note\nthat this is on an SSD, so seeking is very cheap).\n\nBut the bulk of the time still goes to actually retrieving the object\ndata, so that's probably a more interesting area to focus, anyway (and\nif we can reduce object accesses, we reduce their lookup, too :) ).\n\n> If I understand correctly, you only need to compute sha1 on the\n> workdir files and compare it with sha1 files recorded in index/gitdir.\n> It seems that to get the sha1 from index/gitdir I need to read the\n> packfiles? Maybe it'd be possible to cache/index it somehow, for\n> example in separate and smaller file?\n\nThere are two diffs going on in \"git status\". One is a comparison\nbetween index and worktree. In that one, you need to lstat each file to\nmake sure the cached sha1 we have in the index is up to date. Assuming\nit is, you don't need to touch the file data at all. Then you compare\nthat sha1 to the stage 0 sha1 (i.e., what we typically think of as\n\"staged for commit\"). If they match, you don't need to do more work.\n\nBut the expensive diff-index we've been doing above is comparing the\nindex to the HEAD tree. And doing that is a little trickier. The index\nis a flat list of files with their sha1s. But the HEAD tree is stored\nhierarchically. So to get the sha1 of foo/bar/baz, we have to access the\nroot tree object, find the \"foo\" entry, access its tree object, find the\n\"bar\" entry, access its tree object, and then find the \"baz\" entry. Then\nwe compare the sha1 of the \"baz\" entry to what's in the index.\n\nSo what's where your I/O comes from: accessing each of the tree objects.\nAnd that fact that it isn't just \"compare the HEAD and index sha1s\" is\nthat the index is stored as a list of flat files.\n\nThat being said, we do have an index extension to store the tree sha1 of\nwhole directories (i.e., we populate it when we write a whole tree or\nsubtree into the index from the object db, and it becomes invalidated\nwhen a file becomes modified). This optimization is used by things like\n\"git commit\" to avoid having to recreate the same sub-trees over and\nover when creating tree objects from the index. But we could also use it\nhere to avoid having to even read the sub-tree objects from the object\ndb.\n\n> No, it's ext4 and the disk Seagate Barracuda 7200.12 500GB, as it\n> reads on the cover :)\n> \n> But IMO faster disk won't help with this - times will be smaller, but\n> you'll still have to read the same data, so the subdir times will be\n> just 2x faster than whole repo, won't it? So maybe in my case it will\n> go down to e.g. 2s on subdir, but for someone with larger repository\n> it will still be 10s...\n\nSure. But a certain amount of I/O is going to be unavoidable to get the\nanswer to your question. So you will never be able to achieve the\nwarm-cache case. I'm not saying we can't improve (e.g., I think the\nindex extension thing I mentioned above is a promising approach). But we\nhave to be realistic about what will make things faster; if I/O is your\nproblem, faster disk is one possible solution (especially because some\nof this is related to seeking and latency, an SSD is a nice improvement\nfor cold-cache times).\n\n-Peff\n"},{"id":"184907","messageId":"7vaa4hrtbe.fsf@alter.siamese.dyndns.org","threadId":"29594","inReplyTo":"20120217203755.GA30114@sigill.intra.peff.net","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-17T22:25:25Z","receivedAt":"2012-02-17T22:25:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> That being said, we do have an index extension to store the tree sha1 of\n> whole directories (i.e., we populate it when we write a whole tree or\n> subtree into the index from the object db, and it becomes invalidated\n> when a file becomes modified). This optimization is used by things like\n> \"git commit\" to avoid having to recreate the same sub-trees over and\n> over when creating tree objects from the index. But we could also use it\n> here to avoid having to even read the sub-tree objects from the object\n> db.\n\nLike b65982b (Optimize \"diff-index --cached\" using cache-tree, 2009-05-20)\nperhaps?\n"},{"id":"184908","messageId":"20120217222912.GC31830@sigill.intra.peff.net","threadId":"29594","inReplyTo":"7vaa4hrtbe.fsf@alter.siamese.dyndns.org","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-17T22:29:12Z","receivedAt":"2012-02-17T22:29:12Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 17, 2012 at 02:25:25PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > That being said, we do have an index extension to store the tree sha1 of\n> > whole directories (i.e., we populate it when we write a whole tree or\n> > subtree into the index from the object db, and it becomes invalidated\n> > when a file becomes modified). This optimization is used by things like\n> > \"git commit\" to avoid having to recreate the same sub-trees over and\n> > over when creating tree objects from the index. But we could also use it\n> > here to avoid having to even read the sub-tree objects from the object\n> > db.\n> \n> Like b65982b (Optimize \"diff-index --cached\" using cache-tree, 2009-05-20)\n> perhaps?\n\nThat's what I get for speaking before running \"git log\".\n\nSo yeah, we may be about as reasonably fast as we can go. Or maybe that\noptimization isn't kicking in for some reason. I think going further\nwould require Piotr to do more profiling.\n\n-Peff\n"},{"id":"184972","messageId":"CAA01CsozANwtox06iihKBL8iii175FHAhChmNhG1B0ofGKWcEA@mail.gmail.com","threadId":"29594","inReplyTo":"20120217222912.GC31830@sigill.intra.peff.net","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2012-02-20T08:25:00Z","receivedAt":"2012-02-20T08:25:00Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Fri, Feb 17, 2012 at 11:29 PM, Jeff King wrote:\n> On Fri, Feb 17, 2012 at 02:25:25PM -0800, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>>\n>> > That being said, we do have an index extension to store the tree sha1 of\n>> > whole directories (i.e., we populate it when we write a whole tree or\n>> > subtree into the index from the object db, and it becomes invalidated\n>> > when a file becomes modified). This optimization is used by things like\n>> > \"git commit\" to avoid having to recreate the same sub-trees over and\n>> > over when creating tree objects from the index. But we could also use it\n>> > here to avoid having to even read the sub-tree objects from the object\n>> > db.\n>>\n>> Like b65982b (Optimize \"diff-index --cached\" using cache-tree, 2009-05-20)\n>> perhaps?\n>\n> That's what I get for speaking before running \"git log\".\n>\n> So yeah, we may be about as reasonably fast as we can go. Or maybe that\n> optimization isn't kicking in for some reason. I think going further\n> would require Piotr to do more profiling.\n\nIs the cache set? Not sure how to check it. t0090-cache-tree.sh uses\ntest-dump-cache-tree and executes \"read-tree HEAD\" to establish the\ncache, but in my case read-tree does not make the cache dumpable (but\nit improves status performance).\n\n$ test-dump-cache-tree | wc -l\n0\n$ git read-tree HEAD\n$ test-dump-cache-tree | wc -l\n0\n$ echo 3 | sudo tee /proc/sys/vm/drop_caches && time git status -- .\n[...]\nreal\t0m1.085s\n\n\ngit version 1.7.9.188.g12766\n\n-- \nPiotr Krukowiecki\n"},{"id":"184991","messageId":"20120220140653.GC5131@sigill.intra.peff.net","threadId":"29594","inReplyTo":"CAA01CsozANwtox06iihKBL8iii175FHAhChmNhG1B0ofGKWcEA@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-20T14:06:53Z","receivedAt":"2012-02-20T14:06:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 20, 2012 at 09:25:00AM +0100, Piotr Krukowiecki wrote:\n\n> Is the cache set? Not sure how to check it. t0090-cache-tree.sh uses\n> test-dump-cache-tree and executes \"read-tree HEAD\" to establish the\n> cache, but in my case read-tree does not make the cache dumpable (but\n> it improves status performance).\n> \n> $ test-dump-cache-tree | wc -l\n> 0\n> $ git read-tree HEAD\n> $ test-dump-cache-tree | wc -l\n> 0\n> $ echo 3 | sudo tee /proc/sys/vm/drop_caches && time git status -- .\n> [...]\n> real\t0m1.085s\n\nHmm. I would think test-dump-cache-tree would do it. I don't know why\nread-tree wouldn't fill it in, though.\n\nInterestingly, on my git.git repo, I had an empty cache. Running \"git\nread-tree HEAD\" filled it (according to test-dump-cache-tree). It seems\nthat running \"git checkout\" empties the cache. So perhaps git could do\nbetter about keeping the cache valid over time.\n\n-Peff\n"},{"id":"184992","messageId":"87ty2l38ay.fsf@thomas.inf.ethz.ch","threadId":"29594","inReplyTo":"20120220140653.GC5131@sigill.intra.peff.net","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-02-20T14:09:57Z","receivedAt":"2012-02-20T14:09:57Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King writes:\n\n> On Mon, Feb 20, 2012 at 09:25:00AM +0100, Piotr Krukowiecki wrote:\n>\n>> Is the cache set? Not sure how to check it. t0090-cache-tree.sh uses\n>> test-dump-cache-tree and executes \"read-tree HEAD\" to establish the\n>> cache, but in my case read-tree does not make the cache dumpable (but\n>> it improves status performance).\n>> \n>> $ test-dump-cache-tree | wc -l\n>> 0\n>> $ git read-tree HEAD\n>> $ test-dump-cache-tree | wc -l\n>> 0\n>> $ echo 3 | sudo tee /proc/sys/vm/drop_caches && time git status -- .\n>> [...]\n>> real\t0m1.085s\n>\n> Hmm. I would think test-dump-cache-tree would do it. I don't know why\n> read-tree wouldn't fill it in, though.\n>\n> Interestingly, on my git.git repo, I had an empty cache. Running \"git\n> read-tree HEAD\" filled it (according to test-dump-cache-tree). It seems\n> that running \"git checkout\" empties the cache. So perhaps git could do\n> better about keeping the cache valid over time.\n\ntest_expect_failure 'checkout gives cache-tree' '\n\tgit checkout HEAD^ &&\n\ttest_shallow_cache_tree\n'\n\n;-)\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"184993","messageId":"CACsJy8AHEHDAa2v4DvNgwd1YsBQuRCL9bHaxF70L8O4yWc4gZg@mail.gmail.com","threadId":"29594","inReplyTo":"20120220140653.GC5131@sigill.intra.peff.net","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-20T14:16:43Z","receivedAt":"2012-02-20T14:16:43Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Feb 20, 2012 at 9:06 PM, Jeff King wrote:\n> Interestingly, on my git.git repo, I had an empty cache. Running \"git\n> read-tree HEAD\" filled it (according to test-dump-cache-tree). It seems\n> that running \"git checkout\" empties the cache.  So perhaps git could do\n> better about keeping the cache valid over time.\n\nFor fast forward case when result index matches 100% destination tree,\nyeah we should repopulate cache-tree. \"git reset\" does that. Not sure\nabout other cases though. I don't think we can keep track what\nsubtrees are unchanged after unpack_trees() in order to keep them.\n-- \nDuy\n"},{"id":"184995","messageId":"20120220142206.GA5775@sigill.intra.peff.net","threadId":"29594","inReplyTo":"CACsJy8AHEHDAa2v4DvNgwd1YsBQuRCL9bHaxF70L8O4yWc4gZg@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-20T14:22:06Z","receivedAt":"2012-02-20T14:22:06Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 20, 2012 at 09:16:43PM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> On Mon, Feb 20, 2012 at 9:06 PM, Jeff King wrote:\n> > Interestingly, on my git.git repo, I had an empty cache. Running \"git\n> > read-tree HEAD\" filled it (according to test-dump-cache-tree). It seems\n> > that running \"git checkout\" empties the cache.  So perhaps git could do\n> > better about keeping the cache valid over time.\n> \n> For fast forward case when result index matches 100% destination tree,\n> yeah we should repopulate cache-tree. \"git reset\" does that. Not sure\n> about other cases though. I don't think we can keep track what\n> subtrees are unchanged after unpack_trees() in order to keep them.\n\nYeah, doing it after unpack_trees seems crazy. But I really feel like\nunpack_trees should be able to handle cache updates as it unpacks. It\nknows what is being updated and what is being merged. But maybe it is\nmore complicated than that; I haven't looked at the code yet.\n\n-Peff\n"},{"id":"184996","messageId":"20120220143644.GA13938@do","threadId":"29594","inReplyTo":"87ty2l38ay.fsf@thomas.inf.ethz.ch","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-20T14:36:44Z","receivedAt":"2012-02-20T14:36:44Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Feb 20, 2012 at 03:09:57PM +0100, Thomas Rast wrote:\n> > Interestingly, on my git.git repo, I had an empty cache. Running \"git\n> > read-tree HEAD\" filled it (according to test-dump-cache-tree). It seems\n> > that running \"git checkout\" empties the cache. So perhaps git could do\n> > better about keeping the cache valid over time.\n> \n> test_expect_failure 'checkout gives cache-tree' '\n> \tgit checkout HEAD^ &&\n> \ttest_shallow_cache_tree\n> '\n> \n> ;-)\n\nQuick and dirty that passes that test. I think we could do better if\nwe analyse two way merge rules carefully and avoid this diff, but\nthat's too much for me right now.\n\n-- 8< --\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 5bf96ba..c06287a 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -319,6 +319,10 @@ static void show_local_changes(struct object *head, struct diff_options *opts)\n \t\tdie(_(\"diff_setup_done failed\"));\n \tadd_pending_object(&rev, head, NULL);\n \trun_diff_index(&rev, 0);\n+\tif (!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES)) {\n+\t\tstruct tree *tree = parse_tree_indirect(head->sha1);\n+\t\tprime_cache_tree(&active_cache_tree, tree);\n+\t}\n }\n \n static void describe_detached_head(const char *msg, struct commit *commit)\n@@ -493,13 +497,13 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\t}\n \t}\n \n+\tif (!opts->force && !opts->quiet)\n+\t\tshow_local_changes(&new->commit->object, &opts->diff_options);\n+\n \tif (write_cache(newfd, active_cache, active_nr) ||\n \t commit_locked_index(lock_file))\n \t\tdie(_(\"unable to write new index file\"));\n \n-\tif (!opts->force && !opts->quiet)\n-\t\tshow_local_changes(&new->commit->object, &opts->diff_options);\n-\n \treturn 0;\n }\n \n-- 8< --\n-- \nDuy\n"},{"id":"184997","messageId":"20120220143952.GA8387@sigill.intra.peff.net","threadId":"29594","inReplyTo":"20120220143644.GA13938@do","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-20T14:39:52Z","receivedAt":"2012-02-20T14:39:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 20, 2012 at 09:36:44PM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> > test_expect_failure 'checkout gives cache-tree' '\n> > \tgit checkout HEAD^ &&\n> > \ttest_shallow_cache_tree\n> > '\n> > \n> > ;-)\n> \n> Quick and dirty that passes that test. I think we could do better if\n> we analyse two way merge rules carefully and avoid this diff, but\n> that's too much for me right now.\n\nUnpack trees is already sprinkled with cache_tree_invalidate_path. But\nsomething seems to throw away the cache tree entirely (I think it may be\nthat the extension simply isn't copied over to the destination index).\nI'm walking through it right now.\n\n-Peff\n"},{"id":"184998","messageId":"20120220151134.GA13135@sigill.intra.peff.net","threadId":"29594","inReplyTo":"20120220143952.GA8387@sigill.intra.peff.net","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-20T15:11:34Z","receivedAt":"2012-02-20T15:11:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 20, 2012 at 09:39:52AM -0500, Jeff King wrote:\n\n> On Mon, Feb 20, 2012 at 09:36:44PM +0700, Nguyen Thai Ngoc Duy wrote:\n> \n> > > test_expect_failure 'checkout gives cache-tree' '\n> > > \tgit checkout HEAD^ &&\n> > > \ttest_shallow_cache_tree\n> > > '\n> > > \n> > > ;-)\n> > \n> > Quick and dirty that passes that test. I think we could do better if\n> > we analyse two way merge rules carefully and avoid this diff, but\n> > that's too much for me right now.\n> \n> Unpack trees is already sprinkled with cache_tree_invalidate_path. But\n> something seems to throw away the cache tree entirely (I think it may be\n> that the extension simply isn't copied over to the destination index).\n> I'm walking through it right now.\n\nHmm. OK, this doesn't pass the test, but I think it is better than the\ncurrent behavior.\n\nBasically, what happens now with \"git checkout\" is this:\n\n 1. read_cache pulls the cache_tree from disk into the_index\n\n 2. we call unpack_trees with o->src_index == o->dst_index ==\n &the_index.\n\n 3. during tree traversal, unpack_trees callbacks properly calls\n cache_tree_invalidate_path whenever it updates a path. We write the\n results into o->result.\n\n 4. At the end of unpack_trees, we forget about src_index, and copy\n o->result into *o->dst_index byte for byte. I.e., we overwrite\n the_index.cache_tree, which has been properly updated the whole\n time, with NULL, dropping it entirely (in fact, I believe it even\n creates a memory leak of the old cache_tree).\n\nThis one-liner improves that a bit:\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 8be3f6c..e8aedea 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1135,6 +1135,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\t}\n \t}\n \n+\to->result.cache_tree = o->src_index->cache_tree;\n \to->src_index = NULL;\n \tret = check_updates(o) ? (-2) : 0;\n \tif (o->dst_index)\n\nby copying the cache_tree (which has been updated all along) from\nsrc_index into the result (which will then make it into the\ndestination index, which of course in this case is the same as the\nsource index, anyway).\n\nIt makes \"git checkout\" with no changes just work (since we preserve the\ncache tree, and it doesn't need updated). It makes something like \"git\ncheckout HEAD^\" work, keeping most of the cache-tree intact, but\ninvalidating trees containing paths that were modified.\n\nBut it does not actually insert the _destination_ tree into the cache\ntree. Which we can do in certain situations, but only if there were no\npaths in the tree that were left unchanged (e.g., you modify \"foo\", then\n\"git checkout HEAD^\", which updates \"bar\". Your tree does not match\nHEAD^, and must be invalidated). While it would be cool to be able to\nhandle those complex cases, making this one simple change covers most of\nthe cases people care about (i.e., leaving the cache-tree intact for all\nof the directories that weren't touched at all).\n\nI think this implementation matches the intent of the original calls to\ncache_tree_invalidate_path sprinkled throughout unpack-trees.c. But I\nhave to say that it seems a little odd for us to be modifying the\no->src_index throughout the whole thing. I would think the right thing\nwould be to make a deep copy of o->src_index->cache_tree into\no->result.cache_tree as the very first thing, and then update\no->result.cache_tree throughout the tree traversal. There is no point in\ninvalidating src_index's cache_tree, since it is not receiving the\nupdates.\n\nIn practice, this doesn't tend to matter because everybody just sets src\nand dst to &the_index anyway. The one exception seems to be diff_cache,\nwhich sets dst_index to NULL. But it doesn't matter there, because we\nare only using oneway_diff as our callback, which does not actually\nwrite or invalidate anything in the cache.\n\n-Peff\n"},{"id":"185004","messageId":"87d3991gyg.fsf@thomas.inf.ethz.ch","threadId":"29594","inReplyTo":"20120220151134.GA13135@sigill.intra.peff.net","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-02-20T18:45:59Z","receivedAt":"2012-02-20T18:45:59Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King writes:\n\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 8be3f6c..e8aedea 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -1135,6 +1135,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n> \t\t}\n> \t}\n> \n> +\to->result.cache_tree = o->src_index->cache_tree;\n> \to->src_index = NULL;\n> \tret = check_updates(o) ? (-2) : 0;\n> \tif (o->dst_index)\n\nBrilliant. I know I'm stealing Junio's punchline, but please make it so\n:-)\n\nBrowsing around in history, it seems that this was silently broken by\n34110cd (Make 'unpack_trees()' have a separate source and destination\nindex, 2008-03-06), which introduced the distinction between source and\ndestination index. Before that they were the same, so the cache tree\nwould have been updated correctly.\n\n> It makes \"git checkout\" with no changes just work (since we preserve the\n> cache tree, and it doesn't need updated). It makes something like \"git\n> checkout HEAD^\" work, keeping most of the cache-tree intact, but\n> invalidating trees containing paths that were modified.\n\nGreat. Here's a test you could use. It's a bit noisy because the\nshallow in test_shallow_cache_tree no longer made any sense, but I think\nit tests what we want to see.\n\ndiff --git i/t/t0090-cache-tree.sh w/t/t0090-cache-tree.sh\nindex 6c33e28..5706305 100755\n--- i/t/t0090-cache-tree.sh\n+++ w/t/t0090-cache-tree.sh\n@@ -16,14 +16,16 @@ cmp_cache_tree () {\n # We don't bother with actually checking the SHA1:\n # test-dump-cache-tree already verifies that all existing data is\n # correct.\n-test_shallow_cache_tree () {\n-\tprintf \"SHA (%d entries, 0 subtrees)\\n\" $(git ls-files|wc -l) >expect &&\n+test_cache_tree () {\n+\tprintf \"SHA (%d entries, 1 subtrees)\\n\" $(git ls-files|wc -l) >expect &&\n+\tprintf \"SHA sub/ (%d entries, 0 subtrees)\\n\" $(git ls-files sub|wc -l) >>expect &&\n \tcmp_cache_tree expect\n }\n \n test_invalid_cache_tree () {\n-\techo \"invalid (0 subtrees)\" >expect &&\n-\tprintf \"SHA #(ref) (%d entries, 0 subtrees)\\n\" $(git ls-files|wc -l) >>expect &&\n+\techo \"invalid (1 subtrees)\" >expect &&\n+\tprintf \"SHA #(ref) (%d entries, 1 subtrees)\\n\" $(git ls-files|wc -l) >>expect &&\n+\tprintf \"SHA sub/ (%d entries, 0 subtrees)\\n\" $(git ls-files sub|wc -l) >>expect &&\n \tcmp_cache_tree expect\n }\n \n@@ -33,13 +35,16 @@ test_no_cache_tree () {\n }\n \n test_expect_failure 'initial commit has cache-tree' '\n+\tmkdir sub &&\n+\techo bar > sub/bar &&\n+\tgit add sub/bar &&\n \ttest_commit foo &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n test_expect_success 'read-tree HEAD establishes cache-tree' '\n \tgit read-tree HEAD &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n test_expect_success 'git-add invalidates cache-tree' '\n@@ -59,7 +64,7 @@ test_expect_success 'update-index invalidates cache-tree' '\n test_expect_success 'write-tree establishes cache-tree' '\n \ttest-scrap-cache-tree &&\n \tgit write-tree &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n test_expect_success 'test-scrap-cache-tree works' '\n@@ -70,24 +75,39 @@ test_expect_success 'test-scrap-cache-tree works' '\n \n test_expect_success 'second commit has cache-tree' '\n \ttest_commit bar &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n test_expect_success 'reset --hard gives cache-tree' '\n \ttest-scrap-cache-tree &&\n \tgit reset --hard &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n test_expect_success 'reset --hard without index gives cache-tree' '\n \trm -f .git/index &&\n \tgit reset --hard &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n-test_expect_failure 'checkout gives cache-tree' '\n+test_expect_success 'checkout HEAD leaves cache-tree intact' '\n+\tgit read-tree HEAD &&\n+\tgit checkout HEAD &&\n+\ttest_cache_tree\n+'\n+\n+# NEEDSWORK: only one of these two can succeed. The second is there\n+# because it would be the better result.\n+test_expect_success 'checkout HEAD^ correctly invalidates cache-tree' '\n+\tgit checkout HEAD^ &&\n+\ttest_invalid_cache_tree\n+'\n+\n+test_expect_failure 'checkout HEAD^ gives full cache-tree' '\n+\tgit checkout master &&\n+\tgit read-tree HEAD &&\n \tgit checkout HEAD^ &&\n-\ttest_shallow_cache_tree\n+\ttest_cache_tree\n '\n \n test_done\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"185013","messageId":"7vr4xpl1nm.fsf@alter.siamese.dyndns.org","threadId":"29594","inReplyTo":"20120220140653.GC5131@sigill.intra.peff.net","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-20T19:56:13Z","receivedAt":"2012-02-20T19:56:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Interestingly, on my git.git repo, I had an empty cache. Running \"git\n> read-tree HEAD\" filled it (according to test-dump-cache-tree). It seems\n> that running \"git checkout\" empties the cache. So perhaps git could do\n> better about keeping the cache valid over time.\n\nAt least in the early days unpack-trees built the result by manually\nadding an entry without calling the add_index_entry() all over the place,\nwhich meant that it was futile to pretend that there is even a slight\nchance that complex beast would correctly invalidate cached tree\ninformation at all the necessary places. I recall that I added a code to\nnuke the cache tree at the very beginning of \"merging\" codepaths to avoid\nany bogus cache tree result to be stored in the resulting index.\n\nThese days, we have src_index and dst_index, and dst_index IIRC can start\nas empty in which case \"start from kept information and selectively\ninvalidate\" would not work at all. When src_index and dst_index are the\nsame, however, you should be able to keep the cached tree valid, at least\nin theory.\n"},{"id":"185014","messageId":"7vmx8dl1ln.fsf@alter.siamese.dyndns.org","threadId":"29594","inReplyTo":"87ty2l38ay.fsf@thomas.inf.ethz.ch","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-20T19:57:24Z","receivedAt":"2012-02-20T19:57:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> test_expect_failure 'checkout gives cache-tree' '\n> \tgit checkout HEAD^ &&\n> \ttest_shallow_cache_tree\n> '\n\nDepending on what state you start the checkout from, that is not a valid\ntest. Some form of \"git reset\" before the checkout to ensure the initial\nstate is needed.\n"},{"id":"185015","messageId":"87wr7hz35w.fsf@thomas.inf.ethz.ch","threadId":"29594","inReplyTo":"7vmx8dl1ln.fsf@alter.siamese.dyndns.org","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-02-20T19:59:55Z","receivedAt":"2012-02-20T19:59:55Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano writes:\n\n> Thomas Rast writes:\n>\n>> test_expect_failure 'checkout gives cache-tree' '\n>> \tgit checkout HEAD^ &&\n>> \ttest_shallow_cache_tree\n>> '\n>\n> Depending on what state you start the checkout from, that is not a valid\n> test. Some form of \"git reset\" before the checkout to ensure the initial\n> state is needed.\n\nOh, I was just quoting what we already had at the end of t0090 since\n4eb0346f. The test preceding it runs 'git reset --hard'.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"185016","messageId":"7vfwe5l13w.fsf@alter.siamese.dyndns.org","threadId":"29594","inReplyTo":"20120220151134.GA13135@sigill.intra.peff.net","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-20T20:08:03Z","receivedAt":"2012-02-20T20:08:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> 4. At the end of unpack_trees, we forget about src_index, and copy\n> o->result into *o->dst_index byte for byte. I.e., we overwrite\n> the_index.cache_tree, which has been properly updated the whole\n> time,\n\nI strongly suspect that \"properly updated\" part needs to be thoroughly\naudited. I wouldn't be surprised that this behaviour is what we did when\nwe split src_index vs dst_index when he rewrote unpack_trees() in order to\nemulate the original \"unpack-trees is beyond salvation because it does not\nmaintain cache tree correctly, just nuke it\" behaviour.\n\n> But it does not actually insert the _destination_ tree into the cache\n> tree. Which we can do in certain situations, but only if there were no\n> paths in the tree that were left unchanged (e.g., you modify \"foo\", then\n> \"git checkout HEAD^\", which updates \"bar\". Your tree does not match\n> HEAD^, and must be invalidated). While it would be cool to be able to\n> handle those complex cases,...\n\nIt may look cool but it may not be a good change. You are spending extra\ncycles to optimize for the next write-tree that may not happen before the\nindex is further updated.\n\n> I think this implementation matches the intent of the original calls to\n> cache_tree_invalidate_path sprinkled throughout unpack-trees.c.\n\nYes, and as long as we invalidate all the directories that need to be\ninvalidated during the unpack-tree operation, I think it is a correct\nthing to do.\n\n> But I\n> have to say that it seems a little odd for us to be modifying the\n> o->src_index throughout the whole thing.\n\nYes, that part is logically *wrong*. I think it is a remnant from the\ndays when there was no distinction between src_index and dst_index.\n\n> I would think the right thing\n> would be to make a deep copy of o->src_index->cache_tree into\n> o->result.cache_tree as the very first thing, and then update\n> o->result.cache_tree throughout the tree traversal.\n\nYes.\n"},{"id":"185017","messageId":"20120220200902.GA5314@sigill.intra.peff.net","threadId":"29594","inReplyTo":"7vr4xpl1nm.fsf@alter.siamese.dyndns.org","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-20T20:09:03Z","receivedAt":"2012-02-20T20:09:03Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 20, 2012 at 11:56:13AM -0800, Junio C Hamano wrote:\n\n> These days, we have src_index and dst_index, and dst_index IIRC can start\n> as empty in which case \"start from kept information and selectively\n> invalidate\" would not work at all. When src_index and dst_index are the\n> same, however, you should be able to keep the cached tree valid, at least\n> in theory.\n\nYeah, I was worried that the cache invalidations sprinkled throughout\nunpack-trees.c would not be sufficient (and because we are invalidating,\na missing invalidation would give us bogus cache info, which is Very\nBad).\n\nSo I think the one-liner I posted before is not sufficient in the\ngeneral case, because it definitely doesn't consider where the\ndestination is starting from. It should at least be more like:\n\n if (src_index == dst_index) {\n /* We would ordinarily want to do a deep copy here, but since\n * we know that we will be overwriting src_index in the long\n * run, it's OK to just take ownership of its cache_tree. */\n o->result.cache_tree = o->src_index->cache_tree;\n o->src_index->cache_tree = NULL;\n }\n\n [... do the usual tree traversal here, except invalidate entries in\n o->result.call_tree instead of o->src_index. That makes it a\n no-op when src_index != dst_index (because we have no cache tree\n defined in result, then), and otherwise we are invalidating what\n will go into the result...]\n\n [then as before, we copy the result to dst_index; except now the\n result may have src_index's cache_tree plus any invalidations]\n o->result = *o->dst_index;\n\nAnd fortunately that does exactly what we want in all cases, because we\nalways either read from and write to the_index, or we write to NULL (in\nwhich case we will not bother with a cache_tree for the result, and it\nis fixing a minor bug that we might be invalidating src_index's tree in the\nfirst place).\n\nI'm still slightly worried that we are missing some invalidation\nsomewhere deep in unpack_tree's callbacks (especially because they _are_\ncallbacks, and invalidating the cache_tree properly is now a promise\nthat the callbacks have to make).\n\n-Peff\n"},{"id":"185019","messageId":"20120220201749.GA5405@sigill.intra.peff.net","threadId":"29594","inReplyTo":"7vfwe5l13w.fsf@alter.siamese.dyndns.org","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-20T20:17:49Z","receivedAt":"2012-02-20T20:17:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 20, 2012 at 12:08:03PM -0800, Junio C Hamano wrote:\n\n> > 4. At the end of unpack_trees, we forget about src_index, and copy\n> > o->result into *o->dst_index byte for byte. I.e., we overwrite\n> > the_index.cache_tree, which has been properly updated the whole\n> > time,\n> \n> I strongly suspect that \"properly updated\" part needs to be thoroughly\n> audited. I wouldn't be surprised that this behaviour is what we did when\n> we split src_index vs dst_index when he rewrote unpack_trees() in order to\n> emulate the original \"unpack-trees is beyond salvation because it does not\n> maintain cache tree correctly, just nuke it\" behaviour.\n\nYep, I am also concerned about that.\n\n> > But it does not actually insert the _destination_ tree into the cache\n> > tree. Which we can do in certain situations, but only if there were no\n> > paths in the tree that were left unchanged (e.g., you modify \"foo\", then\n> > \"git checkout HEAD^\", which updates \"bar\". Your tree does not match\n> > HEAD^, and must be invalidated). While it would be cool to be able to\n> > handle those complex cases,...\n> \n> It may look cool but it may not be a good change. You are spending extra\n> cycles to optimize for the next write-tree that may not happen before the\n> index is further updated.\n\nI don't think it would be too many cycles; you would have to mark each\ntree you enter as having items from the left-hand tree or the right-hand\ntree. If only one, you can reuse the cache-tree entry (or tree sha1, if\ncoming from a tree). Otherwise, you must invalidate. And it doesn't\njust help the next write-tree, but any intermediate index diffs.\n\nOf course any such change would need timings to justify it, though.\n\nThat being said, I think just invalidating really covers 99% of the\ncases. What we really care about is that when I modify kernel/foo.c, the\n~2300 other directories (besides \"\" and \"kernel\") don't need rebuilt,\nand that is relatively simple to do. Even if doing it the other way\nproduced a tiny speedup, I would be concerned with the increase in code\ncomplexity.\n\n> > I think this implementation matches the intent of the original calls to\n> > cache_tree_invalidate_path sprinkled throughout unpack-trees.c.\n> \n> Yes, and as long as we invalidate all the directories that need to be\n> invalidated during the unpack-tree operation, I think it is a correct\n> thing to do.\n\nOK. I'll do some reading of the code to convince myself that the\nunpack_trees callbacks are doing the right thing. I'm not sure of a good\nautomatic test that would detect a failure there. Just making test cases\nis going to end up too contrived, unless we are missing something really\nobvious.\n\nI'm thinking maybe something like replaying the commit history of\nlinux-2.6 and making sure that each the tree generated by the cache-tree\nin each case matches the actual committed tree.\n\n> > But I\n> > have to say that it seems a little odd for us to be modifying the\n> > o->src_index throughout the whole thing.\n> \n> Yes, that part is logically *wrong*. I think it is a remnant from the\n> days when there was no distinction between src_index and dst_index.\n\nOK. I'll include a fix for that in the series I prepare.\n\n-Peff\n"},{"id":"185020","messageId":"20120220203540.GA5966@sigill.intra.peff.net","threadId":"29594","inReplyTo":"87d3991gyg.fsf@thomas.inf.ethz.ch","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-20T20:35:40Z","receivedAt":"2012-02-20T20:35:40Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 20, 2012 at 07:45:59PM +0100, Thomas Rast wrote:\n\n> > +\to->result.cache_tree = o->src_index->cache_tree;\n> > \to->src_index = NULL;\n> > \tret = check_updates(o) ? (-2) : 0;\n> > \tif (o->dst_index)\n> \n> Brilliant. I know I'm stealing Junio's punchline, but please make it so\n> :-)\n> \n> Browsing around in history, it seems that this was silently broken by\n> 34110cd (Make 'unpack_trees()' have a separate source and destination\n> index, 2008-03-06), which introduced the distinction between source and\n> destination index. Before that they were the same, so the cache tree\n> would have been updated correctly.\n\nOK, good. When you write a one-liner that makes a huge change in\nperformance, it is usually a good idea to think to yourself \"no, it\ncouldn't be this easy, could it?\".\n\nBut after more discussion from people more clueful than I (this is the\nfirst time I've even looked at cache-tree code), I'm feeling like this\nis the right direction, at least, if not exactly the right patch.\nAnd seeing that it is in fact a regression in 34110cd, and that the\nexisting cache-tree invalidations predate that makes me feel better. At\none point, at least, they were complete and we were depending on them to\nbe accurate. Things may have changed since then, of course, but I at\nleast know that they were sufficient in 34110cd^.\n\n> +# NEEDSWORK: only one of these two can succeed. The second is there\n> +# because it would be the better result.\n> +test_expect_success 'checkout HEAD^ correctly invalidates cache-tree' '\n> +\tgit checkout HEAD^ &&\n> +\ttest_invalid_cache_tree\n> +'\n> +\n> +test_expect_failure 'checkout HEAD^ gives full cache-tree' '\n> +\tgit checkout master &&\n> +\tgit read-tree HEAD &&\n> \tgit checkout HEAD^ &&\n> -\ttest_shallow_cache_tree\n> +\ttest_cache_tree\n> '\n\nI think you can construct two tests that will both work in the \"ideal\"\ncase. In the first one, you move to a tree that updates \"foo\", and\ntherefore the root cache-tree is invalidated.\n\nIn the second, you update \"subdir1/foo\" in the index, then move to a\ncommit that differs in \"subdir1/bar\" and \"subdir2/bar\". You should see\nthat subdir2 has the cache-tree of the destination commit, but that\nsubdir1 is invalidated (and therefore the root is also invalidated).\nThat will fail with my patch, of course, as it would invalidate subdir2,\nalso; so it would just be an expect_failure for somebody in the future.\n\nIn general, t0090 could benefit from using a larger tree. For example,\nthe add test does \"git add foo\" and checks that the root cache-tree was\ninvalidated. But it should _also_ check that the cache-tree for a\nsubdirectory is _not_ invalidated (and it isn't; git-add does the right\nthing).\n\nI'll see if I can work up some fancier tests, too.\n\n-Peff\n"},{"id":"185037","messageId":"7v1uppkvpx.fsf@alter.siamese.dyndns.org","threadId":"29594","inReplyTo":"20120220203540.GA5966@sigill.intra.peff.net","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-20T22:04:26Z","receivedAt":"2012-02-20T22:04:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> ... Things may have changed since then, of course, but I at\n> least know that they were sufficient in 34110cd^.\n\nLooking at where cache_tree_free() is called, I think back then the\ntwo-way merge was deemed OK, but we did not trust three-way merge or\nmerge-recursive at all.\n\n> I think you can construct two tests that will both work in the \"ideal\"\n> case. In the first one, you move to a tree that updates \"foo\", and\n> therefore the root cache-tree is invalidated.\n\nI have to warn you that under-invalidation of cache-tree is extremely hard\nto find. The only way I know, which I had to resort to when dealing with\na handful of instances of under-invalidation bugs, is to run write-tree\nwith potentially corrupt cache-tree and then using the same index but with\nthe cache-tree purged, run another write-tree and check to see if two\ntrees match.\n\n> In general, t0090 could benefit from using a larger tree. For example,\n> the add test does \"git add foo\" and checks that the root cache-tree was\n> invalidated. But it should _also_ check that the cache-tree for a\n> subdirectory is _not_ invalidated (and it isn't; git-add does the right\n> thing).\n\nIt is OK to check that we do not over-invalidate for performance, but it\nis a lot more important to make sure we do not under-invalidate for\ncorrectness. I am a bit worried that you seem to be putting more stress\non the former.\n"},{"id":"185042","messageId":"20120220224140.GA7116@sigill.intra.peff.net","threadId":"29594","inReplyTo":"7v1uppkvpx.fsf@alter.siamese.dyndns.org","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-20T22:41:40Z","receivedAt":"2012-02-20T22:41:40Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 20, 2012 at 02:04:26PM -0800, Junio C Hamano wrote:\n\n> > ... Things may have changed since then, of course, but I at\n> > least know that they were sufficient in 34110cd^.\n> \n> Looking at where cache_tree_free() is called, I think back then the\n> two-way merge was deemed OK, but we did not trust three-way merge or\n> merge-recursive at all.\n\nThanks, I'll take a look more closely at those cases.\n\n> It is OK to check that we do not over-invalidate for performance, but it\n> is a lot more important to make sure we do not under-invalidate for\n> correctness. I am a bit worried that you seem to be putting more stress\n> on the former.\n\nI think it is just selection bias of the specific parts of his tests\nthat I was responding to. I completely agree that correctness is way\nmore important, and I'm also trying to come up with tests to validate\ncorrectness. I just wasn't talking about them there.\n\nI still think replaying real-world test cases is going to be more likely\nto find issues in invalidation. I can come up with lots of simple\ntest-cases, but they're not likely to find anything we wouldn't find in\nthe code with trivial inspection. I think a combination of careful\nanalysis and real-world validation is going to be more helpful in the\nlong run than the kind of simplistic tests that are in t0090.\n\n-Peff\n"},{"id":"185045","messageId":"7v8vjxjd47.fsf@alter.siamese.dyndns.org","threadId":"29594","inReplyTo":"20120220224140.GA7116@sigill.intra.peff.net","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-20T23:31:36Z","receivedAt":"2012-02-20T23:31:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I still think replaying real-world test cases is going to be more likely\n> to find issues in invalidation.\n\nYes, but it depends on what kind of replaying you have in mind. It is\nvery hard to come up with \"replaying real-world test case\".\n\nFor example, randomly picking commit pair <$A,$B> from the kernel\nrepository and running\n\n\tgit reset --hard $A\n git checkout $B\n T0=$(git write-tree)\n drop-cache-tree\n T1=$(git write-tree)\n test \"$T0\" = \"$T1\" && test \"$T0\" = $(git rev-parse $B^{tree})\n\nis necessary but I do not think that is sufficient. We also want to do\nsomething like:\n\n\tgit reset --hard $A\n ... modify paths that do not change between $A and $B\n ... git add these paths\n git write-tree\n git checkout $B\n\nwith and without the \"git write-tree\" to see the part of the cache-tree\nsmudged by the modification behaves sanely. The codepath that is used\nto deal with the case where the index does not match $A but matches $B is\nalso different, so the \"modify path and git add\" step would have to be\ncrafted carefully to touch all bases.\n"},{"id":"185057","messageId":"CAA01Csrmnx9TcmH8TkYyJDx5dcre0ASKr+1-v-8fwP7YPVWQJg@mail.gmail.com","threadId":"29594","inReplyTo":"7v8vjxjd47.fsf@alter.siamese.dyndns.org","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2012-02-21T07:21:35Z","receivedAt":"2012-02-21T07:21:35Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Tue, Feb 21, 2012 at 12:31 AM, Junio C Hamano wrote:\n> Jeff King writes:\n[...]\n\nHi,\n\nI hope the fixes will also help git-svn users? I.e. there's a lot of\nrebasing (and cherry-picking - at least in my case) and probably some\nother stuff going on under hood. I.e. I hope that if I git-svn rebase\nor cherry-pick, it won't invalidate the cache...\n\nThanks,\n-- \nPiotr Krukowiecki\n"},{"id":"185080","messageId":"CACsJy8DE86qzA1=GiKZFRCt5aH8X4iMyDvfrhnqwmbq52szhHg@mail.gmail.com","threadId":"29594","inReplyTo":"20120220143644.GA13938@do","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-21T14:45:13Z","receivedAt":"2012-02-21T14:45:13Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Feb 20, 2012 at 9:36 PM, Nguyen Thai Ngoc Duy wrote:\n> On Mon, Feb 20, 2012 at 03:09:57PM +0100, Thomas Rast wrote:\n>> > Interestingly, on my git.git repo, I had an empty cache. Running \"git\n>> > read-tree HEAD\" filled it (according to test-dump-cache-tree). It seems\n>> > that running \"git checkout\" empties the cache.  So perhaps git could do\n>> > better about keeping the cache valid over time.\n>>\n>> test_expect_failure 'checkout gives cache-tree' '\n>>       git checkout HEAD^ &&\n>>       test_shallow_cache_tree\n>> '\n>>\n>> ;-)\n>\n> Quick and dirty that passes that test.\n\nI'm aware that Jeff's tackling at lower level, which retains\ncache-tree for many more cases. But this patch seems simple and safe\nto me, and in my experience this case happens quite often (or maybe I\ntend to keep my index clean). Junio, any chance this patch may get in?\n\n> -- 8< --\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 5bf96ba..c06287a 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -319,6 +319,10 @@ static void show_local_changes(struct object *head, struct diff_options *opts)\n>                die(_(\"diff_setup_done failed\"));\n>        add_pending_object(&rev, head, NULL);\n>        run_diff_index(&rev, 0);\n> +       if (!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES)) {\n> +               struct tree *tree = parse_tree_indirect(head->sha1);\n> +               prime_cache_tree(&active_cache_tree, tree);\n> +       }\n>  }\n>\n>  static void describe_detached_head(const char *msg, struct commit *commit)\n> @@ -493,13 +497,13 @@ static int merge_working_tree(struct checkout_opts *opts,\n>                }\n>        }\n>\n> +       if (!opts->force && !opts->quiet)\n> +               show_local_changes(&new->commit->object, &opts->diff_options);\n> +\n>        if (write_cache(newfd, active_cache, active_nr) ||\n>            commit_locked_index(lock_file))\n>                die(_(\"unable to write new index file\"));\n>\n> -       if (!opts->force && !opts->quiet)\n> -               show_local_changes(&new->commit->object, &opts->diff_options);\n> -\n>        return 0;\n>  }\n>\n> -- 8< --\n> --\n> Duy\n-- \nDuy\n"},{"id":"185095","messageId":"7v8vjwgfoq.fsf@alter.siamese.dyndns.org","threadId":"29594","inReplyTo":"CACsJy8DE86qzA1=GiKZFRCt5aH8X4iMyDvfrhnqwmbq52szhHg@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-21T19:16:37Z","receivedAt":"2012-02-21T19:16:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> I'm aware that Jeff's tackling at lower level, which retains\n> cache-tree for many more cases.\n>\n> But this patch seems simple and safe\n> to me, and in my experience this case happens quite often (or maybe I\n> tend to keep my index clean). Junio, any chance this patch may get in?\n\nI do not think we are talking about a duplicated effort here.\n\nBy definition, the change to hook into unpack_trees() and making sure we\ninvalidate all the necessary subtrees in the cache cannot give you a cache\ntree that is more populated than what you started with. And the train of\nthought in Peff's message is to improve this invalidation---we currently\ninvalidate everything ;-)\n\nSomebody has to populate the cache tree fully when we _know_ the index\nmatches a certain tree, and adding a call to prime_cache_tree() in\nstrategic places is a way to do so. The most obvious is write-tree, but\nthere are a few other existing codepaths that do so.\n\nBecause prime_cache_tree() by itself is a fairly expensive operation that\nreads all the trees recursively, its benefits need to be evaluated. It\nshould to happen only in an operation that is already heavy-weight, is\nlikely to have read all the trees and have many of them in-core cache, and\nalso relatively rarely happens compared to \"git add\" so that the cost can\nbe amortised over time, such as \"reset --(hard|mixed)\".\n\nSwitching branches is likely to fall into that category, but that is just\nmy gut feeling. I would feel better at night if somebody did a benchmark\n;-)\n\nOne thing we do not currently do anywhere that _might_ be of merit is to\nmake a call to cache_tree_update() instead of prime_cache_tree() when we\nalready know that only a very small subpart of the cache-tree is invalid\nand it is cheaper to repair it by rehashing only a small portion of the\nindex than to re-prime the entire cache tree with prime_cache_tree().\n"},{"id":"185138","messageId":"CACsJy8C3Myqs4=GvURWqCTxGp0R1RWotdiHGnnvBSaxyTteujw@mail.gmail.com","threadId":"29594","inReplyTo":"7v8vjwgfoq.fsf@alter.siamese.dyndns.org","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-22T02:12:22Z","receivedAt":"2012-02-22T02:12:22Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Feb 22, 2012 at 2:16 AM, Junio C Hamano wrote:\n> Because prime_cache_tree() by itself is a fairly expensive operation that\n> reads all the trees recursively, its benefits need to be evaluated. It\n> should to happen only in an operation that is already heavy-weight, is\n> likely to have read all the trees and have many of them in-core cache, and\n> also relatively rarely happens compared to \"git add\" so that the cost can\n> be amortised over time, such as \"reset --(hard|mixed)\".\n>\n> Switching branches is likely to fall into that category, but that is just\n> my gut feeling.  I would feel better at night if somebody did a benchmark\n> ;-)\n\nIn this particular case, \"git diff --cached\" is run internally, so I\nsay all trees are read once and hopefully most of them still in OS\ncache. Will run some benchmark, maybe with the coming perf test suite.\n\n> One thing we do not currently do anywhere that _might_ be of merit is to\n> make a call to cache_tree_update() instead of prime_cache_tree() when we\n> already know that only a very small subpart of the cache-tree is invalid\n> and it is cheaper to repair it by rehashing only a small portion of the\n> index than to re-prime the entire cache tree with prime_cache_tree().\n\nThat makes me think if \"diff --cached\" can take advantage of\ncache-tree to avoid walking down valid cached trees and do tree-tree\ndiff in those cases instead. Not sure if it gains us anything but code\ncomplexity.\n-- \nDuy\n"},{"id":"185144","messageId":"7v7gzfefw1.fsf@alter.siamese.dyndns.org","threadId":"29594","inReplyTo":"CACsJy8C3Myqs4=GvURWqCTxGp0R1RWotdiHGnnvBSaxyTteujw@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-22T02:55:10Z","receivedAt":"2012-02-22T02:55:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> That makes me think if \"diff --cached\" can take advantage of\n> cache-tree to avoid walking down valid cached trees and do tree-tree\n> diff in those cases instead. Not sure if it gains us anything but code\n> complexity.\n\nWhy do I have this funny feeling that we saw that comment in this thread\nalready?\n"},{"id":"185146","messageId":"7vvcmzczku.fsf@alter.siamese.dyndns.org","threadId":"29594","inReplyTo":"CACsJy8DE86qzA1=GiKZFRCt5aH8X4iMyDvfrhnqwmbq52szhHg@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-22T03:32:49Z","receivedAt":"2012-02-22T03:32:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n>> diff --git a/builtin/checkout.c b/builtin/checkout.c\n>> index 5bf96ba..c06287a 100644\n>> --- a/builtin/checkout.c\n>> +++ b/builtin/checkout.c\n>> @@ -319,6 +319,10 @@ static void show_local_changes(struct object *head, struct diff_options *opts)\n>> die(_(\"diff_setup_done failed\"));\n>> add_pending_object(&rev, head, NULL);\n>> run_diff_index(&rev, 0);\n>> + if (!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES)) {\n>> + struct tree *tree = parse_tree_indirect(head->sha1);\n>> + prime_cache_tree(&active_cache_tree, tree);\n>> + }\n>> }\n\nI think this patch is wrong on at least two counts.\n\n * The run_diff_index(&rev, 0) you reused is doing \"diff HEAD\" and not\n \"diff --cached HEAD\". The added check does not say anything about the\n comparison between the index and the tree at the HEAD.\n\n * Even if we added an extra run_diff_index(&rev, 1) there, or added a\n call to index_differs_from() to run \"diff --cached HEAD\" to check what\n needs to be checked, it is still not quite right.\n\nOn the latter point, imagine what happens in the two invocations of\ncheckout in the following sequence:\n\n $ git reset --hard master\n $ git checkout master\n $ git checkout master\n\nThe second one should notice that the cache tree is fully valid, so the\ninternal \"diff --cached\" it runs should only open the top-level tree\nand scan entries in it, without recursing into any of the subtrees, and\nrealize that the index is in sync with \"HEAD\", which should be a very\ncheap operation (that is the whole point of the current topic of our\ndiscussion looking at the cache-tree). Then the new code calls\nprime_cache_tree() to read _everything_?\n\nProbably cache_tree_fully_valid() should be called before deciding that we\nneed to re-populate the cache tree from \"HEAD\".\n"},{"id":"185165","messageId":"20120222103418.GA27199@tre","threadId":"29594","inReplyTo":"7v8vjwgfoq.fsf@alter.siamese.dyndns.org","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-22T10:34:19Z","receivedAt":"2012-02-22T10:34:19Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Feb 21, 2012 at 11:16:37AM -0800, Junio C Hamano wrote:\n> Nguyen Thai Ngoc Duy writes:\n> \n> > I'm aware that Jeff's tackling at lower level, which retains\n> > cache-tree for many more cases.\n> >\n> > But this patch seems simple and safe\n> > to me, and in my experience this case happens quite often (or maybe I\n> > tend to keep my index clean). Junio, any chance this patch may get in?\n> \n> I do not think we are talking about a duplicated effort here.\n> \n> By definition, the change to hook into unpack_trees() and making sure we\n> invalidate all the necessary subtrees in the cache cannot give you a cache\n> tree that is more populated than what you started with. And the train of\n> thought in Peff's message is to improve this invalidation---we currently\n> invalidate everything ;-)\n> \n> Somebody has to populate the cache tree fully when we _know_ the index\n> matches a certain tree, and adding a call to prime_cache_tree() in\n> strategic places is a way to do so. The most obvious is write-tree, but\n> there are a few other existing codepaths that do so.\n> \n> Because prime_cache_tree() by itself is a fairly expensive operation that\n> reads all the trees recursively, its benefits need to be evaluated. It\n> should to happen only in an operation that is already heavy-weight, is\n> likely to have read all the trees and have many of them in-core cache, and\n> also relatively rarely happens compared to \"git add\" so that the cost can\n> be amortised over time, such as \"reset --(hard|mixed)\".\n\nIt's tradeoff. As you said prime_cache_tree() is expensive.\ncache_tree_update is supposed to be cheap. But cache_tree_update() when\nempty is even more expensive than prime_cache_tree(). So it boils down\nhow much cache-tree we have after unpack_trees() and whether it's worth\nrepopulate cache-tree again.\n\n> Switching branches is likely to fall into that category, but that is just\n> my gut feeling. I would feel better at night if somebody did a benchmark\n> ;-)\n\nI timed prime_cache_tree() and cache_tree_update() while switching branch\non linux-2.6, between v2.6.32 and a quite recent version. prime_cache_tree()\ntook ~55ms while cache_tree_update() 150ms or 90ms (depending on final tree).\nIt depends on your view, whether 55ms is expensive on such a reasonably large\nrepository. I took several seconds for me to complete the checkout though.\n\nChecking out with \"-q\" prime_cache_tree() took 145ms and 80ms respectively,\nas expensive as cache_tree_update()\n\nIf cache-tree is only used at commit time, I think we could delay\nprime_cache_tree() until absolutely needed. We could add an optional index\nextension recording the fact that index matches certain tree.\nOn the first cache_tree_invalidate_path(), if cache-tree is still\nempty, we prime cache-tree, then invalidate the requested path.\nIt would then add no cost to a quick branch switch.\n\nBut if diff-cached also takes advantage of cache-tree, it's a different story.\n\nAnyway, I think this patch does better than my last one\n\n-- 8< --\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 6b9061f..e7eaeec 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -387,6 +387,7 @@ static int merge_working_tree(struct checkout_opts *opts,\n \tint ret;\n \tstruct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));\n \tint newfd = hold_locked_index(lock_file, 1);\n+\tint head_index_mismatch = 1;\n \n \tif (read_cache_preload(NULL) < 0)\n \t\treturn error(_(\"corrupt index file\"));\n@@ -396,6 +397,7 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\tret = reset_tree(new->commit->tree, opts, 1);\n \t\tif (ret)\n \t\t\treturn ret;\n+\t\thead_index_mismatch = 0;\n \t} else {\n \t\tstruct tree_desc trees[2];\n \t\tstruct tree *tree;\n@@ -490,7 +492,27 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\t\tret = reset_tree(new->commit->tree, opts, 0);\n \t\t\tif (ret)\n \t\t\t\treturn ret;\n-\t\t}\n+\t\t} else\n+\t\t\thead_index_mismatch = topts.head_index_mismatch;\n+\t}\n+\n+\t/*\n+\t * Currently cache-tree is always destroyed after\n+\t * unpack_trees(). It's probably a good idea to repopulate\n+\t * cache-tree. If the user makes a few modifications and\n+\t * commits, tree generation woulda be cheap. If they switch\n+\t * away again, not so cheap.\n+\t *\n+\t * When unpack_trees() learns to retains as much cache-tree as\n+\t * possible, this code probably does not help much on tree\n+\t * generation, unless the tree difference between to heads are\n+\t * too far, little cache-tree can be kept.\n+\t */\n+\tif (!head_index_mismatch &&\n+\t !cache_tree_fully_valid(active_cache_tree)) {\n+\t\tif (!new->commit->tree->object.parsed)\n+\t\t\tparse_tree(new->commit->tree);\n+\t\tprime_cache_tree(&active_cache_tree, new->commit->tree);\n \t}\n \n \tif (write_cache(newfd, active_cache, active_nr) ||\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 7c9ecf6..f2c518f 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1022,6 +1022,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \to->result.timestamp.nsec = o->src_index->timestamp.nsec;\n \to->merge_size = len;\n \tmark_all_ce_unused(o->src_index);\n+\tif (o->fn != twoway_merge)\n+\t\to->head_index_mismatch = 1;\n \n \t/*\n \t * Sparse checkout loop #1: set NEW_SKIP_WORKTREE on existing entries\n@@ -1736,6 +1738,8 @@ int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n \t\t (oldtree && newtree &&\n \t\t !same(oldtree, newtree) && /* 18 and 19 */\n \t\t same(current, newtree))) {\n+\t\t\tif (!newtree || (newtree && !same(current, newtree)))\n+\t\t\t\to->head_index_mismatch = 1;\n \t\t\treturn keep_entry(current, o);\n \t\t}\n \t\telse if (oldtree && !newtree && same(current, oldtree)) {\ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex 5e432f5..b75b64e 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -48,7 +48,8 @@ struct unpack_trees_options {\n \t\t gently,\n \t\t exiting_early,\n \t\t show_all_errors,\n-\t\t dry_run;\n+\t\t dry_run,\n+\t\t head_index_mismatch;\n \tconst char *prefix;\n \tint cache_bottom;\n \tstruct dir_struct *dir;\n-- 8< --\n"},{"id":"185172","messageId":"CACsJy8BZyokLSpbA=C6YEO1JajyemT9vLG=C9W4E4coA5OfszQ@mail.gmail.com","threadId":"29594","inReplyTo":"7v7gzfefw1.fsf@alter.siamese.dyndns.org","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-22T12:54:03Z","receivedAt":"2012-02-22T12:54:03Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Feb 22, 2012 at 9:55 AM, Junio C Hamano wrote:\n> Nguyen Thai Ngoc Duy writes:\n>\n>> That makes me think if \"diff --cached\" can take advantage of\n>> cache-tree to avoid walking down valid cached trees and do tree-tree\n>> diff in those cases instead. Not sure if it gains us anything but code\n>> complexity.\n>\n> Why do I have this funny feeling that we saw that comment in this thread\n> already?\n\nSimple. You wrote it and I missed it.\n\nOn Sat, Feb 18, 2012 at 5:25 AM, Junio C Hamano wrote:\n> Jeff King writes:\n>\n>> That being said, we do have an index extension to store the tree sha1 of\n>> whole directories (i.e., we populate it when we write a whole tree or\n>> subtree into the index from the object db, and it becomes invalidated\n>> when a file becomes modified). This optimization is used by things like\n>> \"git commit\" to avoid having to recreate the same sub-trees over and\n>> over when creating tree objects from the index. But we could also use it\n>> here to avoid having to even read the sub-tree objects from the object\n>> db.\n>\n> Like b65982b (Optimize \"diff-index --cached\" using cache-tree, 2009-05-20)\n> perhaps?\n\nThis optimizes the case when a cached tree matches entirely.I wonder\nwhether it's faster if we switch to tree-tree diff whenever we find\nvalid cached trees. If cache-tree is fully valid, \"git diff --cached\nfoo\" would be equivalent to \"git diff HEAD foo\".\n\nI tried \"git diff --raw HEAD HEAD~100\" (where HEAD was\nv3.1-rc1-272-g73e0881 on linux-2.6) and \"git diff --cached --raw\nHEAD~100\" with no cache-tree. The former is a little bit faster than\nthe latter (177ms vs 275ms). On gentoo-x86, 70k worktree files, it's\n4.33s vs 4.45s. But in tree-tree diff we pay high in cold cache case\nfor loading trees from \"HEAD\". So no, probably not worth more code\nchanges. Your optimization is good enough.\n-- \nDuy\n"},{"id":"185173","messageId":"878vjvxb1i.fsf@thomas.inf.ethz.ch","threadId":"29594","inReplyTo":"CACsJy8BZyokLSpbA=C6YEO1JajyemT9vLG=C9W4E4coA5OfszQ@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-02-22T13:17:13Z","receivedAt":"2012-02-22T13:17:13Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> On Sat, Feb 18, 2012 at 5:25 AM, Junio C Hamano wrote:\n>> Jeff King writes:\n>>\n>>> That being said, we do have an index extension to store the tree sha1 of\n>>> whole directories (i.e., we populate it when we write a whole tree or\n>>> subtree into the index from the object db, and it becomes invalidated\n>>> when a file becomes modified). This optimization is used by things like\n>>> \"git commit\" to avoid having to recreate the same sub-trees over and\n>>> over when creating tree objects from the index. But we could also use it\n>>> here to avoid having to even read the sub-tree objects from the object\n>>> db.\n>>\n>> Like b65982b (Optimize \"diff-index --cached\" using cache-tree, 2009-05-20)\n>> perhaps?\n>\n> This optimizes the case when a cached tree matches entirely.I wonder\n> whether it's faster if we switch to tree-tree diff whenever we find\n> valid cached trees. If cache-tree is fully valid, \"git diff --cached\n> foo\" would be equivalent to \"git diff HEAD foo\".\n\nNot necessarily; the cache-tree is valid if it faithfully represents\nwhat is in the index. It does not have any direct relation to HEAD.\n\n> I tried \"git diff --raw HEAD HEAD~100\" (where HEAD was\n> v3.1-rc1-272-g73e0881 on linux-2.6) and \"git diff --cached --raw\n> HEAD~100\" with no cache-tree. The former is a little bit faster than\n> the latter (177ms vs 275ms). On gentoo-x86, 70k worktree files, it's\n> 4.33s vs 4.45s. But in tree-tree diff we pay high in cold cache case\n> for loading trees from \"HEAD\". So no, probably not worth more code\n> changes. Your optimization is good enough.\n\nI'm still wondering about using mincore() to good effect. I tried it\nfor git-grep, but it ended up slowing things down. However, it irks me\nthat in some cases a clueful use of one form over the other can really\nmake a huge performance difference, e.g.,\n\n git grep stuff\n git grep HEAD stuff\n\nIf I am in a big repository that I haven't used in a while, the HEAD\nform will be much faster as the worktree search would fault many files.\nOTOH if I am in a heavily-used repository (and perhaps just said 'make'\nminutes ago) the worktree version will avoid the pack decompression\neffort.\n\nSadly this also has the problem that we must first determine whether\nsubstituting HEAD for the worktree (or vice versa) is valid at all. For\ngrep perhaps there could be a \"just do a fast search somewhere\" option\nsince usually you are looking for something that hasn't changed in ages.\n\nOk, that was almost completely beside the point of this thread.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"188872","messageId":"CAA01Cso0bjN5d40p0jRKdWt_vJ06C+X+Q1PJqtEsAheYfHBiSw@mail.gmail.com","threadId":"29594","inReplyTo":"7vvcmzczku.fsf@alter.siamese.dyndns.org","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2012-04-10T15:16:34Z","receivedAt":"2012-04-10T15:16:34Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Wed, Feb 22, 2012 at 4:32 AM, Junio C Hamano wrote:\n> Nguyen Thai Ngoc Duy writes:\n>\n>>> diff --git a/builtin/checkout.c b/builtin/checkout.c\n>>> index 5bf96ba..c06287a 100644\n>>> --- a/builtin/checkout.c\n>>> +++ b/builtin/checkout.c\n>>> @@ -319,6 +319,10 @@ static void show_local_changes(struct object *head, struct diff_options *opts)\n>>>                die(_(\"diff_setup_done failed\"));\n>>>        add_pending_object(&rev, head, NULL);\n>>>        run_diff_index(&rev, 0);\n>>> +       if (!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES)) {\n>>> +               struct tree *tree = parse_tree_indirect(head->sha1);\n>>> +               prime_cache_tree(&active_cache_tree, tree);\n>>> +       }\n>>>  }\n>\n> I think this patch is wrong on at least two counts.\n>\n>  * The run_diff_index(&rev, 0) you reused is doing \"diff HEAD\" and not\n>   \"diff --cached HEAD\".  The added check does not say anything about the\n>   comparison between the index and the tree at the HEAD.\n>\n>  * Even if we added an extra run_diff_index(&rev, 1) there, or added a\n>   call to index_differs_from() to run \"diff --cached HEAD\" to check what\n>   needs to be checked, it is still not quite right.\n>\n> On the latter point, imagine what happens in the two invocations of\n> checkout in the following sequence:\n>\n>   $ git reset --hard master\n>   $ git checkout master\n>   $ git checkout master\n>\n> The second one should notice that the cache tree is fully valid, so the\n> internal \"diff --cached\" it runs should only open the top-level tree\n> and scan entries in it, without recursing into any of the subtrees, and\n> realize that the index is in sync with \"HEAD\", which should be a very\n> cheap operation (that is the whole point of the current topic of our\n> discussion looking at the cache-tree).  Then the new code calls\n> prime_cache_tree() to read _everything_?\n>\n> Probably cache_tree_fully_valid() should be called before deciding that we\n> need to re-populate the cache tree from \"HEAD\".\n\nHi,\n\ncould I ask what is the status of this? There were some patches\nposted, but I think nothing final?\n\nThanks,\n\n-- \nPiotr Krukowiecki\n"},{"id":"188885","messageId":"7vbomzqzuc.fsf@alter.siamese.dyndns.org","threadId":"29594","inReplyTo":"CAA01Cso0bjN5d40p0jRKdWt_vJ06C+X+Q1PJqtEsAheYfHBiSw@mail.gmail.com","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-10T16:23:59Z","receivedAt":"2012-04-10T16:23:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Piotr Krukowiecki writes:\n\n> could I ask what is the status of this? There were some patches\n> posted, but I think nothing final?\n\nI do not think you meant to address your inquiry to me, but I think these\npatches tried out some ideas, got issues discovered in them and then got\nabandoned before resulting in a working code that is ready for testing.\n\nI wish there were fewer such series, but it happens (see \"Stalled\" section\nin What's cooking).\n"},{"id":"188891","messageId":"20120410180044.GA23758@sigill.intra.peff.net","threadId":"29594","inReplyTo":"7vbomzqzuc.fsf@alter.siamese.dyndns.org","subject":"Re: git status: small difference between stating whole repository and small subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-10T18:00:44Z","receivedAt":"2012-04-10T18:00:44Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 10, 2012 at 09:23:59AM -0700, Junio C Hamano wrote:\n\n> > could I ask what is the status of this? There were some patches\n> > posted, but I think nothing final?\n> \n> I do not think you meant to address your inquiry to me, but I think these\n> patches tried out some ideas, got issues discovered in them and then got\n> abandoned before resulting in a working code that is ready for testing.\n\nYes. I think we decided that we needed some pretty good testing to add\nthe cache_tree handling back into unpack_trees. I'd still like to do\nthat testing, but haven't done it yet.\n\n-Peff\n"}]}