{"thread":{"id":"19234","subject":"(unknown)","startedAt":"2009-05-07T17:01:53Z","lastAt":"2009-05-09T16:44:28Z","messageCount":34,"participants":["Bevan Watkiss","Alex Riesen","Linus Torvalds","Björn Steinbrink","Junio C Hamano","david@lang.hm","Johan Herland","Brandon Casey","Kjetil Barvik"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"113233","messageId":"454B76988CBF42F5BCACA5061125D263@caottdt504","threadId":"19234","inReplyTo":null,"subject":"(unknown)","fromName":"Bevan Watkiss","fromEmail":"bevan.watkiss@cloakware.com","sentAt":"2009-05-07T17:01:53Z","receivedAt":"2009-05-07T17:01:53Z","isPatch":false,"sender":{"key":"bevan.watkiss@cloakware.com","avatar":null},"body":"I am trying to create a working tree for people to read from and have it\nupdate from a bare repository regularly.  Right now I am using git-pull to\nfetch the changes, but it’s running slow due to the size of my repo and the\nspeed of the hardware as it seems to be checking the working tree for any\nchanges.  \n\nIs there a way to make the pull ignore the local working tree and only look\nat files that are changed in the change sets being pulled?\n\nBevan\n"},{"id":"113235","messageId":"81b0412b0905071013y241f7eas8417127e51ff52fa@mail.gmail.com","threadId":"19234","inReplyTo":"454B76988CBF42F5BCACA5061125D263@caottdt504","subject":"Re:","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-07T17:13:51Z","receivedAt":"2009-05-07T17:13:51Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/7 Bevan Watkiss <bevan.watkiss@cloakware.com>:\n> I am trying to create a working tree for people to read from and have it\n> update from a bare repository regularly.  Right now I am using git-pull to\n> fetch the changes, but it’s running slow due to the size of my repo and the\n> speed of the hardware as it seems to be checking the working tree for any\n> changes.\n>\n> Is there a way to make the pull ignore the local working tree and only look\n> at files that are changed in the change sets being pulled?\n\nAssuming you didn't modify that directory you pull into,\ngit pull will do almost exactly what you described. Almost,\nbecause the operation (the merge) will involve looking for local\nchanges (committed and not).\n\nIt should be faster to do something like this:\n\n  git fetch && git reset --hard origin/master\n\nAgain, assuming the directory supposed to be read-only.\nOtherwise, you have to merge (i.e. git pull).\n"},{"id":"113238","messageId":"D75C0FA80F7041FFAAC50B314788AD6F@caottdt504","threadId":"19234","inReplyTo":"81b0412b0905071013y241f7eas8417127e51ff52fa@mail.gmail.com","subject":"RE:","fromName":"Bevan Watkiss","fromEmail":"bevan.watkiss@cloakware.com","sentAt":"2009-05-07T17:26:29Z","receivedAt":"2009-05-07T17:26:29Z","isPatch":false,"sender":{"key":"bevan.watkiss@cloakware.com","avatar":null},"body":"It's the looking for local changes I'm trying to avoid.  Doing a reset still\ngoes over the tree, which isn't helpful.\n\nBasically I have a copy of my tree where only git can write to it, so I know\nthe files are right.  The NAS box I have the tree on is slow, so reading the\ntree adds about 10 minutes to the process when I only want to update a few\nfiles.\n\n-----Original Message-----\nFrom: Alex Riesen [mailto:raa.lkml@gmail.com] \nSent: May 7, 2009 1:14 PM\nTo: Bevan Watkiss\nCc: git@vger.kernel.org\nSubject: Re:\n\n2009/5/7 Bevan Watkiss <bevan.watkiss@cloakware.com>:\n> I am trying to create a working tree for people to read from and have it\n> update from a bare repository regularly.  Right now I am using git-pull to\n> fetch the changes, but it’s running slow due to the size of my repo and\nthe\n> speed of the hardware as it seems to be checking the working tree for any\n> changes.\n>\n> Is there a way to make the pull ignore the local working tree and only\nlook\n> at files that are changed in the change sets being pulled?\n\nAssuming you didn't modify that directory you pull into,\ngit pull will do almost exactly what you described. Almost,\nbecause the operation (the merge) will involve looking for local\nchanges (committed and not).\n\nIt should be faster to do something like this:\n\n  git fetch && git reset --hard origin/master\n\nAgain, assuming the directory supposed to be read-only.\nOtherwise, you have to merge (i.e. git pull).\n"},{"id":"113242","messageId":"81b0412b0905071118q46eb98b0k20f148e6a179a81f@mail.gmail.com","threadId":"19234","inReplyTo":"D75C0FA80F7041FFAAC50B314788AD6F@caottdt504","subject":"Re:","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-07T18:18:01Z","receivedAt":"2009-05-07T18:18:01Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/7 Bevan Watkiss <bevan.watkiss@cloakware.com>:\n> It's the looking for local changes I'm trying to avoid.  Doing a reset still\n> goes over the tree, which isn't helpful.\n\nThe stat(2) is slow? Then try setting core.ignoreStat (see manpage\nof git config) to true: git config core.ignorestat true\nand read below.\n\n> Basically I have a copy of my tree where only git can write to it, so I know\n> the files are right.  The NAS box I have the tree on is slow, so reading the\n> tree adds about 10 minutes to the process when I only want to update a few\n> files.\n\nTry \"git checkout origin/master\". It uses index and shouldn't checkout files\nwhich are uptodate with the index. And actually, git merge should fast-forward,\nin your case and will update just the changed files...\n\nOf course, you can always compare HEAD and origin/master, and resolve\nthe changes yourself (see git diff -z --name-status), but it is unlikely to be\nany faster.\n"},{"id":"113244","messageId":"D47BEC5B0D55467894A05B6219127126@caottdt504","threadId":"19234","inReplyTo":"81b0412b0905071118q46eb98b0k20f148e6a179a81f@mail.gmail.com","subject":"RE:","fromName":"Bevan Watkiss","fromEmail":"bevan.watkiss@cloakware.com","sentAt":"2009-05-07T18:48:20Z","receivedAt":"2009-05-07T18:48:20Z","isPatch":false,"sender":{"key":"bevan.watkiss@cloakware.com","avatar":null},"body":"Still took 11 minutes.\n\nThe idea I've come up with today is something along the lines of\ngit fetch origin/master\ngit log --name-only ..<hash> | xargs git checkout -f --\n\nThis should work to quickly keep my files upto date, and I can then\nperiodically pull properly to move the HEAD.\n\nThanks for the info\n\nBevan\n\n-----Original Message-----\nFrom: Alex Riesen [mailto:raa.lkml@gmail.com] \nSent: May 7, 2009 2:18 PM\nTo: Bevan Watkiss\nCc: git@vger.kernel.org\nSubject: Re:\n\n2009/5/7 Bevan Watkiss <bevan.watkiss@cloakware.com>:\n> It's the looking for local changes I'm trying to avoid.  Doing a reset\nstill\n> goes over the tree, which isn't helpful.\n\nThe stat(2) is slow? Then try setting core.ignoreStat (see manpage\nof git config) to true: git config core.ignorestat true\nand read below.\n\n> Basically I have a copy of my tree where only git can write to it, so I\nknow\n> the files are right.  The NAS box I have the tree on is slow, so reading\nthe\n> tree adds about 10 minutes to the process when I only want to update a few\n> files.\n\nTry \"git checkout origin/master\". It uses index and shouldn't checkout files\nwhich are uptodate with the index. And actually, git merge should\nfast-forward,\nin your case and will update just the changed files...\n\nOf course, you can always compare HEAD and origin/master, and resolve\nthe changes yourself (see git diff -z --name-status), but it is unlikely to\nbe\nany faster.\n"},{"id":"113246","messageId":"alpine.LFD.2.01.0905071148500.4983@localhost.localdomain","threadId":"19234","inReplyTo":"D75C0FA80F7041FFAAC50B314788AD6F@caottdt504","subject":"RE:","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-07T18:56:05Z","receivedAt":"2009-05-07T18:56:05Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 7 May 2009, Bevan Watkiss wrote:\n> \n> Basically I have a copy of my tree where only git can write to it, so I know\n> the files are right.  The NAS box I have the tree on is slow, so reading the\n> tree adds about 10 minutes to the process when I only want to update a few\n> files.\n\nOuch.\n\nYou could try doing\n\n\t[core]\n\t\tpreloadindex = true\n\nand see if that helps some of your loads. It does limit even the parallel \ntree stat to 20 or so, but if most of your cost is in just doing the \nlstat() over the files to see that they haven't changed, you might be \ngetting a factor-of-20 speedup for at least _some_ of what you do.\n\nIf you can, it might also be interesting to see system call trace patterns \n(with times!) to see if there is something obviously horribly bad going \non. If you're running under Linux, and don't think the data contains \nanything very private, send me the output of \"strace -f -T\" of the most \nproblematic operations, and maybe I can see if I can come up with anything \ninteresting.\n\nI have long refused to use networked filesystems because I used to find \nthem -so- painful when working with CVS, so none of my performance work \nhas ever really directly concentrated on long-latency filesystems. Even \nthe index preload was all done \"blind\" with other people reporting issues \n(and happily I could see some of the effects with local filesystems and \nmultiple CPU's ;).\n\n\t\t\tLinus\n\t\n"},{"id":"113251","messageId":"A07C3E66E84D46ACB37EDC7D396CCA62@caottdt504","threadId":"19234","inReplyTo":"alpine.LFD.2.01.0905071148500.4983@localhost.localdomain","subject":"RE:","fromName":"Bevan Watkiss","fromEmail":"bevan.watkiss@cloakware.com","sentAt":"2009-05-07T19:37:59Z","receivedAt":"2009-05-07T19:37:59Z","isPatch":false,"sender":{"key":"bevan.watkiss@cloakware.com","avatar":null},"body":"Looking at the trace it does appear that most of this is the lstat.  It's\nthe problem of having many tiny files on a network drive, and trying to use\ngit for something it's not meant.\n\nThe log has 265430 lines of lstat and 10887 other lines.  If you still want\nthe log file I'll strip out the directory names and send it off.\n\nIt would be nice to have an option that you can pull only the files that\nchanged in the changesets you are updating and ignore the state of the other\nfiles.\n\nBevan\n\n-----Original Message-----\nFrom: Linus Torvalds [mailto:torvalds@linux-foundation.org] \nSent: May 7, 2009 2:56 PM\nTo: Bevan Watkiss\nCc: 'Alex Riesen'; git@vger.kernel.org\nSubject: RE: \n\n\n\nOn Thu, 7 May 2009, Bevan Watkiss wrote:\n> \n> Basically I have a copy of my tree where only git can write to it, so I\nknow\n> the files are right.  The NAS box I have the tree on is slow, so reading\nthe\n> tree adds about 10 minutes to the process when I only want to update a few\n> files.\n\nOuch.\n\nYou could try doing\n\n\t[core]\n\t\tpreloadindex = true\n\nand see if that helps some of your loads. It does limit even the parallel \ntree stat to 20 or so, but if most of your cost is in just doing the \nlstat() over the files to see that they haven't changed, you might be \ngetting a factor-of-20 speedup for at least _some_ of what you do.\n\nIf you can, it might also be interesting to see system call trace patterns \n(with times!) to see if there is something obviously horribly bad going \non. If you're running under Linux, and don't think the data contains \nanything very private, send me the output of \"strace -f -T\" of the most \nproblematic operations, and maybe I can see if I can come up with anything \ninteresting.\n\nI have long refused to use networked filesystems because I used to find \nthem -so- painful when working with CVS, so none of my performance work \nhas ever really directly concentrated on long-latency filesystems. Even \nthe index preload was all done \"blind\" with other people reporting issues \n(and happily I could see some of the effects with local filesystems and \nmultiple CPU's ;).\n\n\t\t\tLinus\n\t\n"},{"id":"113259","messageId":"20090507195609.GA17395@atjola.homenet","threadId":"19234","inReplyTo":"D47BEC5B0D55467894A05B6219127126@caottdt504","subject":"Re:","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2009-05-07T19:56:09Z","receivedAt":"2009-05-07T19:56:09Z","isPatch":false,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"[Please don't top-post...]\n\nOn 2009.05.07 14:48:20 -0400, Bevan Watkiss wrote:\n> From: Alex Riesen [mailto:raa.lkml@gmail.com] \n> > 2009/5/7 Bevan Watkiss <bevan.watkiss@cloakware.com>:\n> > > It's the looking for local changes I'm trying to avoid.  Doing a\n> > > reset still goes over the tree, which isn't helpful.\n> > \n> > The stat(2) is slow? Then try setting core.ignoreStat (see manpage\n> > of git config) to true: git config core.ignorestat true and read\n> > below.\n>\n> Still took 11 minutes.\n\nIIRC, to see the effects of core.ignorestat, you need to have updated\nall files once. So you might need, for example, \"git checkout -f HEAD\"\n(not sure if a plain checkout is enough) once first, and then the future\n\"git checkout $something\" should be faster.\n\nBjörn\n"},{"id":"113260","messageId":"alpine.LFD.2.01.0905071248250.4983@localhost.localdomain","threadId":"19234","inReplyTo":"A07C3E66E84D46ACB37EDC7D396CCA62@caottdt504","subject":"RE:","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-07T20:07:40Z","receivedAt":"2009-05-07T20:07:40Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 7 May 2009, Bevan Watkiss wrote:\n>\n> Looking at the trace it does appear that most of this is the lstat.  It's\n> the problem of having many tiny files on a network drive, and trying to use\n> git for something it's not meant.\n> \n> The log has 265430 lines of lstat and 10887 other lines.  If you still want\n> the log file I'll strip out the directory names and send it off.\n\nActually, if it's just the lstat's, then it's not all that interesting any \nmore, it's a known problem with at least a known _partial_ solution.\n\nHowever, I think it turns out that we've only enabled the index preloading \nwith \"git diff\" and \"git commit\". Not on \"git checkout\".\n\nSo start off doing that\n\n> \t[core]\n> \t\tpreloadindex = true\n\nAND apply the following patch to git, and see how much (if any) that \nhelps. It sounds like you have a pretty damn large repository, together \nwith a slow filesystem. It really could be a big improvement.\n\nThe patch is TOTALLY UNTESTED. It also worries me that 'git checkout' \nseems to do _two_ 'lstat()' calls per file. I didn't look any more \nclosely, but there may be other issues here.\n\n\t\tLinus\n\n---\n builtin-checkout.c |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex 15f0c32..3100ccd 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -216,7 +216,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,\n \tstruct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));\n \n \tnewfd = hold_locked_index(lock_file, 1);\n-\tif (read_cache() < 0)\n+\tif (read_cache_preload(pathspec) < 0)\n \t\treturn error(\"corrupt index file\");\n \n \tif (source_tree)\n@@ -367,7 +367,7 @@ static int merge_working_tree(struct checkout_opts *opts,\n \tint newfd = hold_locked_index(lock_file, 1);\n \tint reprime_cache_tree = 0;\n \n-\tif (read_cache() < 0)\n+\tif (read_cache_preload(NULL) < 0)\n \t\treturn error(\"corrupt index file\");\n \n \tcache_tree_free(&active_cache_tree);\n"},{"id":"113261","messageId":"alpine.LFD.2.01.0905071312000.4983@localhost.localdomain","threadId":"19234","inReplyTo":"alpine.LFD.2.01.0905071248250.4983@localhost.localdomain","subject":"RE:","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-07T20:20:28Z","receivedAt":"2009-05-07T20:20:28Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 7 May 2009, Linus Torvalds wrote:\n>\n> The patch is TOTALLY UNTESTED. It also worries me that 'git checkout' \n> seems to do _two_ 'lstat()' calls per file. I didn't look any more \n> closely, but there may be other issues here.\n\nHmm. The second pass comes from \n\n\tshow_local_changes(&new->commit->object);\n\n(this is the \"git checkout\" without actual filenames), and is suppressed \nif we ask for a quiet checkout. But it's sad how it re-loads the index. I \nwonder where the CE_VALID bit got dropped.\n\n\t\t\tLinus\n"},{"id":"113262","messageId":"7v63gcejad.fsf@alter.siamese.dyndns.org","threadId":"19234","inReplyTo":"alpine.LFD.2.01.0905071312000.4983@localhost.localdomain","subject":"Re:","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-07T20:43:06Z","receivedAt":"2009-05-07T20:43:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Thu, 7 May 2009, Linus Torvalds wrote:\n>>\n>> The patch is TOTALLY UNTESTED. It also worries me that 'git checkout' \n>> seems to do _two_ 'lstat()' calls per file. I didn't look any more \n>> closely, but there may be other issues here.\n>\n> Hmm. The second pass comes from \n>\n> \tshow_local_changes(&new->commit->object);\n>\n> (this is the \"git checkout\" without actual filenames), and is suppressed \n> if we ask for a quiet checkout. But it's sad how it re-loads the index. I \n> wonder where the CE_VALID bit got dropped.\n\nI do not think you mean CE_VALID; CE_UPTODATE isn't it?\n"},{"id":"113266","messageId":"alpine.LFD.2.01.0905071433250.4983@localhost.localdomain","threadId":"19234","inReplyTo":"7v63gcejad.fsf@alter.siamese.dyndns.org","subject":"Re:","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-07T21:33:37Z","receivedAt":"2009-05-07T21:33:37Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 7 May 2009, Junio C Hamano wrote:\n>\n> I do not think you mean CE_VALID; CE_UPTODATE isn't it?\n\nYes, sorry.\n\n\t\tLinus\n"},{"id":"113269","messageId":"alpine.LFD.2.01.0905071446500.4983@localhost.localdomain","threadId":"19234","inReplyTo":"alpine.LFD.2.01.0905071312000.4983@localhost.localdomain","subject":"RE:","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-07T21:55:44Z","receivedAt":"2009-05-07T21:55:44Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 7 May 2009, Linus Torvalds wrote:\n> \n> Hmm. The second pass comes from \n> \n> \tshow_local_changes(&new->commit->object);\n> \n> (this is the \"git checkout\" without actual filenames), and is suppressed \n> if we ask for a quiet checkout. But it's sad how it re-loads the index. I \n> wonder where the CE_VALID bit got dropped.\n\nAhh. It's not actually dropped, it's still there.\n\nIt's just that 'get_stat_data()' doesn't check it, when asking for \nnoncached data.\n\nThe logic of 'get_stat_data()' is that it will return the stat data from \nthe filesystem (unless we explicitly ask for just the cached case, in \nwhich case it will take it from the cache entry directly).\n\nHowever, the code doesn't realize that if ce_uptodate() is true, then we \nalready know the stat data, so no need to do the lstat() again, and we \ncan take it all from the cache entry regardless of whether we asked for \nfilesystem data or cached data.\n\nSo here's a better patch. It should cut down the 'lstat()' calls from \"git \ncheckout\" a lot.\n\nIt looks obvious enough, and it passes testing (and now \"git checkout\" \nonly does about as many lstat's as there are files in the repository, and \nthey seem to all be properly asynchronous if 'core.preloadindex' is set.\n\nSomebody should check. It would be interesting to hear about whether this \nmakes a performance impact, especially with slow filesystems and/or other \noperating systems that have a relatively higher cost for 'lstat()'.\n\n\t\tLinus\n\n---\n builtin-checkout.c |    4 ++--\n diff-lib.c         |    2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex 15f0c32..3100ccd 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -216,7 +216,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,\n \tstruct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));\n \n \tnewfd = hold_locked_index(lock_file, 1);\n-\tif (read_cache() < 0)\n+\tif (read_cache_preload(pathspec) < 0)\n \t\treturn error(\"corrupt index file\");\n \n \tif (source_tree)\n@@ -367,7 +367,7 @@ static int merge_working_tree(struct checkout_opts *opts,\n \tint newfd = hold_locked_index(lock_file, 1);\n \tint reprime_cache_tree = 0;\n \n-\tif (read_cache() < 0)\n+\tif (read_cache_preload(NULL) < 0)\n \t\treturn error(\"corrupt index file\");\n \n \tcache_tree_free(&active_cache_tree);\ndiff --git a/diff-lib.c b/diff-lib.c\nindex a310fb2..0aba6cd 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -214,7 +214,7 @@ static int get_stat_data(struct cache_entry *ce,\n \tconst unsigned char *sha1 = ce->sha1;\n \tunsigned int mode = ce->ce_mode;\n \n-\tif (!cached) {\n+\tif (!cached && !ce_uptodate(ce)) {\n \t\tint changed;\n \t\tstruct stat st;\n \t\tchanged = check_removed(ce, &st);\n"},{"id":"113272","messageId":"alpine.DEB.1.10.0905071521130.15782@asgard","threadId":"19234","inReplyTo":"alpine.LFD.2.01.0905071446500.4983@localhost.localdomain","subject":"RE:","fromName":"","fromEmail":"david@lang.hm","sentAt":"2009-05-07T22:27:32Z","receivedAt":"2009-05-07T22:27:32Z","isPatch":false,"sender":{"key":"david@lang.hm","avatar":null},"body":"On Thu, 7 May 2009, Linus Torvalds wrote:\n\nthis patch is worthwhile in itself, but the use case that is presented \nhere is slightly different, and I wonder if it's common enough to be worth \nhaving a config option for.\n\nhis use case (as I understand it) is that the working tree is never \nupdated by anything other than git. it never recieves patches or manual \nedits.\n\nas such _any_ lstats of the tree are a waste of time. if git knows what \nwas checked out before and what is being checked out now, it can find what \nfiles need to be changed.\n\nthis situation is not common for most developers, but it would be \nreasonable for build farms, so it's not just a one-person issue.\n\nDavid Lang\n\n\n> On Thu, 7 May 2009, Linus Torvalds wrote:\n>>\n>> Hmm. The second pass comes from\n>>\n>> \tshow_local_changes(&new->commit->object);\n>>\n>> (this is the \"git checkout\" without actual filenames), and is suppressed\n>> if we ask for a quiet checkout. But it's sad how it re-loads the index. I\n>> wonder where the CE_VALID bit got dropped.\n>\n> Ahh. It's not actually dropped, it's still there.\n>\n> It's just that 'get_stat_data()' doesn't check it, when asking for\n> noncached data.\n>\n> The logic of 'get_stat_data()' is that it will return the stat data from\n> the filesystem (unless we explicitly ask for just the cached case, in\n> which case it will take it from the cache entry directly).\n>\n> However, the code doesn't realize that if ce_uptodate() is true, then we\n> already know the stat data, so no need to do the lstat() again, and we\n> can take it all from the cache entry regardless of whether we asked for\n> filesystem data or cached data.\n>\n> So here's a better patch. It should cut down the 'lstat()' calls from \"git\n> checkout\" a lot.\n>\n> It looks obvious enough, and it passes testing (and now \"git checkout\"\n> only does about as many lstat's as there are files in the repository, and\n> they seem to all be properly asynchronous if 'core.preloadindex' is set.\n>\n> Somebody should check. It would be interesting to hear about whether this\n> makes a performance impact, especially with slow filesystems and/or other\n> operating systems that have a relatively higher cost for 'lstat()'.\n>\n> \t\tLinus\n>\n> ---\n> builtin-checkout.c |    4 ++--\n> diff-lib.c         |    2 +-\n> 2 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin-checkout.c b/builtin-checkout.c\n> index 15f0c32..3100ccd 100644\n> --- a/builtin-checkout.c\n> +++ b/builtin-checkout.c\n> @@ -216,7 +216,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,\n> \tstruct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));\n>\n> \tnewfd = hold_locked_index(lock_file, 1);\n> -\tif (read_cache() < 0)\n> +\tif (read_cache_preload(pathspec) < 0)\n> \t\treturn error(\"corrupt index file\");\n>\n> \tif (source_tree)\n> @@ -367,7 +367,7 @@ static int merge_working_tree(struct checkout_opts *opts,\n> \tint newfd = hold_locked_index(lock_file, 1);\n> \tint reprime_cache_tree = 0;\n>\n> -\tif (read_cache() < 0)\n> +\tif (read_cache_preload(NULL) < 0)\n> \t\treturn error(\"corrupt index file\");\n>\n> \tcache_tree_free(&active_cache_tree);\n> diff --git a/diff-lib.c b/diff-lib.c\n> index a310fb2..0aba6cd 100644\n> --- a/diff-lib.c\n> +++ b/diff-lib.c\n> @@ -214,7 +214,7 @@ static int get_stat_data(struct cache_entry *ce,\n> \tconst unsigned char *sha1 = ce->sha1;\n> \tunsigned int mode = ce->ce_mode;\n>\n> -\tif (!cached) {\n> +\tif (!cached && !ce_uptodate(ce)) {\n> \t\tint changed;\n> \t\tstruct stat st;\n> \t\tchanged = check_removed(ce, &st);\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"113273","messageId":"alpine.LFD.2.01.0905071531030.4983@localhost.localdomain","threadId":"19234","inReplyTo":"alpine.DEB.1.10.0905071521130.15782@asgard","subject":"RE:","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-07T22:36:29Z","receivedAt":"2009-05-07T22:36:29Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 7 May 2009, david@lang.hm wrote:\n> \n> his use case (as I understand it) is that the working tree is never updated by\n> anything other than git. it never recieves patches or manual edits.\n\nWell, you can certainly just use the CE_VALID bit in the index too (and \nthis time I really mean CE_VALID). But it won't help anybody else, so it \nwouldn't be nearly as interesting. And I wonder how badly that code has \nrotted, thanks to not getting used.\n\nBut yes, one thing to do would be\n\n\tgit update-index --assume-unchanged --refresh\n\nwhich should hopefully set the bit, and then after that setting \n'core.ignoreStat' should hopefully keep it set.\n\nOf course, you had then better _never_ make any mistakes and touch the \nfiles with non-git commands.\n\nAnd hope that the code still works ;)\n\n\t\tLinus\n"},{"id":"113274","messageId":"alpine.DEB.1.10.0905071543120.15782@asgard","threadId":"19234","inReplyTo":"alpine.LFD.2.01.0905071531030.4983@localhost.localdomain","subject":"RE:","fromName":"","fromEmail":"david@lang.hm","sentAt":"2009-05-07T22:43:40Z","receivedAt":"2009-05-07T22:43:40Z","isPatch":false,"sender":{"key":"david@lang.hm","avatar":null},"body":"On Thu, 7 May 2009, Linus Torvalds wrote:\n\n> On Thu, 7 May 2009, david@lang.hm wrote:\n>>\n>> his use case (as I understand it) is that the working tree is never updated by\n>> anything other than git. it never recieves patches or manual edits.\n>\n> Well, you can certainly just use the CE_VALID bit in the index too (and\n> this time I really mean CE_VALID). But it won't help anybody else, so it\n> wouldn't be nearly as interesting. And I wonder how badly that code has\n> rotted, thanks to not getting used.\n>\n> But yes, one thing to do would be\n>\n> \tgit update-index --assume-unchanged --refresh\n>\n> which should hopefully set the bit, and then after that setting\n> 'core.ignoreStat' should hopefully keep it set.\n>\n> Of course, you had then better _never_ make any mistakes and touch the\n> files with non-git commands.\n\neven with this a git checkout -f should replace all files, correct?\n\nDavid Lang\n\n> And hope that the code still works ;)\n>\n> \t\tLinus\n>\n"},{"id":"113275","messageId":"alpine.LFD.2.01.0905071553570.4983@localhost.localdomain","threadId":"19234","inReplyTo":"alpine.DEB.1.10.0905071543120.15782@asgard","subject":"RE:","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-07T23:00:14Z","receivedAt":"2009-05-07T23:00:14Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 7 May 2009, david@lang.hm wrote:\n> \n> even with this a git checkout -f should replace all files, correct?\n\nHmm. I don't think so.\n\nAs far as I recall, \"-f\" only overrides certain errors (like unmerged \nfiles or not up-to-date content), it doesn't change behavior wrt files \nthat git thinks are already up-to-date.\n\nBut I didn't check.\n\n\t\tLinus\n"},{"id":"113277","messageId":"alpine.DEB.1.10.0905071607080.15782@asgard","threadId":"19234","inReplyTo":"alpine.LFD.2.01.0905071553570.4983@localhost.localdomain","subject":"RE:","fromName":"","fromEmail":"david@lang.hm","sentAt":"2009-05-07T23:07:51Z","receivedAt":"2009-05-07T23:07:51Z","isPatch":false,"sender":{"key":"david@lang.hm","avatar":null},"body":"On Thu, 7 May 2009, Linus Torvalds wrote:\n\n> On Thu, 7 May 2009, david@lang.hm wrote:\n>>\n>> even with this a git checkout -f should replace all files, correct?\n>\n> Hmm. I don't think so.\n>\n> As far as I recall, \"-f\" only overrides certain errors (like unmerged\n> files or not up-to-date content), it doesn't change behavior wrt files\n> that git thinks are already up-to-date.\n\nwhat about a reset --hard? (is there any command that would force the \nfiles to be re-written, no matter what git thinks is already there)\n\nDavid Lang\n"},{"id":"113280","messageId":"alpine.LFD.2.01.0905071613130.4983@localhost.localdomain","threadId":"19234","inReplyTo":"alpine.DEB.1.10.0905071607080.15782@asgard","subject":"RE:","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-07T23:18:56Z","receivedAt":"2009-05-07T23:18:56Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 7 May 2009, david@lang.hm wrote:\n> \n> what about a reset --hard? (is there any command that would force the files to\n> be re-written, no matter what git thinks is already there)\n\nNo, not \"git reset --hard\" either, I think. Git very much tries to avoid \nrewriting files, and if you've told it that file contents are stable, it \nwill believe you.\n\nIn fact, I think people used CE_VALID explicitly for the missing parts of \n\"partial checkouts\", so if we'd suddenly start writing files despite them \nbeing marked as ok in the tree, I think we'd have broken that part.\n\n(Although again - I'm not sure who would use CE_VALID and friends).\n\nIf you want to force everything to be rewritten, you should just remove \nthe index (or remove the specific entries in it if you want to do it just \nto a particular file) and then do a \"git checkout\" to re-read and \nre-populate the tree.\n\nBut I'm not really seeing why you want to do this. If you told git that it \nshouldn't care about the working tree, why do you now want it do care?\n\n\t\t\tLinus\n"},{"id":"113281","messageId":"alpine.DEB.1.10.0905071629230.15782@asgard","threadId":"19234","inReplyTo":"alpine.LFD.2.01.0905071613130.4983@localhost.localdomain","subject":"RE:","fromName":"","fromEmail":"david@lang.hm","sentAt":"2009-05-07T23:31:09Z","receivedAt":"2009-05-07T23:31:09Z","isPatch":false,"sender":{"key":"david@lang.hm","avatar":null},"body":"On Thu, 7 May 2009, Linus Torvalds wrote:\n\n> On Thu, 7 May 2009, david@lang.hm wrote:\n>>\n>> what about a reset --hard? (is there any command that would force the files to\n>> be re-written, no matter what git thinks is already there)\n>\n> No, not \"git reset --hard\" either, I think. Git very much tries to avoid\n> rewriting files, and if you've told it that file contents are stable, it\n> will believe you.\n>\n> In fact, I think people used CE_VALID explicitly for the missing parts of\n> \"partial checkouts\", so if we'd suddenly start writing files despite them\n> being marked as ok in the tree, I think we'd have broken that part.\n>\n> (Although again - I'm not sure who would use CE_VALID and friends).\n>\n> If you want to force everything to be rewritten, you should just remove\n> the index (or remove the specific entries in it if you want to do it just\n> to a particular file) and then do a \"git checkout\" to re-read and\n> re-populate the tree.\n>\n> But I'm not really seeing why you want to do this. If you told git that it\n> shouldn't care about the working tree, why do you now want it do care?\n\nto be able to manually recover from the case where someone did things that \nthey weren't supposed to\n\nremoving the index and doing a checkout would be a reasonable thing to do \n(at least conceptually), I will admit that I don't remember ever seeing a \ncommand (or discussion of one) that would let me do that.\n"},{"id":"113284","messageId":"200905080157.15605.johan@herland.net","threadId":"19234","inReplyTo":"alpine.DEB.1.10.0905071629230.15782@asgard","subject":"Re:","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2009-05-07T23:57:15Z","receivedAt":"2009-05-07T23:57:15Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Friday 08 May 2009, david@lang.hm wrote:\n> removing the index and doing a checkout would be a reasonable thing to do\n> (at least conceptually), I will admit that I don't remember ever seeing a\n> command (or discussion of one) that would let me do that.\n\nWhat about:\n\n  rm .git/index\n  git checkout -f\n\nor maybe:\n\n  git update-index --no-assume-unchanged --refresh\n  git checkout -f\n\nHm?\n\n....Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"113325","messageId":"81b0412b0905080117v3aad0c44o7b3bbcc7fe70d3b1@mail.gmail.com","threadId":"19234","inReplyTo":"alpine.LFD.2.01.0905071446500.4983@localhost.localdomain","subject":"Re:","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-08T08:17:03Z","receivedAt":"2009-05-08T08:17:03Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/7 Linus Torvalds <torvalds@linux-foundation.org>:\n>\n> Somebody should check. It would be interesting to hear about whether this\n> makes a performance impact, especially with slow filesystems and/or other\n> operating systems that have a relatively higher cost for 'lstat()'.\n>\n\nI did (cygwin). My guess, the improvement is completely dwarfed by the\nother overheads (like starting git and writing files).\n\n# Without the patch\nreal    11m22.338s\nuser    0m54.629s\nsys     8m33.638s\n\n# With checkout index preload\nreal    11m14.361s\nuser    0m46.609s\nsys     7m56.300s\n\nThe script:\n\n#!/bin/sh\n\nif [ \"$1\" = setup ]; then\n    for i in 1 2 3 4\n    do\n        n=$(date)\n        for f in `seq 1 10000`\n        do\n            echo \"$n\" >file$f\n        done\n        git add .\n        printf \"Commit $i:\"\n        git commit -m\"$n\"\n    done\n    exit\nfi\n\nexport GIT_EXEC_PATH=/d/git-win\ntime for f in `seq 1 10`\ndo\n    $GIT_EXEC_PATH/git checkout master~3 &&\n    $GIT_EXEC_PATH/git checkout master~2 &&\n    $GIT_EXEC_PATH/git checkout master~1 &&\n    $GIT_EXEC_PATH/git checkout master\ndone\nexit\n"},{"id":"113349","messageId":"alpine.LFD.2.01.0905080734260.4983@localhost.localdomain","threadId":"19234","inReplyTo":"81b0412b0905080117v3aad0c44o7b3bbcc7fe70d3b1@mail.gmail.com","subject":"Re:","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-08T14:39:38Z","receivedAt":"2009-05-08T14:39:38Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 8 May 2009, Alex Riesen wrote:\n> \n> I did (cygwin). My guess, the improvement is completely dwarfed by the\n> other overheads (like starting git and writing files).\n\nOh, I meant \"git checkout\" as in not even switching branches, or perhaps \nswitching branches but just changing a single file (among thousands).\n\nIf you actually end up re-writing all files, then yes, it will obviously \nbe totally dominated by other things.\n\nFor example, in the kernel, switching between two branches that only \ndiffer in one file (Makefile) went from 0.18 seconds down to 0.14 seconds \nfor me just because of the fewer lstat() calls.\n\nNoticeable? No. But it might be more noticeable on some other OS, or with \nsome networked filesystem.\n\n\t\tLinus\n"},{"id":"113353","messageId":"eFUCK0_CEtLa6Qvg6X1SqHmCgRnY3_3dy3OCJK26lGP-_kDRyWtlRA@cipher.nrlssc.navy.mil","threadId":"19234","inReplyTo":"alpine.LFD.2.01.0905080734260.4983@localhost.localdomain","subject":"Re:","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-05-08T15:51:51Z","receivedAt":"2009-05-08T15:51:51Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Linus Torvalds wrote:\n> \n> On Fri, 8 May 2009, Alex Riesen wrote:\n>> I did (cygwin). My guess, the improvement is completely dwarfed by the\n>> other overheads (like starting git and writing files).\n> \n> Oh, I meant \"git checkout\" as in not even switching branches, or perhaps \n> switching branches but just changing a single file (among thousands).\n> \n> If you actually end up re-writing all files, then yes, it will obviously \n> be totally dominated by other things.\n> \n> For example, in the kernel, switching between two branches that only \n> differ in one file (Makefile) went from 0.18 seconds down to 0.14 seconds \n> for me just because of the fewer lstat() calls.\n> \n> Noticeable? No. But it might be more noticeable on some other OS, or with \n> some networked filesystem.\n\nplain 'git checkout' on linux kernel over NFS.\n\nBest time without patch: 1.20 seconds\n\n  0.45user 0.71system 0:01.20elapsed 96%CPU (0avgtext+0avgdata 0maxresident)k\n  0inputs+0outputs (0major+15467minor)pagefaults 0swaps\n\nBest time with patch (core.preloadindex = true): 1.10 seconds\n\n  0.43user 4.00system 0:01.10elapsed 402%CPU (0avgtext+0avgdata 0maxresident)k\n  0inputs+0outputs (0major+13999minor)pagefaults 0swaps\n\nBest time with patch (core.preloadindex = false): 0.84 seconds\n\n  0.42user 0.39system 0:00.84elapsed 96%CPU (0avgtext+0avgdata 0maxresident)k\n  0inputs+0outputs (0major+13965minor)pagefaults 0swaps\n\nBest time with read_cache_preload patch only: 1.38 seconds\n\n  0.45user 4.42system 0:01.38elapsed 352%CPU (0avgtext+0avgdata 0maxresident)k\n  0inputs+0outputs (0major+13990minor)pagefaults 0swaps\n\nThe read_cache_preload() changes actually slow things down for me for this\ncase.\n\nReduction in lstat's gives a nice 30% improvement.\n\n-brandon\n"},{"id":"113356","messageId":"049619646E0C4825A1A1D1113FE6422E@caottdt504","threadId":"19234","inReplyTo":"alpine.DEB.1.10.0905071629230.15782@asgard","subject":"RE:","fromName":"Bevan Watkiss","fromEmail":"bevan.watkiss@cloakware.com","sentAt":"2009-05-08T16:14:22Z","receivedAt":"2009-05-08T16:14:22Z","isPatch":false,"sender":{"key":"bevan.watkiss@cloakware.com","avatar":null},"body":"\n\n> -----Original Message-----\n> From: david@lang.hm [mailto:david@lang.hm]\n> Sent: May 7, 2009 7:31 PM\n> To: Linus Torvalds\n> Cc: Bevan Watkiss; 'Alex Riesen'; Git Mailing List\n> Subject: RE:\n> \n> On Thu, 7 May 2009, Linus Torvalds wrote:\n> \n> > On Thu, 7 May 2009, david@lang.hm wrote:\n> >>\n> >> what about a reset --hard? (is there any command that would force the\n> files to\n> >> be re-written, no matter what git thinks is already there)\n> >\n> > No, not \"git reset --hard\" either, I think. Git very much tries to avoid\n> > rewriting files, and if you've told it that file contents are stable, it\n> > will believe you.\n> >\n> > In fact, I think people used CE_VALID explicitly for the missing parts\n> of\n> > \"partial checkouts\", so if we'd suddenly start writing files despite\n> them\n> > being marked as ok in the tree, I think we'd have broken that part.\n> >\n> > (Although again - I'm not sure who would use CE_VALID and friends).\n> >\n> > If you want to force everything to be rewritten, you should just remove\n> > the index (or remove the specific entries in it if you want to do it\n> just\n> > to a particular file) and then do a \"git checkout\" to re-read and\n> > re-populate the tree.\n> >\n> > But I'm not really seeing why you want to do this. If you told git that\n> it\n> > shouldn't care about the working tree, why do you now want it do care?\n> \n> to be able to manually recover from the case where someone did things that\n> they weren't supposed to\n> \n> removing the index and doing a checkout would be a reasonable thing to do\n> (at least conceptually), I will admit that I don't remember ever seeing a\n> command (or discussion of one) that would let me do that.\n\nAdded the patch and now the time is down to 4 1/2 minutes.  Still a little\nslow for my needs though.  \n\nSince I'm looking for a more instantaneous update I'll probably use\nsomething more along the lines of\n\tgit fetch origin/master\n\tgit log --name-only ..HEAD\nto get the list of files that have changed and copy them from a local\nrepository.  Nightly doing a real pull to confirm the files are correct and\nup to date.\n\nBevan\n"},{"id":"113357","messageId":"alpine.LFD.2.01.0905080857130.4983@localhost.localdomain","threadId":"19234","inReplyTo":"eFUCK0_CEtLa6Qvg6X1SqHmCgRnY3_3dy3OCJK26lGP-_kDRyWtlRA@cipher.nrlssc.navy.mil","subject":"Re:","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-08T16:15:50Z","receivedAt":"2009-05-08T16:15:50Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 8 May 2009, Brandon Casey wrote:\n> \n> plain 'git checkout' on linux kernel over NFS.\n\nThanks.\n\n> Best time without patch: 1.20 seconds\n> \n>   0.45user 0.71system 0:01.20elapsed 96%CPU (0avgtext+0avgdata 0maxresident)k\n>   0inputs+0outputs (0major+15467minor)pagefaults 0swaps\n> \n> Best time with patch (core.preloadindex = true): 1.10 seconds\n> \n>   0.43user 4.00system 0:01.10elapsed 402%CPU (0avgtext+0avgdata 0maxresident)k\n>   0inputs+0outputs (0major+13999minor)pagefaults 0swaps\n> \n> Best time with patch (core.preloadindex = false): 0.84 seconds\n> \n>   0.42user 0.39system 0:00.84elapsed 96%CPU (0avgtext+0avgdata 0maxresident)k\n>   0inputs+0outputs (0major+13965minor)pagefaults 0swaps\n\nOk, that is _disgusting_. The parallelism clearly works (402%CPU), but the \nsystem time overhead is horrible. Going from 0.39s system time to 4s of \nsystem time is really quite nasty.\n\nIs there any possibility you could oprofile this (run it in a loop to get \nbetter profiles)? It very much sounds like some serious lock contention, \nand I'd love to hear more about exactly which lock it's hitting.\n\nAlso, you're already almost totally CPU-bound, with 96% CPU for the \nsingle-threaded csase. So you may be running over NFS, but your NFS server \nis likely pretty good and/or the client just captures everything in the \ncaches anyway.\n\nI don't recall what the Linux NFS stat cache timeout is, but it's less \nthan a minute. I suspect that you ran things in a tight loop, which is why \nyou then got effectively the local caching behavior for the best times. \n\nCan you do a \"best time\" check but with a 60-second pause between runs \n(and before), to see what happens when the client doesn't do caching?\n\n> Best time with read_cache_preload patch only: 1.38 seconds\n> \n>   0.45user 4.42system 0:01.38elapsed 352%CPU (0avgtext+0avgdata 0maxresident)k\n>   0inputs+0outputs (0major+13990minor)pagefaults 0swaps\n\nYeah, here you're not getting any advantage of fewer lstats, and you \nshow the same \"almost entirely CPU-bound on four cores\" behavior, and the \nsame (probable) lock contention that has pushed the system time way up.\n\n> The read_cache_preload() changes actually slow things down for me for this\n> case.\n> \n> Reduction in lstat's gives a nice 30% improvement.\n\nYes, I think the one-liner lstat avoidance is a real fix regardless. And \nthe preloading sounds like it hits serialization overhead in the kernel, \nwhich I'm not at all surprised at, but not being surprised doesn't mean \nthat I'm not interested to hear where it is.\n\nThe Linux VFS dcache itself should scale better than that (but who knows - \ncacheline ping-pong due to lock contention can easily cause a 10x slowdown \neven without being _totally_ contended all the time). So I would _suspect_ \nthat it's some NFS lock that you're seeing, but I'd love to know more.\n\nBtw, those system times are pretty high to begin with, so I'd love to know \nkernel version and see a profile even without the parallel case and \npresumably lock contention. Because while I probably have a faster \nmachine anyway, what I see iis:\n\n\t[torvalds@nehalem linux]$ /usr/bin/time git checkout\n\t0.13user 0.05system 0:00.19elapsed 98%CPU (0avgtext+0avgdata 0maxresident)k\n\t0inputs+0outputs (0major+13334minor)pagefaults 0swaps\n\nie my \"system\" time is _much_ lower than yours (and lower than your system \ntime). This is the 'without patch' time, btw, so this has extra lstat's. \nAnd my system time is still lower than my user time, so I wonder where all \n_your_ system time comes from. Your system time is much more comparable to \nuser time even in the good case, and I wonder why?\n\nCould be just that kernel code tends to have more cache misses, and my 8MB \ncache captures it all, and yours doesn't. Regardless, a profile would be \nvery interesting.\n\n\t\t\tLinus\n"},{"id":"113359","messageId":"86y6t77d8t.fsf_-_@broadpark.no","threadId":"19234","inReplyTo":"alpine.LFD.2.01.0905071446500.4983@localhost.localdomain","subject":"'git checkout' and unlink() calls (was: Re: )","fromName":"Kjetil Barvik","fromEmail":"barvik@broadpark.no","sentAt":"2009-05-08T16:47:46Z","receivedAt":"2009-05-08T16:47:46Z","isPatch":false,"sender":{"key":"barvik@broadpark.no","avatar":null},"body":"* Linus Torvalds <torvalds@linux-foundation.org> writes:\n| So here's a better patch. It should cut down the 'lstat()' calls from\n| \"git checkout\" a lot.\n|\n| It looks obvious enough, and it passes testing (and now \"git checkout\"\n| only does about as many lstat's as there are files in the repository,\n| and they seem to all be properly asynchronous if 'core.preloadindex'\n| is set.\n\n  I did a test by switching from v2.6.27 to v2.6.25, and now the only\n  \"lstat()-difference\" between with and without the -q option is 2\n  lstat() calls extra done without the -q option.  And, compared to over\n  41 000 lstat() calls, that is not noticable. Very good!\n\n| Somebody should check. It would be interesting to hear about whether\n| this makes a performance impact, especially with slow filesystems\n| and/or other operating systems that have a relatively higher cost for\n| 'lstat()'.\n\n  Below is a table which is output from\n\n      strace -o result -T git checkout my-v2.6.25   /* from my-v2.6.27 */\n\n  where the \"result\" file is run through a perl script to pretty print it:\n\nTOTAL        113988 100.000% OK:107252 NOT:  6736   6.263578 sec   55 usec/call\nlstat64       41114  36.069% OK: 35829 NOT:  5285   0.710936 sec   17 usec/call\nopen          15027  13.183% OK: 13872 NOT:  1155   0.559302 sec   37 usec/call\nunlink        14379  12.614% OK: 14374 NOT:     5   3.720167 sec  259 usec/call\nwrite         14207  12.464% OK: 14207 NOT:     0   0.754196 sec   53 usec/call\nclose         13872  12.170% OK: 13872 NOT:     0   0.185572 sec   13 usec/call\nfstat64       13862  12.161% OK: 13862 NOT:     0   0.169952 sec   12 usec/call\nrmdir           551   0.483% OK:   269 NOT:   282   0.035534 sec   64 usec/call\nbrk             510   0.447% OK:   510 NOT:     0   0.014804 sec   29 usec/call\nmkdir           174   0.153% OK:   174 NOT:     0   0.102625 sec  590 usec/call\nmmap2           102   0.089% OK:   102 NOT:     0   0.001725 sec   17 usec/call\nread             68   0.060% OK:    68 NOT:     0   0.000999 sec   15 usec/call\nmunmap           61   0.054% OK:    61 NOT:     0   0.005037 sec   83 usec/call\naccess           20   0.018% OK:    12 NOT:     8   0.000348 sec   17 usec/call\nmprotect         13   0.011% OK:    13 NOT:     0   0.000193 sec   15 usec/call\nstat64            7   0.006% OK:     7 NOT:     0   0.000109 sec   16 usec/call\ngetcwd            3   0.003% OK:     3 NOT:     0   0.000053 sec   18 usec/call\nchdir             3   0.003% OK:     3 NOT:     0   0.000048 sec   16 usec/call\nfcntl64           3   0.003% OK:     3 NOT:     0   0.000036 sec   12 usec/call\nrename            2   0.002% OK:     2 NOT:     0   0.001553 sec  776 usec/call\nsetitimer         2   0.002% OK:     2 NOT:     0   0.000028 sec   14 usec/call\ngetdents64        2   0.002% OK:     2 NOT:     0   0.000039 sec   20 usec/call\nuname             1   0.001% OK:     1 NOT:     0   0.000013 sec   13 usec/call\ntime              1   0.001% OK:     1 NOT:     0   0.000011 sec   11 usec/call\nfutex             1   0.001% OK:     1 NOT:     0   0.000013 sec   13 usec/call\nreadlink          1   0.001% OK:     0 NOT:     1   0.000018 sec   18 usec/call\nexecve            1   0.001% OK:     1 NOT:     0   0.000256 sec  256 usec/call\ngetrlimit         1   0.001% OK:     1 NOT:     0   0.000011 sec   11 usec/call\n\n  So, if the numbers from strace is trustable, 0.71 seconds is used on\n  41 114 calls to lstat64().  But, look at the unlink line, where each\n  call took 259 microseconds (= 0.259 milliseconds), and all 14 379\n  calls took 3.72 seconds.\n\n  It should be noted that when switching branch the other way (from .25\n  to .27), the unlink() calls used less time (below 160 microseconds\n  each).  Also note that the above was tested by only 3 runs.  Warm\n  cache.  ext4 disk partition with git compiled with the USE_NSEC=1\n  option.\n\n  Most (all?) of the unlink() calls seems to be from the following lines\n  from the checkout_entry() funciton in entry.c\n\n\t/*\n\t * We unlink the old file, to get the new one with the\n\t * right permissions (including umask, which is nasty\n\t * to emulate by hand - much easier to let the system\n\t * just do the right thing)\n\t */\n\tif (S_ISDIR(st.st_mode)) {\n\t\t/* If it is a gitlink, leave it alone! */\n\t\tif (S_ISGITLINK(ce->ce_mode))\n\t\t\treturn 0;\n\t\tif (!state->force)\n\t\t\treturn error(\"%s is a directory\", path);\n\t\tremove_subtree(path);\n\t} else if (unlink(path))\n\t\treturn error(\"unable to unlink old '%s' (%s)\", path, strerror(errno));\n\n  -- kjetil\n"},{"id":"113360","messageId":"Ah7lj3UWxgwxNiQs6kqiiVurulv4F00ssWrb3OzfTrXYlK8ZBCSBOQ@cipher.nrlssc.navy.mil","threadId":"19234","inReplyTo":"alpine.LFD.2.01.0905080857130.4983@localhost.localdomain","subject":"Re:","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-05-08T17:27:05Z","receivedAt":"2009-05-08T17:27:05Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Linus Torvalds wrote:\n> \n> On Fri, 8 May 2009, Brandon Casey wrote:\n>> plain 'git checkout' on linux kernel over NFS.\n> \n> Thanks.\n> \n>> Best time without patch: 1.20 seconds\n>>\n>>   0.45user 0.71system 0:01.20elapsed 96%CPU (0avgtext+0avgdata 0maxresident)k\n>>   0inputs+0outputs (0major+15467minor)pagefaults 0swaps\n>>\n>> Best time with patch (core.preloadindex = true): 1.10 seconds\n>>\n>>   0.43user 4.00system 0:01.10elapsed 402%CPU (0avgtext+0avgdata 0maxresident)k\n>>   0inputs+0outputs (0major+13999minor)pagefaults 0swaps\n>>\n>> Best time with patch (core.preloadindex = false): 0.84 seconds\n>>\n>>   0.42user 0.39system 0:00.84elapsed 96%CPU (0avgtext+0avgdata 0maxresident)k\n>>   0inputs+0outputs (0major+13965minor)pagefaults 0swaps\n> \n> Ok, that is _disgusting_. The parallelism clearly works (402%CPU), but the \n> system time overhead is horrible. Going from 0.39s system time to 4s of \n> system time is really quite nasty.\n> \n> Is there any possibility you could oprofile this (run it in a loop to get \n> better profiles)? It very much sounds like some serious lock contention, \n> and I'd love to hear more about exactly which lock it's hitting.\n\nPossibly, I'll see if our sysadmin has time to \"play\".\n\n> Also, you're already almost totally CPU-bound, with 96% CPU for the \n> single-threaded csase. So you may be running over NFS, but your NFS server \n> is likely pretty good and/or the client just captures everything in the \n> caches anyway.\n> \n> I don't recall what the Linux NFS stat cache timeout is, but it's less \n> than a minute. I suspect that you ran things in a tight loop, which is why \n> you then got effectively the local caching behavior for the best times. \n\nYeah, that's what I did.\n\n> Can you do a \"best time\" check but with a 60-second pause between runs \n> (and before), to see what happens when the client doesn't do caching?\n\nNo problem.\n\n>> Best time with read_cache_preload patch only: 1.38 seconds\n>>\n>>   0.45user 4.42system 0:01.38elapsed 352%CPU (0avgtext+0avgdata 0maxresident)k\n>>   0inputs+0outputs (0major+13990minor)pagefaults 0swaps\n> \n> Yeah, here you're not getting any advantage of fewer lstats, and you \n> show the same \"almost entirely CPU-bound on four cores\" behavior, and the \n> same (probable) lock contention that has pushed the system time way up.\n> \n>> The read_cache_preload() changes actually slow things down for me for this\n>> case.\n>>\n>> Reduction in lstat's gives a nice 30% improvement.\n> \n> Yes, I think the one-liner lstat avoidance is a real fix regardless. And \n> the preloading sounds like it hits serialization overhead in the kernel, \n> which I'm not at all surprised at, but not being surprised doesn't mean \n> that I'm not interested to hear where it is.\n> \n> The Linux VFS dcache itself should scale better than that (but who knows - \n> cacheline ping-pong due to lock contention can easily cause a 10x slowdown \n> even without being _totally_ contended all the time). So I would _suspect_ \n> that it's some NFS lock that you're seeing, but I'd love to know more.\n> \n> Btw, those system times are pretty high to begin with, so I'd love to know \n> kernel version and see a profile even without the parallel case and \n> presumably lock contention. Because while I probably have a faster \n> machine anyway, what I see iis:\n> \n> \t[torvalds@nehalem linux]$ /usr/bin/time git checkout\n> \t0.13user 0.05system 0:00.19elapsed 98%CPU (0avgtext+0avgdata 0maxresident)k\n> \t0inputs+0outputs (0major+13334minor)pagefaults 0swaps\n> \n> ie my \"system\" time is _much_ lower than yours (and lower than your system \n> time). This is the 'without patch' time, btw, so this has extra lstat's. \n> And my system time is still lower than my user time, so I wonder where all \n> _your_ system time comes from. Your system time is much more comparable to \n> user time even in the good case, and I wonder why?\n> \n> Could be just that kernel code tends to have more cache misses, and my 8MB \n> cache captures it all, and yours doesn't. Regardless, a profile would be \n> very interesting.\n\nSomething is definitely up.\n\nI provided timing results for your original preload_cache implementation\nwhich affected status and diff, which was part of the justification for\nmerging it in.\n\n   http://article.gmane.org/gmane.comp.version-control.git/100998\n\nYou can see that cold cache system time for 'git status' went from 0.36 to\n0.52 seconds.  Fine.  I just ran it again, and now I'm getting system time\nof 10 seconds!  This is the same machine.\n\nSimilarly for the cold cache 'git checkout' reruns:\n\nBest without patch: 6.02 (systime 1.57)\n\n  0.43user 1.57system 0:06.02elapsed 33%CPU (0avgtext+0avgdata 0maxresident)k\n  5336inputs+0outputs (12major+15472minor)pagefaults 0swaps\n\nBest with patch (preload_cache,lstat reduction): 2.69 (systime 10.47)\n\n  0.45user 10.47system 0:02.69elapsed 405%CPU (0avgtext+0avgdata 0maxresident)k\n  5336inputs+0outputs (12major+13985minor)pagefaults 0swaps\n\n\nOS: Centos4.7\n\n$ cat /proc/version\nLinux version 2.6.9-78.0.17.ELsmp (mockbuild@builder16.centos.org) (gcc version 3.4.6 20060404 (Red Hat 3.4.6-9)) #1 SMP Thu Mar 12 20:05:15 EDT 2009\n\n-brandon\n"},{"id":"113361","messageId":"OWEdfN5mNBoNl1TcdOvhhNfi_nLsao-aFrHkz_rNtuX_4lqXHisfcQ@cipher.nrlssc.navy.mil","threadId":"19234","inReplyTo":"Ah7lj3UWxgwxNiQs6kqiiVurulv4F00ssWrb3OzfTrXYlK8ZBCSBOQ@cipher.nrlssc.navy.mil","subject":"Re:","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-05-08T17:43:52Z","receivedAt":"2009-05-08T17:43:52Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Brandon Casey wrote:\n> Linus Torvalds wrote:\n>> And \n>> the preloading sounds like it hits serialization overhead in the kernel, \n>> which I'm not at all surprised at, but not being surprised doesn't mean \n>> that I'm not interested to hear where it is.\n>>\n>> The Linux VFS dcache itself should scale better than that (but who knows - \n>> cacheline ping-pong due to lock contention can easily cause a 10x slowdown \n>> even without being _totally_ contended all the time). So I would _suspect_ \n>> that it's some NFS lock that you're seeing, but I'd love to know more.\n>>\n>> Btw, those system times are pretty high to begin with, so I'd love to know \n>> kernel version and see a profile even without the parallel case and \n>> presumably lock contention.\n\nHere's an strace of 'git checkout':\n\nBefore (cold cache):\n% time     seconds  usecs/call     calls    errors syscall\n------ ----------- ----------- --------- --------- ----------------\n 98.60    6.365501         111     57432           lstat64\n  0.50    0.031984         359        89         2 close\n  0.25    0.015818         115       137        77 open\n  0.12    0.007670          23       339           write\n  0.09    0.005631         110        51           munmap\n  0.08    0.004873          49        99        69 stat64\n  0.07    0.004771         140        34        15 access\n  0.05    0.003083         280        11         5 waitpid\n  0.05    0.002973          10       284           brk\n  0.04    0.002816         469         6           execve\n<snip>\n\nAfter (cold cache, no lstat fix, just cache_preload):\n% time     seconds  usecs/call     calls    errors syscall\n------ ----------- ----------- --------- --------- ----------------\n 90.90   23.717981         413     57432           lstat64\n  8.72    2.273917      162423        14         2 futex\n  0.12    0.032241         948        34           close\n  0.04    0.011507         202        57           munmap\n  0.04    0.009648         132        73           mmap2\n  0.03    0.008508         149        57        20 open\n  0.03    0.007771         311        25           mprotect\n  0.03    0.007758         388        20           clone\n  0.03    0.007548          23       334           write\n  0.02    0.005247         262        20        10 access\n\n-brandon\n"},{"id":"113362","messageId":"alpine.LFD.2.01.0905081038160.4983@localhost.localdomain","threadId":"19234","inReplyTo":"Ah7lj3UWxgwxNiQs6kqiiVurulv4F00ssWrb3OzfTrXYlK8ZBCSBOQ@cipher.nrlssc.navy.mil","subject":"Re:","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-08T17:44:02Z","receivedAt":"2009-05-08T17:44:02Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 8 May 2009, Brandon Casey wrote:\n> \n> Something is definitely up.\n> \n> I provided timing results for your original preload_cache implementation\n> which affected status and diff, which was part of the justification for\n> merging it in.\n> \n>    http://article.gmane.org/gmane.comp.version-control.git/100998\n> \n> You can see that cold cache system time for 'git status' went from 0.36 to\n> 0.52 seconds.  Fine.  I just ran it again, and now I'm getting system time\n> of 10 seconds!  This is the same machine.\n\nGrr.\n\n> OS: Centos4.7\n> \n> $ cat /proc/version\n> Linux version 2.6.9-78.0.17.ELsmp (mockbuild@builder16.centos.org) (gcc version 3.4.6 20060404 (Red Hat 3.4.6-9)) #1 SMP Thu Mar 12 20:05:15 EDT 2009\n\nOk, if that's really the true kernel version (2.6.9), then that's some \nancient kernel there. At the same time it's obviously been recompiled \nrecently, so it got updated. At a guess, something got screwed up. But I \nhave absolutely _no_ way to even guess what kernel patches centos puts in \ntheir ancient kernel builds.\n\nPerhaps a centos bugzilla entry might be appropriate? Somebody there might \nknow what changed.\n\nOf course, it _could_ be an external change too, where the NFS server or \ntiming changed just enough to trigger a pre-existing issue. But that would \nbe pretty unlikely.\n\n\t\t\tLinus\n"},{"id":"113363","messageId":"alpine.LFD.2.01.0905081050420.4983@localhost.localdomain","threadId":"19234","inReplyTo":"86y6t77d8t.fsf_-_@broadpark.no","subject":"Re: 'git checkout' and unlink() calls (was: Re: )","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-08T17:57:32Z","receivedAt":"2009-05-08T17:57:32Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 8 May 2009, Kjetil Barvik wrote:\n> \n>   So, if the numbers from strace is trustable, 0.71 seconds is used on\n>   41 114 calls to lstat64().  But, look at the unlink line, where each\n>   call took 259 microseconds (= 0.259 milliseconds), and all 14 379\n>   calls took 3.72 seconds.\n\nThe system call times from strace are not really trustworthy. The overhead \nof tracing and in particular all the context switching back and forth \nbetween the tracer and the tracee means that the numbers should be taken \nwith a large grain of salt. \n\nThat said, they definitely aren't totally made up, and they tend to show \nreal issues.\n\nIn this particular case, what is going on is that 'lstat()' does no IO at \nall, while 'unlink()' generally at the very least will add things to some \njournal etc, and when the journal fills up, it will force IO.\n\nSo doing 15k unlink() calls really is _much_ more expensive than doing 41k \nlstat() calls, since the latter will never force any IO at all (ok, so \neven doing just an lstat() may add atime updates etc to directories, but \neven if atime is enabled, that tends to only trigger one IO per second at \nmost, and we never have to do any sync IO).\n\n>   It should be noted that when switching branch the other way (from .25\n>   to .27), the unlink() calls used less time (below 160 microseconds\n>   each).\n\nI don't think they are really \"260 us each\" or \"160 us each\". It's rather \nmore likely that there are a few that are big due to forced IO, and most \nare in the couple of us case.\n\n\t\tLinus\n"},{"id":"113374","messageId":"alpine.LFD.2.01.0905081432150.4983@localhost.localdomain","threadId":"19234","inReplyTo":"OWEdfN5mNBoNl1TcdOvhhNfi_nLsao-aFrHkz_rNtuX_4lqXHisfcQ@cipher.nrlssc.navy.mil","subject":"Re:","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-08T21:49:50Z","receivedAt":"2009-05-08T21:49:50Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 8 May 2009, Brandon Casey wrote:\n> \n> Before (cold cache):\n> % time     seconds  usecs/call     calls    errors syscall\n> ------ ----------- ----------- --------- --------- ----------------\n>  98.60    6.365501         111     57432           lstat64\n> \n> After (cold cache, no lstat fix, just cache_preload):\n> % time     seconds  usecs/call     calls    errors syscall\n> ------ ----------- ----------- --------- --------- ----------------\n>  90.90   23.717981         413     57432           lstat64\n\nYes, interesting. I really smells like it's all fixed performance and \nthere is a single lock around it. That 111us -> 413us increase is very \nconsistent with four cores all serializing on the same lock. So it \nparallelizes to all four cores, but then will take exactly as long in \ntotal.\n\nQuite frankly, 2.6.9 is so old that I have absolutely _no_ memory of what \nwe used to do back then. Not that I follow NFS all that much even now - I \ndid some of the original page cache and dentry work on the Linux NFS \nclient way back when, but that was when I actually used NFS (and we were \nconverting everything to the page cache).\n\nI've long since forgotten everything I knew, and I'm just as happy about \nthat. But clearly something is bad, and equally clearly it worked much \nbetter for you a couple of months ago. Which does imply that there's \nprobably some centos issues.\n\nCan you ask your MIS people if it would be possible to at least _test_ a \nnew kernel? In 2.6.9, I'm quite frankly inclined to just say \"it will \nlikely never get fixed unless centos knows what it is\", but if you test a \nmore modern kernel and see similar issues, then I'll be intrigued.\n\nIt's kind of sad, but at the same time, NFS was using the BKL up into \n2.6.26 or something like that (about a year ago). And your kernel is \nbased on something _much_ older.\n\nThat said, even with the BKL, NFS should allow all the actual IO to be \ndone in parallel (since the BKL is dropped on scheduling). But it's really \nwasting a _lot_ of CPU time, and that hurts you enormously, even though \nthe cold-cache case still seems to win, judging by your other email:\n\n> Best without patch: 6.02 (systime 1.57)\n> \n>   0.43user 1.57system 0:06.02elapsed 33%CPU (0avgtext+0avgdata 0maxresident)k\n>   5336inputs+0outputs (12major+15472minor)pagefaults 0swaps\n> \n> Best with patch (preload_cache,lstat reduction): 2.69 (systime 10.47)\n> \n>   0.45user 10.47system 0:02.69elapsed 405%CPU (0avgtext+0avgdata 0maxresident)k\n>   5336inputs+0outputs (12major+13985minor)pagefaults 0swaps\n\nso there's a _huge_ increase in system time (again), but the change from \n33% CPU -> 405% CPU makes up for it and you get lower elapsed times.\n\nBut that 7x increase in system time really is sad. I do suspect it's \nlikely due to spinning on the BKL. And if so, then a modern kernel should \nfix it.\n\n\t\t\tLinus\n"},{"id":"113379","messageId":"twBG1KnSrgPNk7NoVey4mgig1BeAk7e1GHOT90PSV9ZGTs-zCWYdtA@cipher.nrlssc.navy.mil","threadId":"19234","inReplyTo":"alpine.LFD.2.01.0905081432150.4983@localhost.localdomain","subject":"Re:","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-05-08T23:04:52Z","receivedAt":"2009-05-08T23:04:52Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Linus Torvalds wrote:\n> \n> On Fri, 8 May 2009, Brandon Casey wrote:\n>> Before (cold cache):\n>> % time     seconds  usecs/call     calls    errors syscall\n>> ------ ----------- ----------- --------- --------- ----------------\n>>  98.60    6.365501         111     57432           lstat64\n>>\n>> After (cold cache, no lstat fix, just cache_preload):\n>> % time     seconds  usecs/call     calls    errors syscall\n>> ------ ----------- ----------- --------- --------- ----------------\n>>  90.90   23.717981         413     57432           lstat64\n> \n> Yes, interesting. I really smells like it's all fixed performance and \n> there is a single lock around it. That 111us -> 413us increase is very \n> consistent with four cores all serializing on the same lock. So it \n> parallelizes to all four cores, but then will take exactly as long in \n> total.\n\nMakes sense to me.\n\n> Quite frankly, 2.6.9 is so old that I have absolutely _no_ memory of what \n> we used to do back then. Not that I follow NFS all that much even now - I \n> did some of the original page cache and dentry work on the Linux NFS \n> client way back when, but that was when I actually used NFS (and we were \n> converting everything to the page cache).\n> \n> I've long since forgotten everything I knew, and I'm just as happy about \n> that. But clearly something is bad, and equally clearly it worked much \n> better for you a couple of months ago. Which does imply that there's \n> probably some centos issues.\n\nIn case you're not aware CentOS is just repacked RHEL.  I'm not sure if\ncentos has the resources for investigating problems.  We also have RHEL\nlicenses, so hopefully I'll be able to come up with something to submit\nto them.\n\n> Can you ask your MIS people if it would be possible to at least _test_ a \n> new kernel? In 2.6.9, I'm quite frankly inclined to just say \"it will \n> likely never get fixed unless centos knows what it is\", but if you test a \n> more modern kernel and see similar issues, then I'll be intrigued.\n\nI think it's possible.  Just not on this specific machine.  Not sure what\nwe have lying around multi-processor wise.  Also, it won't happen until\nnext week since it's late Friday afternoon here.\n\nbtw, I've since done some more testing on some centos5.3 boxes we have.\nI get similar results (less ancient kernel 2.6.18).  I've also scanned\nthrough the errata announcements that RedHat has released for their\nkernel updates.  A few of them involve NFS.  Possibly, whatever RedHat\nmodified in the 5.X kernel was also backported to the 4.X kernel.\n\n> It's kind of sad, but at the same time, NFS was using the BKL up into \n> 2.6.26 or something like that (about a year ago). And your kernel is \n> based on something _much_ older.\n> \n> That said, even with the BKL, NFS should allow all the actual IO to be \n> done in parallel (since the BKL is dropped on scheduling). But it's really \n> wasting a _lot_ of CPU time, and that hurts you enormously, even though \n> the cold-cache case still seems to win, judging by your other email:\n>\n>> Best without patch: 6.02 (systime 1.57)\n>>\n>>   0.43user 1.57system 0:06.02elapsed 33%CPU (0avgtext+0avgdata 0maxresident)k\n>>   5336inputs+0outputs (12major+15472minor)pagefaults 0swaps\n>>\n>> Best with patch (preload_cache,lstat reduction): 2.69 (systime 10.47)\n>>\n>>   0.45user 10.47system 0:02.69elapsed 405%CPU (0avgtext+0avgdata 0maxresident)k\n>>   5336inputs+0outputs (12major+13985minor)pagefaults 0swaps\n> \n> so there's a _huge_ increase in system time (again), but the change from \n> 33% CPU -> 405% CPU makes up for it and you get lower elapsed times.\n> \n> But that 7x increase in system time really is sad. I do suspect it's \n> likely due to spinning on the BKL. And if so, then a modern kernel should \n> fix it.\n\nThanks, I'll try to test next week.\n\n-brandon\n"},{"id":"113415","messageId":"alpine.LFD.2.01.0905090926300.3586@localhost.localdomain","threadId":"19234","inReplyTo":"twBG1KnSrgPNk7NoVey4mgig1BeAk7e1GHOT90PSV9ZGTs-zCWYdtA@cipher.nrlssc.navy.mil","subject":"Re:","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-09T16:44:28Z","receivedAt":"2009-05-09T16:44:28Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 8 May 2009, Brandon Casey wrote:\n> \n> btw, I've since done some more testing on some centos5.3 boxes we have.\n> I get similar results (less ancient kernel 2.6.18).\n\nYes, 2.6.18 is still much too old to matter from a locking standpoint. \n\nWhen people initially worried about scalability, the issues were more \nabout server side stuff and the cached cases. NFS (as a client) is \ncertainly used on the server side too, but it tends to be a somewhat \nsecondary worry where only specific parts really matter. So people worked \na lot more on the core kernel, and on local high-performance filesystem \nscaling.\n\nOnly lately have we been pretty aggressive about finally really getting \nrid of the old \"single big lock\" (BKL) model entirely, or moving outwards \nfrom the core.\n\nAnd while we removed the BKL from the normal NFS read/write paths long \nlong ago, all the name lookup and directory handling code still had it \nuntil a year ago.\n\nThat, btw, is directly explained by perceived scalability issues: NFS is \nfairly often used as the backing store for a database and scaling thus \nmatters there. But databases tend to keep their few big files open and use \npread/pwrite - so pathname lookup is not nearly as significant for server \nops as plain read/write.\n\n(Pathname lookup is important for things like web servers etc, but they \nrely heavily on caching for that, and the cached case scales fine).\n\n> I've also scanned through the errata announcements that RedHat has \n> released for their kernel updates.  A few of them involve NFS.  \n> Possibly, whatever RedHat modified in the 5.X kernel was also backported \n> to the 4.X kernel.\n\nThat is very possibly the case. Expanding the BKL usage in some case could \neasily trigger the lock getting contention - and the way lock contention \nworks, once you get a just even a small _hint_ of contention, things often \nfall off a cliff. The contention slows locking down, which in turn causes \nmore CPU usage, which in turn causes _more_ contention.\n\nSo even a small amount of extra locking - or even just slowing down some \ncode that was inside the lock - can have catastrophic behavioural changes \nwhen the lock is close to being a problem. You do not get a nice gradual \nslowdown at all - you just hit a hard wall.\n\nI guess I should really try to set up some fileserver here at home to \nimprove my test coverage. And to do better backups (or the little private \ndata I have that I can't just mirror out to the world by turning it into \nan open-source project ;^)\n\n\t\t\t\tLinus\n"}]}