{"thread":{"id":"26814","subject":"git status reads too many files","startedAt":"2011-03-21T12:40:21Z","lastAt":"2011-03-22T00:26:54Z","messageCount":9,"participants":["Lasse Makholm","Junio C Hamano","Piotr Krukowiecki","Eric Raible"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"163905","messageId":"AANLkTikV4S51DXLADiRXWqjXdTD1OBLSdKjEWALZ9Ebh@mail.gmail.com","threadId":"26814","inReplyTo":null,"subject":"git status reads too many files","fromName":"Lasse Makholm","fromEmail":"lasse.makholm@gmail.com","sentAt":"2011-03-21T12:40:21Z","receivedAt":"2011-03-21T12:40:21Z","isPatch":false,"sender":{"key":"lasse.makholm@gmail.com","avatar":"https://gravatar.com/avatar/5bc3a34e4fa8ba26b38c333f370b15c034351e18e09b056a2608f3904ae1a4c1?d=mp&s=160"},"body":"After a git checkout, git status has a tendency to read all the files\nthat were updated during the checkout. In git.git for example:\n\n$ git checkout -b here v1.7.4\nSwitched to a new branch 'here'\n$ git checkout -b there v1.7.4.1\nSwitched to a new branch 'there'\n$ strace -o /tmp/trace1 git status\n# On branch there\nnothing to commit (working directory clean)\n$ grep ^open /tmp/trace1 | wc -l\n414\n$ git diff --name-only here..there | tail -1\nwrapper.c\n$ grep -A2 wrapper.c /tmp/trace1\nlstat(\"wrapper.c\", {st_mode=S_IFREG|0644, st_size=7617, ...}) = 0\nopen(\"wrapper.c\", O_RDONLY)             = 3\nread(3, \"/*\\n * Various trivial helper wra\"..., 7617) = 7617\nclose(3)                                = 0\n$\n\nThis persistent across multiple runs of git status:\n\n$ strace -o /tmp/trace2 git status\n# On branch there\nnothing to commit (working directory clean)\n$ grep ^open /tmp/trace2 | wc -l\n414\n$\n\n...until the index is touched:\n\n$ touch .git/index\n$ strace -o /tmp/trace3 git status\n# On branch there\nnothing to commit (working directory clean)\n$ grep ^open /tmp/trace3 | wc -l\n362\n$\n\nThis happening at least with 1.7.0.4 and 1.7.4.1.343.ga91df (master as\nof now)...\n\nFurther scrutiny reveals that when this happens, the index and the\nnewly updated files in the working tree have identical modification\ntimes, so I guess git status is reading all files which are not older\nthan the index...\n\nDiscussing this with Mr. Schindelin last week, my first thought was\nthat checkout should ensure that the index is newer than any of the\nfiles in the working tree but Johannes seems to think that instead,\nthe first git status run should touch the index, thus preventing the\nnext run from reading the files again.\n\nI'm not familiar enough with the semantics of the index to say which\nway is correct or intended, but surely the current behaviour is not\ndesirable.\n\nBug?\n\n-- \n/Lasse\n"},{"id":"163924","messageId":"7vipvcs9xt.fsf@alter.siamese.dyndns.org","threadId":"26814","inReplyTo":"AANLkTikV4S51DXLADiRXWqjXdTD1OBLSdKjEWALZ9Ebh@mail.gmail.com","subject":"Re: git status reads too many files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-21T16:41:18Z","receivedAt":"2011-03-21T16:41:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lasse Makholm <lasse.makholm@gmail.com> writes:\n\n> This persistent across multiple runs of git status:\n>\n> $ strace -o /tmp/trace2 git status\n> # On branch there\n> nothing to commit (working directory clean)\n> $ grep ^open /tmp/trace2 | wc -l\n> 414\n> $\n>\n> ...until the index is touched:\n>\n> $ touch .git/index\n\nDon't do this; you are breaking the racy-git protection.\n\nI think we opportunistically update the .git/index file in \"git status\" to\nrefresh the stat bits (but we don't error out when we cannot write a new\nindex, as you may be only browsing somebody else's repository with only a\nread access to it).  It probably should be just the matter of adding a bit\nof logic to notice that your index is racily clean.\n\nLet me cook something real quick.\n"},{"id":"163926","messageId":"7vtyewqtmk.fsf@alter.siamese.dyndns.org","threadId":"26814","inReplyTo":"7vipvcs9xt.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/2] diff/status: refactor opportunistic index update","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-21T17:16:10Z","receivedAt":"2011-03-21T17:16:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When we had to refresh the index internally before running diff or status,\nwe opportunistically updated the $GIT_INDEX_FILE so that later invocation\nof git can use the lstat(2) we already did in this invocation.\n\nMake them share a helper function to do so.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n > I think we opportunistically update the .git/index file in \"git status\" to\n > refresh the stat bits (but we don't error out when we cannot write a new\n > index, as you may be only browsing somebody else's repository with only a\n > read access to it).  It probably should be just the matter of adding a bit\n > of logic to notice that your index is racily clean.\n >\n > Let me cook something real quick.\n\n builtin/commit.c |    9 ++-------\n builtin/diff.c   |    7 +------\n cache.h          |    1 +\n read-cache.c     |   12 ++++++++++++\n 4 files changed, 16 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 66fdd22..0b6ce2f 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1090,13 +1090,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \trefresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec, NULL, NULL);\n \n \tfd = hold_locked_index(&index_lock, 0);\n-\tif (0 <= fd) {\n-\t\tif (active_cache_changed &&\n-\t\t    !write_cache(fd, active_cache, active_nr))\n-\t\t\tcommit_locked_index(&index_lock);\n-\t\telse\n-\t\t\trollback_lock_file(&index_lock);\n-\t}\n+\tif (0 <= fd)\n+\t\tupdate_index_if_able(&the_index, &index_lock);\n \n \ts.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;\n \ts.in_merge = in_merge;\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex a43d326..bab4bd9 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -197,12 +197,7 @@ static void refresh_index_quietly(void)\n \tdiscard_cache();\n \tread_cache();\n \trefresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);\n-\n-\tif (active_cache_changed &&\n-\t    !write_cache(fd, active_cache, active_nr))\n-\t\tcommit_locked_index(lock_file);\n-\n-\trollback_lock_file(lock_file);\n+\tupdate_index_if_able(&the_index, lock_file);\n }\n \n static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv)\ndiff --git a/cache.h b/cache.h\nindex 2ef2fa3..9a3cc8e 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -520,6 +520,7 @@ extern NORETURN void unable_to_lock_index_die(const char *path, int err);\n extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);\n extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);\n extern int commit_lock_file(struct lock_file *);\n+extern void update_index_if_able(struct index_state *, struct lock_file *);\n \n extern int hold_locked_index(struct lock_file *, int);\n extern int commit_locked_index(struct lock_file *);\ndiff --git a/read-cache.c b/read-cache.c\nindex 1f42473..561dc66 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1545,6 +1545,18 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce)\n \treturn result;\n }\n \n+/*\n+ * Opportunisticly update the index but do not complain if we can't\n+ */\n+void update_index_if_able(struct index_state *istate, struct lock_file *lockfile)\n+{\n+\tif (istate->cache_changed) &&\n+\t    !write_index(istate, lockfile->fd))\n+\t\tcommit_locked_index(lockfile);\n+\telse\n+\t\trollback_lock_file(lockfile);\n+}\n+\n int write_index(struct index_state *istate, int newfd)\n {\n \tgit_SHA_CTX c;\n-- \n1.7.4.1.554.gfdad8\n"},{"id":"163927","messageId":"7voc54qtmf.fsf@alter.siamese.dyndns.org","threadId":"26814","inReplyTo":"7vipvcs9xt.fsf@alter.siamese.dyndns.org","subject":"[PATCH 2/2] update $GIT_INDEX_FILE when there are racily clean entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-21T17:18:19Z","receivedAt":"2011-03-21T17:18:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Traditional \"opportunistic index update\" done by read-only \"diff\" and\n\"status\" was about updating cached lstat(2) information in the index for\nthe next round.  We missed another obvious optimization opportunity to\nwhen there are racily clean entries that will ceas to be racily clean\nby updating $GIT_INDEX_FILE.\n\nNoticed by Lasse Makholm by stracing \"git status\" in a fresh checkout and\ncounting the number of open(2) calls.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n read-cache.c |   15 ++++++++++++++-\n 1 files changed, 14 insertions(+), 1 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 561dc66..971e277 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1545,12 +1545,25 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce)\n \treturn result;\n }\n \n+static int has_racy_timestamp(struct index_state *istate)\n+{\n+\tint entries = istate->cache_nr;\n+\tint i;\n+\n+\tfor (i = 0; i < entries; i++) {\n+\t\tstruct cache_entry *ce = istate->cache[i];\n+\t\tif (is_racy_timestamp(istate, ce))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n /*\n  * Opportunisticly update the index but do not complain if we can't\n  */\n void update_index_if_able(struct index_state *istate, struct lock_file *lockfile)\n {\n-\tif (istate->cache_changed) &&\n+\tif ((istate->cache_changed || has_racy_timestamp(istate)) &&\n \t    !write_index(istate, lockfile->fd))\n \t\tcommit_locked_index(lockfile);\n \telse\n-- \n1.7.4.1.554.gfdad8\n"},{"id":"163955","messageId":"AANLkTinUqzgpiX_X+kpUuOSxNqRVp+OC1HOreEkF6yhX@mail.gmail.com","threadId":"26814","inReplyTo":"7vtyewqtmk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] diff/status: refactor opportunistic index update","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2011-03-21T18:46:22Z","receivedAt":"2011-03-21T18:46:22Z","isPatch":true,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Mon, Mar 21, 2011 at 6:16 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> +void update_index_if_able(struct index_state *istate, struct lock_file *lockfile)\n> +{\n> +       if (istate->cache_changed) &&\n> +           !write_index(istate, lockfile->fd))\n\nMismatched parenthesis? Should be sth like\n\n+       if (istate->cache_changed &&\n+           !write_index(istate, lockfile->fd))\n\n-- \nPiotr Krukowiecki\n"},{"id":"163961","messageId":"7vmxkop8js.fsf@alter.siamese.dyndns.org","threadId":"26814","inReplyTo":"AANLkTinUqzgpiX_X+kpUuOSxNqRVp+OC1HOreEkF6yhX@mail.gmail.com","subject":"Re: [PATCH 1/2] diff/status: refactor opportunistic index update","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-21T19:39:35Z","receivedAt":"2011-03-21T19:39:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Piotr Krukowiecki <piotr.krukowiecki@gmail.com> writes:\n\n> On Mon, Mar 21, 2011 at 6:16 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> +void update_index_if_able(struct index_state *istate, struct lock_file *lockfile)\n>> +{\n>> +       if (istate->cache_changed) &&\n>> +           !write_index(istate, lockfile->fd))\n>\n> Mismatched parenthesis? Should be sth like\n>\n> +       if (istate->cache_changed &&\n> +           !write_index(istate, lockfile->fd))\n\nYeah, \"rebase -i\" gotcha.  Applying [2/2] should get rid of it anyway.\n"},{"id":"163979","messageId":"AANLkTikPL7Dx5AphGnd1TVAyLNgNh2WVd__Yom134VXb@mail.gmail.com","threadId":"26814","inReplyTo":"7vipvcs9xt.fsf@alter.siamese.dyndns.org","subject":"Re: git status reads too many files","fromName":"Lasse Makholm","fromEmail":"lasse.makholm@gmail.com","sentAt":"2011-03-21T20:39:32Z","receivedAt":"2011-03-21T20:39:32Z","isPatch":false,"sender":{"key":"lasse.makholm@gmail.com","avatar":"https://gravatar.com/avatar/5bc3a34e4fa8ba26b38c333f370b15c034351e18e09b056a2608f3904ae1a4c1?d=mp&s=160"},"body":"On 21 March 2011 17:41, Junio C Hamano <gitster@pobox.com> wrote:\n> Lasse Makholm <lasse.makholm@gmail.com> writes:\n>\n>> This persistent across multiple runs of git status:\n>>\n>> $ strace -o /tmp/trace2 git status\n>> # On branch there\n>> nothing to commit (working directory clean)\n>> $ grep ^open /tmp/trace2 | wc -l\n>> 414\n>> $\n>>\n>> ...until the index is touched:\n>>\n>> $ touch .git/index\n>\n> Don't do this; you are breaking the racy-git protection.\n\nYeah, I know, I was just proving a point... git reset (--hard?) HEAD\nwould achieve the same thing...\n\n> I think we opportunistically update the .git/index file in \"git status\" to\n> refresh the stat bits (but we don't error out when we cannot write a new\n> index, as you may be only browsing somebody else's repository with only a\n> read access to it).  It probably should be just the matter of adding a bit\n> of logic to notice that your index is racily clean.\n\nI figured as much... My original thought of checkout ensuring an index\nnewer than any working file is stupid, of course, for a multitude of\nreasons -- one of which is that the \"next\" timestamp may be a full 2\nseconds away...\n\n> Let me cook something real quick.\n\nSweet, thanks...\n\n-- \n/Lasse\n"},{"id":"163985","messageId":"AANLkTiniZraECRKekxWf6K-mBXfEuGkV6Cwpp4L5pVtm@mail.gmail.com","threadId":"26814","inReplyTo":"7voc54qtmf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] update $GIT_INDEX_FILE when there are racily clean entries","fromName":"Lasse Makholm","fromEmail":"lasse.makholm@gmail.com","sentAt":"2011-03-21T21:23:00Z","receivedAt":"2011-03-21T21:23:00Z","isPatch":true,"sender":{"key":"lasse.makholm@gmail.com","avatar":"https://gravatar.com/avatar/5bc3a34e4fa8ba26b38c333f370b15c034351e18e09b056a2608f3904ae1a4c1?d=mp&s=160"},"body":"Works for me.\n\nThanks.\n--\n/Lasse\n"},{"id":"164009","messageId":"4D87ECCE.4000300@nextest.com","threadId":"26814","inReplyTo":"7voc54qtmf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] update $GIT_INDEX_FILE when there are racily clean entries","fromName":"Eric Raible","fromEmail":"raible@nextest.com","sentAt":"2011-03-22T00:26:54Z","receivedAt":"2011-03-22T00:26:54Z","isPatch":true,"sender":{"key":"raible@nextest.com","avatar":null},"body":"On 11:59 AM, Junio C Hamano wrote:\n> Traditional \"opportunistic index update\" done by read-only \"diff\" and\n> \"status\" was about updating cached lstat(2) information in the index for\n> the next round.  We missed another obvious optimization opportunity to\n> when there are racily clean entries that will ceas to be racily clean\n> by updating $GIT_INDEX_FILE.\n\ns/ceas/cease/\n"}]}