{"thread":{"id":"36731","subject":"git reset for index restoration?","startedAt":"2014-05-22T16:22:43Z","lastAt":"2014-05-22T23:37:41Z","messageCount":22,"participants":["David Turner","Jeff King","Elijah Newren","Junio C Hamano","Duy Nguyen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"242493","messageId":"1400775763.1933.5.camel@stross","threadId":"36731","inReplyTo":null,"subject":"git reset for index restoration?","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-22T16:22:43Z","receivedAt":"2014-05-22T16:22:43Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"If I have a git repository with a clean working tree, and I delete the\nindex, then I can use git reset (with no arguments) to recreate it.\nHowever, when I do recreate it, it doesn't come back the same.  I have\nnot analyzed this in detail, but the effect is that commands like git\nstatus take much longer because they must read objects out of a pack\nfile.  In other words, the index seems to not realize that the index (or\nat least most of it) represents the same state as HEAD.  If I do git\nreset --hard, the index is restored to the original state (it's\nbyte-for-byte identical), and the pack file is no longer read.\n\nBefore I try to dig in to why this should be so, does anyone happen to\nknow off the top of their head?  Does this constitute a bug in git, or a\nbug in my understanding of git?\n"},{"id":"242496","messageId":"20140522164634.GB30419@sigill.intra.peff.net","threadId":"36731","inReplyTo":"1400775763.1933.5.camel@stross","subject":"Re: git reset for index restoration?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-22T16:46:34Z","receivedAt":"2014-05-22T16:46:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 22, 2014 at 12:22:43PM -0400, David Turner wrote:\n\n> If I have a git repository with a clean working tree, and I delete the\n> index, then I can use git reset (with no arguments) to recreate it.\n> However, when I do recreate it, it doesn't come back the same.  I have\n> not analyzed this in detail, but the effect is that commands like git\n> status take much longer because they must read objects out of a pack\n> file.  In other words, the index seems to not realize that the index (or\n> at least most of it) represents the same state as HEAD.  If I do git\n> reset --hard, the index is restored to the original state (it's\n> byte-for-byte identical), and the pack file is no longer read.\n\nAre you sure it's reading a packfile? I would expect that it is rather\nreading the files in the working tree. One of the functions of the index\nis to maintain a cache of the sha1 of the working tree file content and\nthe stat information. Once that is populated, a subsequent \"git status\"\ncan then just lstat() the files to see that its sha1 cache is up to\ndate.\n\n-Peff\n"},{"id":"242497","messageId":"CABPp-BHtYnput7SiAbnqUjpDibTi5o_2MAXfSj17fCdKSC7Hjg@mail.gmail.com","threadId":"36731","inReplyTo":"1400775763.1933.5.camel@stross","subject":"Re: git reset for index restoration?","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2014-05-22T16:46:51Z","receivedAt":"2014-05-22T16:46:51Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, May 22, 2014 at 9:22 AM, David Turner <dturner@twopensource.com> wrote:\n> If I have a git repository with a clean working tree, and I delete the\n> index, then I can use git reset (with no arguments) to recreate it.\n> However, when I do recreate it, it doesn't come back the same.  I have\n> not analyzed this in detail, but the effect is that commands like git\n> status take much longer because they must read objects out of a pack\n> file.  In other words, the index seems to not realize that the index (or\n> at least most of it) represents the same state as HEAD.  If I do git\n> reset --hard, the index is restored to the original state (it's\n> byte-for-byte identical), and the pack file is no longer read.\n>\n> Before I try to dig in to why this should be so, does anyone happen to\n> know off the top of their head?  Does this constitute a bug in git, or a\n> bug in my understanding of git?\n\nIt's not a bug.  The index has additional stat-info it tracks about\nfiles -- inode number, mtime, etc. -- so that it can quickly check\nwhether files are up to date without comparing full file contents in\nthe working copy to the relevant version from .git/objects.\n\n'git reset' means make the index match the commit in HEAD.  It implies\nnothing about the working copy files, which could be different.\nAlthough you say that you have a clean working tree, git doesn't check\nto verify that, so it has to treat these files as stat-dirty until the\nnext operation (e.g. git status) fills these in -- an operation that\ninvolves full comparison of the files in the working copy with the\nrelevant version of the file from under .git/objects. (You may find\n'git update-index --refresh' helpful here.)\n\ngit reset --hard means not only make the index match the commit in\nHEAD, but change files in the working copy to match as well.  In such\na case, git will know that the index matches the working copy (since\nit is enforcing it), so it can update all the stat-info in the index\nand future git-status operations will be fast.\n"},{"id":"242507","messageId":"1400782096.18134.1.camel@stross","threadId":"36731","inReplyTo":"20140522164634.GB30419@sigill.intra.peff.net","subject":"Re: git reset for index restoration?","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-22T18:08:16Z","receivedAt":"2014-05-22T18:08:16Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Thu, 2014-05-22 at 12:46 -0400, Jeff King wrote:\n> On Thu, May 22, 2014 at 12:22:43PM -0400, David Turner wrote:\n> \n> > If I have a git repository with a clean working tree, and I delete the\n> > index, then I can use git reset (with no arguments) to recreate it.\n> > However, when I do recreate it, it doesn't come back the same.  I have\n> > not analyzed this in detail, but the effect is that commands like git\n> > status take much longer because they must read objects out of a pack\n> > file.  In other words, the index seems to not realize that the index (or\n> > at least most of it) represents the same state as HEAD.  If I do git\n> > reset --hard, the index is restored to the original state (it's\n> > byte-for-byte identical), and the pack file is no longer read.\n> \n> Are you sure it's reading a packfile?\n\nWell, it's calling inflate(), and strace says it is reading\ne.g. .git/objects/pack/pack-....{idx,pack}.\n\nSo, I would say so.\n"},{"id":"242509","messageId":"1400782642.18134.8.camel@stross","threadId":"36731","inReplyTo":"CABPp-BHtYnput7SiAbnqUjpDibTi5o_2MAXfSj17fCdKSC7Hjg@mail.gmail.com","subject":"Re: git reset for index restoration?","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-22T18:17:22Z","receivedAt":"2014-05-22T18:17:22Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Thu, 2014-05-22 at 09:46 -0700, Elijah Newren wrote:\n> On Thu, May 22, 2014 at 9:22 AM, David Turner <dturner@twopensource.com> wrote:\n> > If I have a git repository with a clean working tree, and I delete the\n> > index, then I can use git reset (with no arguments) to recreate it.\n> > However, when I do recreate it, it doesn't come back the same.  I have\n> > not analyzed this in detail, but the effect is that commands like git\n> > status take much longer because they must read objects out of a pack\n> > file.  In other words, the index seems to not realize that the index (or\n> > at least most of it) represents the same state as HEAD.  If I do git\n> > reset --hard, the index is restored to the original state (it's\n> > byte-for-byte identical), and the pack file is no longer read.\n> >\n> > Before I try to dig in to why this should be so, does anyone happen to\n> > know off the top of their head?  Does this constitute a bug in git, or a\n> > bug in my understanding of git?\n> \n> It's not a bug.  The index has additional stat-info it tracks about\n> files -- inode number, mtime, etc. -- so that it can quickly check\n> whether files are up to date without comparing full file contents in\n> the working copy to the relevant version from .git/objects.\n> \n> 'git reset' means make the index match the commit in HEAD. \n\nSure, so why does git status the need to read the pack file, given that\nwe already know that the index matches HEAD?\n\n>  It implies\n> nothing about the working copy files, which could be different.\n> Although you say that you have a clean working tree, git doesn't check\n> to verify that, so it has to treat these files as stat-dirty until the\n> next operation (e.g. git status) fills these in -- an operation that\n> involves full comparison of the files in the working copy with the\n> relevant version of the file from under .git/objects. (You may find\n> 'git update-index --refresh' helpful here.)\n\nIn fact, git status does not write the index (at least in this context).\nAnd what is slow is not (only) checking over the working tree, but\nreading the packs.  There should be no need to read files from the ODB\nat all, since the index knows those objects' SHA1s, and it can check\nthose against the working tree's SHA1s.  Interestingly, if strace is to\nbe believed, git status doesn't even do check the working trees SHA1s\nafter a git reset; maybe reset already sets the stat bits?\n\nSo that's why I'm confused.\n"},{"id":"242510","messageId":"20140522182303.GA1167@sigill.intra.peff.net","threadId":"36731","inReplyTo":"1400782096.18134.1.camel@stross","subject":"Re: git reset for index restoration?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-22T18:23:03Z","receivedAt":"2014-05-22T18:23:03Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 22, 2014 at 02:08:16PM -0400, David Turner wrote:\n\n> On Thu, 2014-05-22 at 12:46 -0400, Jeff King wrote:\n> > On Thu, May 22, 2014 at 12:22:43PM -0400, David Turner wrote:\n> >\n> > > If I have a git repository with a clean working tree, and I delete the\n> > > index, then I can use git reset (with no arguments) to recreate it.\n> > > However, when I do recreate it, it doesn't come back the same.  I have\n> > > not analyzed this in detail, but the effect is that commands like git\n> > > status take much longer because they must read objects out of a pack\n> > > file.  In other words, the index seems to not realize that the index (or\n> > > at least most of it) represents the same state as HEAD.  If I do git\n> > > reset --hard, the index is restored to the original state (it's\n> > > byte-for-byte identical), and the pack file is no longer read.\n> >\n> > Are you sure it's reading a packfile?\n>\n> Well, it's calling inflate(), and strace says it is reading\n> e.g. .git/objects/pack/pack-....{idx,pack}.\n>\n> So, I would say so.\n\nThat seems odd that we would be spending extra time there. We do\ninflate() the trees in order to diff the index against HEAD, but we\nshouldn't need to inflate any blobs.\n\nHere it is for me (on linux.git):\n\n  [before, warm cache]\n  $ time perf record -q git status >/dev/null\n  real    0m0.192s\n  user    0m0.080s\n  sys     0m0.108s\n\n  $ perf report | grep -v '#' | head -5\n     7.46%      git  [kernel.kallsyms]   [k] __d_lookup_rcu\n     4.55%      git  libz.so.1.2.8       [.] inflate\n     3.53%      git  libc-2.18.so        [.] __memcmp_sse4_1\n     3.46%      git  [kernel.kallsyms]   [k] security_inode_getattr\n     3.29%      git  git                 [.] memihash\n\n  $ time git reset\n  real    0m0.080s\n  user    0m0.036s\n  sys     0m0.040s\n\nSo status is pretty quick, and the time is going to lstat in the kernel,\nand some tree inflation. Reset is fast, because it has nothing much to\ndo. Now let's kill off the index's stat cache:\n\n  $ rm .git/index\n  $ time perf record -q git reset\n  real    0m0.967s\n  user    0m0.780s\n  sys     0m0.180s\n\nThat took a while. What was it doing?\n\n  $ perf report | grep -v '#' | head -5\n     3.23%      git  [kernel.kallsyms]   [k] copy_user_enhanced_fast_string\n     1.74%      git  libcrypto.so.1.0.0  [.] 0x000000000007e010            \n     1.60%      git  [kernel.kallsyms]   [k] __d_lookup_rcu                \n     1.51%      git  [kernel.kallsyms]   [k] page_fault                    \n     1.44%      git  libc-2.18.so        [.] __memcmp_sse4_1               \n\nReading files and sha1. We hash the working-tree files here (reset\ndoesn't technically need to refresh the index from the working tree to\ncopy entries from HEAD into the index, but it does it so it can do fancy\nthings like tell you about which files are now out-of-date).\n\nNow how does stat fare after this?\n\n  $ time perf record -q git status >/dev/null\n  real    0m0.189s\n  user    0m0.088s\n  sys     0m0.096s\n\nLooks about the same as before to me.\n\nNote that if you use \"read-tree\" instead of \"reset\", it _just_ loads the\nindex, and doesn't touch the working tree. If you then run \"git status\",\nthen _that_ command has to refresh the index, and it will pay the\nhashing cost. Like:\n\n  $ rm .git/index\n  $ time git read-tree HEAD\n  real    0m0.084s\n  user    0m0.064s\n  sys     0m0.016s\n  $ time git status >/dev/null\n  real    0m0.833s\n  user    0m0.712s\n  sys     0m0.112s\n\nAll of this is behaving as I would expect. Can you show us a set of\ncommands that deviate from this?\n\n-Peff\n"},{"id":"242512","messageId":"20140522183930.GB1167@sigill.intra.peff.net","threadId":"36731","inReplyTo":"1400782642.18134.8.camel@stross","subject":"Re: git reset for index restoration?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-22T18:39:30Z","receivedAt":"2014-05-22T18:39:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 22, 2014 at 02:17:22PM -0400, David Turner wrote:\n\n> In fact, git status does not write the index (at least in this context).\n> And what is slow is not (only) checking over the working tree, but\n> reading the packs.  There should be no need to read files from the ODB\n> at all, since the index knows those objects' SHA1s, and it can check\n> those against the working tree's SHA1s.  Interestingly, if strace is to\n> be believed, git status doesn't even do check the working trees SHA1s\n> after a git reset; maybe reset already sets the stat bits?\n\nKeep in mind that \"status\" is running _two_ diffs. One between HEAD and\nthe index, and one between the index and the working tree.\n\nYour timings might be more interesting just run \"git diff-index --cached\nHEAD\", which is the index->HEAD half of that, ignoring the working tree\nstate entirely.\n\nNaively, running that diff will involve walking the trees and the index\nin parallel, which means opening a bunch of tree objects. The cache-tree\nindex extension, however, records tree sha1s, which means we can avoid\nrecursing into parts of the comparison.\n\nComparing the two:\n\n  $ rm .git/index\n  $ git reset\n  $ time git diff-index --cached HEAD\n  real    0m0.044s\n  user    0m0.040s\n  sys     0m0.000s\n\n  $ git reset --hard\n  $ time git diff-index --cached HEAD\n  real    0m0.012s\n  user    0m0.008s\n  sys     0m0.000s\n\ndoes show some improvement. Perhaps \"git reset\" is not writing out the\ncache-tree extension?\n\n-Peff\n"},{"id":"242515","messageId":"1400785669.18134.21.camel@stross","threadId":"36731","inReplyTo":"20140522183930.GB1167@sigill.intra.peff.net","subject":"Re: git reset for index restoration?","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-22T19:07:49Z","receivedAt":"2014-05-22T19:07:49Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Thu, 2014-05-22 at 14:39 -0400, Jeff King wrote:\n> does show some improvement. Perhaps \"git reset\" is not writing out the\n> cache-tree extension?\n \nYes, that seems to be exactly what is going on; the two indexes are\nidentical up to the point where the TREE extension appears.\n\nThanks for clearing that up!  \n\nDo you think that this is a bug in git reset?\n\n(I'm also going to reply to your other mail because it seems like it\nmaybe points up some related bug)\n"},{"id":"242516","messageId":"20140522190959.GA18785@sigill.intra.peff.net","threadId":"36731","inReplyTo":"1400785669.18134.21.camel@stross","subject":"Re: git reset for index restoration?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-22T19:09:59Z","receivedAt":"2014-05-22T19:09:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 22, 2014 at 03:07:49PM -0400, David Turner wrote:\n\n> On Thu, 2014-05-22 at 14:39 -0400, Jeff King wrote:\n> > does show some improvement. Perhaps \"git reset\" is not writing out the\n> > cache-tree extension?\n>  \n> Yes, that seems to be exactly what is going on; the two indexes are\n> identical up to the point where the TREE extension appears.\n> \n> Thanks for clearing that up!  \n> \n> Do you think that this is a bug in git reset?\n\nPossibly. There is a call to prime_cache_tree in builtin/reset.c, which\nlooks like it should trigger during a \"mixed\" or \"hard\" reset (and\nwithout arguments, you should have a mixed reset). But it doesn't seem\nto get called. I haven't traced it further.\n\n-Peff\n"},{"id":"242518","messageId":"1400786789.18134.27.camel@stross","threadId":"36731","inReplyTo":"20140522182303.GA1167@sigill.intra.peff.net","subject":"Re: git reset for index restoration?","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-22T19:26:29Z","receivedAt":"2014-05-22T19:26:29Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Thu, 2014-05-22 at 14:23 -0400, Jeff King wrote:\n> On Thu, May 22, 2014 at 02:08:16PM -0400, David Turner wrote:\n> \n> > On Thu, 2014-05-22 at 12:46 -0400, Jeff King wrote:\n> > > On Thu, May 22, 2014 at 12:22:43PM -0400, David Turner wrote:\n> > >\n> > > > If I have a git repository with a clean working tree, and I delete the\n> > > > index, then I can use git reset (with no arguments) to recreate it.\n> > > > However, when I do recreate it, it doesn't come back the same.  I have\n> > > > not analyzed this in detail, but the effect is that commands like git\n> > > > status take much longer because they must read objects out of a pack\n> > > > file.  In other words, the index seems to not realize that the index (or\n> > > > at least most of it) represents the same state as HEAD.  If I do git\n> > > > reset --hard, the index is restored to the original state (it's\n> > > > byte-for-byte identical), and the pack file is no longer read.\n> > >\n> > > Are you sure it's reading a packfile?\n> >\n> > Well, it's calling inflate(), and strace says it is reading\n> > e.g. .git/objects/pack/pack-....{idx,pack}.\n> >\n> > So, I would say so.\n> \n> That seems odd that we would be spending extra time there. We do\n> inflate() the trees in order to diff the index against HEAD, but we\n> shouldn't need to inflate any blobs.\n\nI just did a fresh clone of linux.git, and it doesn't have TREE (and\nspends time in inflate).  Then I did a reset --hard, and it gained TREE\n(and stopped spending time in inflate).  Then I did a checkout -b, and\nTREE was lost again (time in inflate).  hard reset again (to HEAD, so no\nchange), and TREE returns and status is fast again.\n\nCommitting doesn't seem to create a TREE extension where one doesn't\nexist.\n"},{"id":"242520","messageId":"20140522193030.GA22383@sigill.intra.peff.net","threadId":"36731","inReplyTo":"20140522190959.GA18785@sigill.intra.peff.net","subject":"Re: git reset for index restoration?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-22T19:30:30Z","receivedAt":"2014-05-22T19:30:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"[+cc Junio for cache-tree expertise]\n\nOn Thu, May 22, 2014 at 03:09:59PM -0400, Jeff King wrote:\n\n> > > does show some improvement. Perhaps \"git reset\" is not writing out the\n> > > cache-tree extension?\n> [...]\n> \n> Possibly. There is a call to prime_cache_tree in builtin/reset.c, which\n> looks like it should trigger during a \"mixed\" or \"hard\" reset (and\n> without arguments, you should have a mixed reset). But it doesn't seem\n> to get called. I haven't traced it further.\n\nSo here's what's happening. The prime_cache_tree call is in reset_index,\nand was added by:\n\n  commit 6c52ec8a9ab48b50fc8bf9559467d5a4cf7eee3b\n  Author: Thomas Rast <trast@student.ethz.ch>\n  Date:   Tue Dec 6 18:43:39 2011 +0100\n  \n      reset: update cache-tree data when appropriate\n      \n      In the case of --mixed and --hard, we throw away the old index and\n      rebuild everything from the tree argument (or HEAD).  So we have an\n      opportunity here to fill in the cache-tree data, just as read-tree\n      did.\n      \n      Signed-off-by: Thomas Rast <trast@student.ethz.ch>\n      Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nBut that was counteracted by:\n\n  commit 3fde386a40f38dbaa684c17603e71909b862d021\n  Author: Martin von Zweigbergk <martinvonz@gmail.com>\n  Date:   Mon Jan 14 21:47:51 2013 -0800\n  \n      reset [--mixed]: use diff-based reset whether or not pathspec was given\n      \n      Thanks to b65982b (Optimize \"diff-index --cached\" using cache-tree,\n      2009-05-20), resetting with paths is much faster than resetting\n      without paths. Some timings for the linux-2.6 repo to illustrate this\n      (best of five, warm cache):\n      \n              reset       reset .\n      real    0m0.219s    0m0.080s\n      user    0m0.140s    0m0.040s\n      sys     0m0.070s    0m0.030s\n      \n      These two commands should do the same thing, so instead of having the\n      user type the trailing \" .\" to get the faster do_diff_cache()-based\n      implementation, always use it when doing a mixed reset, with or\n      without paths (so \"git reset $rev\" would also be faster).\n      \n      Timing \"git reset\" shows that it indeed becomes as fast as\n      \"git reset .\" after this patch.\n      \n      Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>\n      Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nWe never call reset_index now, because we handle it via diff.  We could\ncall prime_cache_tree in this case, but I'm not sure if that is a good\nidea, because it primes it from scratch (and so it opens up all those\ntrees that we are trying to avoid touching). I'm not sure if there's an\neasy way to update it incrementally; I don't know the cache-tree code\nvery well.\n\n-Peff\n"},{"id":"242527","messageId":"xmqqha4hmr9d.fsf@gitster.dls.corp.google.com","threadId":"36731","inReplyTo":"20140522193030.GA22383@sigill.intra.peff.net","subject":"Re: git reset for index restoration?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-22T21:34:54Z","receivedAt":"2014-05-22T21:34:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> [+cc Junio for cache-tree expertise]\n> ...\n> We never call reset_index now, because we handle it via diff.  We could\n> call prime_cache_tree in this case, but I'm not sure if that is a good\n> idea, because it primes it from scratch (and so it opens up all those\n> trees that we are trying to avoid touching). I'm not sure if there's an\n> easy way to update it incrementally; I don't know the cache-tree code\n> very well.\n\nThe cache-tree is designed to start in a well-populated state,\nallowing you to efficiently smudge the part you touched by\ninvalidating while keeping the parts you haven't touched intact.\n\nWhat is missing in its API is a more fine-grained support to let us\nsay \"it has degraded too much and we need to bring it into a\nwell-populated state again for it to be truly useful as an\noptimization.\"  There are only two modes of support to revive a\ndegraded cache-tree, one being write_cache_as_tree(), in which case\nwe have to compute necessary tree object names anyway (so there is\nno point discarding the result of the computation), and the other\nbeing calls to prime-cache-tree, in which we happen to know that the\nwhole index contents must match the whole tree structure represented\nby one tree object.\n\nBoth aim to restore the cache-tree into a fully-populated state, and\nthere is no support to populate it \"well enough\" by doing anything\nincremental.  You can call write-tree side incremental, because it\ndoes reuse what is still valid without recomputing tree objects for\nthem---but the result is a fully-populated state.\n\nAdding a more fine-grain support is not against the overall design,\nbut it was unclear what such additional API functions should look\nlike, and where we can call them safely, at least back when we were\nactively improving it.  Two that comes to my mind are:\n\n - We know that the subtrees down in this directory are degraded too\n   much; write-tree only the subtrees that correspond to this\n   directory without restoring other parts of the tree.\n\n - We just populated the index with the subtrees in this directory\n   and know that they should match the tree hierarchy exactly.\n   prime-cache-tree only the parts without affecting other parts of\n   the tree.\n\nAs with calls to existing (whole-tree) prime-cache-tree, the latter\nis an error-prone optimization---I think we had cases where we said\n\"after this operation, we know that the index must exactly match the\ntree we used to muck with the index\" and added a call, and later\ndiscovered that \"must exactly match\" was not true.\n\nThe former forces recomputation, so there is much less safety\nconcern.\n"},{"id":"242528","messageId":"1400795586.18134.40.camel@stross","threadId":"36731","inReplyTo":"xmqqha4hmr9d.fsf@gitster.dls.corp.google.com","subject":"Re: git reset for index restoration?","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-22T21:53:06Z","receivedAt":"2014-05-22T21:53:06Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Thu, 2014-05-22 at 14:34 -0700, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n> > [+cc Junio for cache-tree expertise]\n> > ...\n> > We never call reset_index now, because we handle it via diff.  We could\n> > call prime_cache_tree in this case, but I'm not sure if that is a good\n> > idea, because it primes it from scratch (and so it opens up all those\n> > trees that we are trying to avoid touching). I'm not sure if there's an\n> > easy way to update it incrementally; I don't know the cache-tree code\n> > very well.\n> \n> The cache-tree is designed to start in a well-populated state,\n> allowing you to efficiently smudge the part you touched by\n> invalidating while keeping the parts you haven't touched intact.\n\nAs far as I can tell, the cache-tree does not in fact ever get into a\nwell-populated state (that is, it does not exist at all) under ordinary\ngit operation except by git reset --hard.  Perhaps this was already\nclear from the previous traffic on the thread, but I wanted to make sure\nJunio was also aware of this.\n"},{"id":"242529","messageId":"xmqqd2f5mq5h.fsf@gitster.dls.corp.google.com","threadId":"36731","inReplyTo":"1400795586.18134.40.camel@stross","subject":"Re: git reset for index restoration?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-22T21:58:50Z","receivedAt":"2014-05-22T21:58:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> On Thu, 2014-05-22 at 14:34 -0700, Junio C Hamano wrote:\n>> Jeff King <peff@peff.net> writes:\n>> \n>> > [+cc Junio for cache-tree expertise]\n>> > ...\n>> > We never call reset_index now, because we handle it via diff.  We could\n>> > call prime_cache_tree in this case, but I'm not sure if that is a good\n>> > idea, because it primes it from scratch (and so it opens up all those\n>> > trees that we are trying to avoid touching). I'm not sure if there's an\n>> > easy way to update it incrementally; I don't know the cache-tree code\n>> > very well.\n>> \n>> The cache-tree is designed to start in a well-populated state,\n>> allowing you to efficiently smudge the part you touched by\n>> invalidating while keeping the parts you haven't touched intact.\n>\n> As far as I can tell, the cache-tree does not in fact ever get into a\n> well-populated state (that is, it does not exist at all) under ordinary\n> git operation except by git reset --hard.  Perhaps this was already\n> clear from the previous traffic on the thread, but I wanted to make sure\n> Junio was also aware of this.\n\nYes.  As I said, that should not usually be a problem for those who\ndo the real work (read: commit), at which time write-tree will fully\npopulate the cache-tree.\n"},{"id":"242530","messageId":"1400796077.18134.41.camel@stross","threadId":"36731","inReplyTo":"xmqqd2f5mq5h.fsf@gitster.dls.corp.google.com","subject":"Re: git reset for index restoration?","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-22T22:01:17Z","receivedAt":"2014-05-22T22:01:17Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Thu, 2014-05-22 at 14:58 -0700, Junio C Hamano wrote:\n> David Turner <dturner@twopensource.com> writes:\n> \n> > On Thu, 2014-05-22 at 14:34 -0700, Junio C Hamano wrote:\n> >> Jeff King <peff@peff.net> writes:\n> >> \n> >> > [+cc Junio for cache-tree expertise]\n> >> > ...\n> >> > We never call reset_index now, because we handle it via diff.  We could\n> >> > call prime_cache_tree in this case, but I'm not sure if that is a good\n> >> > idea, because it primes it from scratch (and so it opens up all those\n> >> > trees that we are trying to avoid touching). I'm not sure if there's an\n> >> > easy way to update it incrementally; I don't know the cache-tree code\n> >> > very well.\n> >> \n> >> The cache-tree is designed to start in a well-populated state,\n> >> allowing you to efficiently smudge the part you touched by\n> >> invalidating while keeping the parts you haven't touched intact.\n> >\n> > As far as I can tell, the cache-tree does not in fact ever get into a\n> > well-populated state (that is, it does not exist at all) under ordinary\n> > git operation except by git reset --hard.  Perhaps this was already\n> > clear from the previous traffic on the thread, but I wanted to make sure\n> > Junio was also aware of this.\n> \n> Yes.  As I said, that should not usually be a problem for those who\n> do the real work (read: commit), at which time write-tree will fully\n> populate the cache-tree.\n\nGit commit does not in fact populate the cache-tree.\n"},{"id":"242532","messageId":"xmqq61kxmphw.fsf@gitster.dls.corp.google.com","threadId":"36731","inReplyTo":"1400796077.18134.41.camel@stross","subject":"Re: git reset for index restoration?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-22T22:12:59Z","receivedAt":"2014-05-22T22:12:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n>> Yes.  As I said, that should not usually be a problem for those who\n>> do the real work (read: commit), at which time write-tree will fully\n>> populate the cache-tree.\n>\n> Git commit does not in fact populate the cache-tree.\n\nIf that is the case, we must have broken the write-tree codepath\nover time.\n\nOf course, \"git commit foo\" will need to prepare a temporary index\nwhere only the entry \"foo\" is different from the HEAD version, write\nthe tree to create a commit, but we do not write out the main index\nas a tree after updating the same entry \"foo\" in it (there may be\nother changes relative to HEAD), so its cache-tree is only\ninvalidated at \"foo\" when we updating the entry and we do not spend\nextra cycles to repopulate it.\n\nBut at least my understanding has been that \"git commit\" (no partial\ncommit, write the whole index as a commit) which uses the \"git\nwrite-tree\" machinery knows which subtree has what tree object name\nand populates the cache-tree fully.\n"},{"id":"242533","messageId":"xmqq1tvlmp8w.fsf@gitster.dls.corp.google.com","threadId":"36731","inReplyTo":"xmqq61kxmphw.fsf@gitster.dls.corp.google.com","subject":"Re: git reset for index restoration?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-22T22:18:23Z","receivedAt":"2014-05-22T22:18:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Turner <dturner@twopensource.com> writes:\n>\n>>> Yes.  As I said, that should not usually be a problem for those who\n>>> do the real work (read: commit), at which time write-tree will fully\n>>> populate the cache-tree.\n>>\n>> Git commit does not in fact populate the cache-tree.\n>\n> If that is the case, we must have broken the write-tree codepath\n> over time.\n>\n> Of course, \"git commit foo\" will need to prepare a temporary index\n> where only the entry \"foo\" is different from the HEAD version, write\n> the tree to create a commit, but we do not write out the main index\n> as a tree after updating the same entry \"foo\" in it (there may be\n> other changes relative to HEAD), so its cache-tree is only\n> invalidated at \"foo\" when we updating the entry and we do not spend\n> extra cycles to repopulate it.\n>\n> But at least my understanding has been that \"git commit\" (no partial\n> commit, write the whole index as a commit) which uses the \"git\n> write-tree\" machinery knows which subtree has what tree object name\n> and populates the cache-tree fully.\n\n... and the \"incrementally repair\" Peff talks about would be to\ncover more cases where we may know (either because we have already\ncomputed it to write out a subtree, or we have just read from a\nknown tree to populate a part of the index and we know the paths in\nthe index that correspond to that subtree are exactly the same ones\nas found in the tree we read from) parts of the cache-tree can be\nupdated with tree object names for subtrees, but we don't do\nanything right now.  \"git commit foo\" (where \"foo\" is a directory)\nmay be able to say \"The cache-tree entry for 'foo' can be updated\nwith the tree object of the new HEAD:foo because we know they must\nmatch in the main index\", for example.\n"},{"id":"242534","messageId":"xmqqwqddla63.fsf@gitster.dls.corp.google.com","threadId":"36731","inReplyTo":"xmqq61kxmphw.fsf@gitster.dls.corp.google.com","subject":"Re: git reset for index restoration?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-22T22:29:24Z","receivedAt":"2014-05-22T22:29:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> But at least my understanding has been that \"git commit\" (no partial\n> commit, write the whole index as a commit) which uses the \"git\n> write-tree\" machinery knows which subtree has what tree object name\n> and populates the cache-tree fully.\n\nHere is what I tried just now.\n\n    $ rm .git/index\n    $ git read-tree HEAD HEAD\n\nNote that a single-tree read-tree will populate the cache-tree and\nthat is why I am forcing \"switch branches\" 2-way read-tree here,\nwhich I know will discard the cache-tree fully.\n\n    $ ls -l .git/index\n    -rw-r----- 1 jch eng 249440 May 22 15:20 .git/index\n    $ git checkout HEAD^0\n    $ ls -l .git/index\n    -rw-r----- 1 jch eng 249440 May 22 15:21 .git/index\n\nStill the same size, without cache-tree.\n\n    $ git write-tree\n    57361c4add61b638dad1c1c2542edf877f515c48\n    $ ls -l .git/index\n    -rw-r----- 1 jch eng 254383 May 22 15:21 .git/index\n\nThe size differences come from the recomputation of the cache tree.\nThe result is the same if we replace \"git write-tree\" with a\nwhole-index commit, e.g.\n\n    $ git commit --allow-empty -m foo\n\nand test-dump-cache-tree seem to see a fully populated cache-tree\nafter these steps.\n"},{"id":"242538","messageId":"1400799746.18134.48.camel@stross","threadId":"36731","inReplyTo":"xmqqwqddla63.fsf@gitster.dls.corp.google.com","subject":"Re: git reset for index restoration?","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-22T23:02:26Z","receivedAt":"2014-05-22T23:02:26Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Thu, 2014-05-22 at 15:29 -0700, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > But at least my understanding has been that \"git commit\" (no partial\n> > commit, write the whole index as a commit) which uses the \"git\n> > write-tree\" machinery knows which subtree has what tree object name\n> > and populates the cache-tree fully.\n> \n> Here is what I tried just now.\n> \n>     $ rm .git/index\n>     $ git read-tree HEAD HEAD\n> \n> Note that a single-tree read-tree will populate the cache-tree and\n> that is why I am forcing \"switch branches\" 2-way read-tree here,\n> which I know will discard the cache-tree fully.\n> \n>     $ ls -l .git/index\n>     -rw-r----- 1 jch eng 249440 May 22 15:20 .git/index\n>     $ git checkout HEAD^0\n>     $ ls -l .git/index\n>     -rw-r----- 1 jch eng 249440 May 22 15:21 .git/index\n> \n> Still the same size, without cache-tree.\n> \n>     $ git write-tree\n>     57361c4add61b638dad1c1c2542edf877f515c48\n>     $ ls -l .git/index\n>     -rw-r----- 1 jch eng 254383 May 22 15:21 .git/index\n> \n> The size differences come from the recomputation of the cache tree.\n> The result is the same if we replace \"git write-tree\" with a\n> whole-index commit, e.g.\n> \n>     $ git commit --allow-empty -m foo\n> \n> and test-dump-cache-tree seem to see a fully populated cache-tree\n> after these steps.\n\nI get the same results as you with git write-tree.  But I do not get the\nsame results from a whole-index git commit (I tried your exact\ncommand-line).  That is, when I do git commit with no cache-tree in\nplace, it does not create one. \n\nTo expand: even if git commit did work for me the way it seems to work\nfor you, I still believe that the cache-tree behavior would be\nsuboptimal, because every time a user switches branches, they lose their\ncache-tree, and thus all of their git status commands are slow until\ntheir first commit. But I am willing to believe that my workflow is\natypical, and that most people commit enough soon after switching\nbranches.\n"},{"id":"242541","messageId":"xmqqsio1l82t.fsf@gitster.dls.corp.google.com","threadId":"36731","inReplyTo":"1400799746.18134.48.camel@stross","subject":"Re: git reset for index restoration?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-22T23:14:34Z","receivedAt":"2014-05-22T23:14:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> ... I still believe that the cache-tree behavior would be\n> suboptimal, ...\n\nI do not think anybody doubts that \"suboptimal\"-ness in this thread.\nAs you saw the \"incremental\" thing from Peff and my responses to it,\nthere may be more things we could be doing.  It just has not been at\na ultra high priority, and if we can choose only one change from\npossibilities, losing the entire cache-tree upon switching branches,\nlike in my two-way read-tree example, would be the thing that would\ngive us the most benefit if we somehow change it.\n\nThat however is unfortunately not a low-hanging fruit.  The two-way\nmerge machinery we use for switching branches wants to populate the\nindex one entry at a time, without having any point where you can\nsay \"OK the result in this subdirectory will exactly match this\nsubtree\" which would allow us to say \"prime the cache tree for that\nsubdirectory with this tree object\".\n"},{"id":"242543","messageId":"CACsJy8Bh+igRNsV0nCQ84EvvOh_ye3pqoTi3g_5ix_1LgX-EvQ@mail.gmail.com","threadId":"36731","inReplyTo":"xmqq1tvlmp8w.fsf@gitster.dls.corp.google.com","subject":"Re: git reset for index restoration?","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-22T23:33:47Z","receivedAt":"2014-05-22T23:33:47Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, May 23, 2014 at 5:18 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> ... and the \"incrementally repair\" Peff talks about would be to\n> cover more cases where we may know (either because we have already\n> computed it to write out a subtree, or we have just read from a\n> known tree to populate a part of the index and we know the paths in\n> the index that correspond to that subtree are exactly the same ones\n> as found in the tree we read from) parts of the cache-tree can be\n> updated with tree object names for subtrees, but we don't do\n> anything right now.\n\nI wanted to do this but it's hard. For \"diff --cached\" (which should\nbe where we find out and repair cache-tree), we flatten the trees in\ntraverse_trees() and let unpack-trees figure out the differences\nagainst the index. The's no direct connection between a change and a\ntree. Maybe I missed something. David if you are interested in \"git\nstatus\" performance, this repairing thing could be important when the\nworktree is large because the diff cost increases accordingly if\ncache-tree is not fully populated. Empty cache-tree could cost us\nnearly the same time we save with avoiding opendir/readdir..\n-- \nDuy\n"},{"id":"242544","messageId":"1400801861.18134.49.camel@stross","threadId":"36731","inReplyTo":"CACsJy8Bh+igRNsV0nCQ84EvvOh_ye3pqoTi3g_5ix_1LgX-EvQ@mail.gmail.com","subject":"Re: git reset for index restoration?","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-22T23:37:41Z","receivedAt":"2014-05-22T23:37:41Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Fri, 2014-05-23 at 06:33 +0700, Duy Nguyen wrote:\n> On Fri, May 23, 2014 at 5:18 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> > ... and the \"incrementally repair\" Peff talks about would be to\n> > cover more cases where we may know (either because we have already\n> > computed it to write out a subtree, or we have just read from a\n> > known tree to populate a part of the index and we know the paths in\n> > the index that correspond to that subtree are exactly the same ones\n> > as found in the tree we read from) parts of the cache-tree can be\n> > updated with tree object names for subtrees, but we don't do\n> > anything right now.\n> \n> I wanted to do this but it's hard. For \"diff --cached\" (which should\n> be where we find out and repair cache-tree), we flatten the trees in\n> traverse_trees() and let unpack-trees figure out the differences\n> against the index. The's no direct connection between a change and a\n> tree. Maybe I missed something. David if you are interested in \"git\n> status\" performance, this repairing thing could be important when the\n> worktree is large because the diff cost increases accordingly if\n> cache-tree is not fully populated. Empty cache-tree could cost us\n> nearly the same time we save with avoiding opendir/readdir..\n\nI am interested, and I believe I might be able to start looking into it\nin a week or two.\n"}]}