{"thread":{"id":"4154","subject":"Fix silly typo in new builtin grep","startedAt":"2006-05-16T00:54:01Z","lastAt":"2006-05-16T03:32:15Z","messageCount":6,"participants":["Linus Torvalds","Morten Welinder","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"20034","messageId":"Pine.LNX.4.64.0605151743360.3866@g5.osdl.org","threadId":"4154","inReplyTo":null,"subject":"Fix silly typo in new builtin grep","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-16T00:54:01Z","receivedAt":"2006-05-16T00:54:01Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nThe \"-F\" flag apparently got mis-translated due to some over-eager \ncopy-paste work into a duplicate \"-H\" when using the external grep.\n\nSigned-off-by: Linus Torvalds <torvalds@osdl.org>\n---\n\nMe likee the new built-in grep. The ability to say\n\n\tgit grep __make_request v2.6.13 -- '*.c'\n\nto grep for it in a specific version is well worth the fact that it \nobviously ends up being slower than grepping in the currently checked-out \ntree. It's doing a hell of a lot more, but despite that it's not at all \nthat slow.\n\n(In fact, I would say that doing the above command in just 4 seconds is \ndamn impressive - it's a large code-base, and v2.6.13 is several months, \nand over 20 _thousand_ revisions ago).\n\nAnd now it doesn't have any performance downsides, so it's all upside. \n\nGood job. Pls merge into mainline, I think the \"grep per revision\" is a \nkiller feature.\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 3d6e515..66111de 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -455,7 +455,7 @@ static int external_grep(struct grep_opt\n \tpush_arg(\"grep\");\n \tpush_arg(\"-H\");\n \tif (opt->fixed)\n-\t\tpush_arg(\"-H\");\n+\t\tpush_arg(\"-F\");\n \tif (opt->linenum)\n \t\tpush_arg(\"-n\");\n \tif (opt->regflags & REG_EXTENDED)\n"},{"id":"20035","messageId":"Pine.LNX.4.64.0605151801100.3866@g5.osdl.org","threadId":"4154","inReplyTo":"Pine.LNX.4.64.0605151743360.3866@g5.osdl.org","subject":"Re: Fix silly typo in new builtin grep","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-16T01:07:02Z","receivedAt":"2006-05-16T01:07:02Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 15 May 2006, Linus Torvalds wrote:\n> \n> Me likee the new built-in grep. The ability to say\n> \n> \tgit grep __make_request v2.6.13 -- '*.c'\n> \n> to grep for it in a specific version is well worth the fact that it \n> obviously ends up being slower than grepping in the currently checked-out \n> tree. It's doing a hell of a lot more, but despite that it's not at all \n> that slow.\n\nSide note: it looks like the version generation really isn't much of a \ncost. Grepping in v2.6.13 isn't really noticeably slower than the \n\"pre-external grep\" was for grepping in the checked-out file tree.\n\nSo it looks like we _could_ improve the grepping of specific versions \nnoticeably if we were to have a better regex library that was as optimized \nas what the external GNU grep seems to do. The actual revision data \ngeneration doesn't seem to be the biggest cost, and at least in _theory_ \nwe could probably speed things up by a factor of two with a faster regex \nlibrary.\n\nThat's good to keep in mind. It may be that the glibc regexp is just not \nvery good, but quite frankly, I would personally not be surprised if it's \nbetter than most (ie windows, for example).\n\n\t\t\tLinus\n"},{"id":"20037","messageId":"118833cc0605151910s7619ddf0x8f014adba2a1eba5@mail.gmail.com","threadId":"4154","inReplyTo":"Pine.LNX.4.64.0605151801100.3866@g5.osdl.org","subject":"Re: Fix silly typo in new builtin grep","fromName":"Morten Welinder","fromEmail":"mwelinder@gmail.com","sentAt":"2006-05-16T02:10:38Z","receivedAt":"2006-05-16T02:10:38Z","isPatch":false,"sender":{"key":"mwelinder@gmail.com","avatar":null},"body":"If I read the code right, it calls regexec for every single character\non every single line.  No wonder that takes a while!  Just call it\nonce and it'll search for its match quite nicely.\n\nIf that's not enough, the two obvious optimizations are...\n\n1. If the pattern contains no regexp characters  (and that is very\n    common), do a strstr.\n\n2. If the pattern must start with a specific character, search for that\n    by itself.\n\nM.\n"},{"id":"20039","messageId":"7vu07qfyj0.fsf@assigned-by-dhcp.cox.net","threadId":"4154","inReplyTo":"Pine.LNX.4.64.0605151743360.3866@g5.osdl.org","subject":"Re: Fix silly typo in new builtin grep","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-16T03:18:11Z","receivedAt":"2006-05-16T03:18:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> The \"-F\" flag apparently got mis-translated due to some over-eager \n> copy-paste work into a duplicate \"-H\" when using the external grep.\n\nThanks.  I've pushed it out to \"master\", along with some other\nstuff.\n\n> Me likee the new built-in grep. The ability to say\n>\n> \tgit grep __make_request v2.6.13 -- '*.c'\n>\n> to grep for it in a specific version is well worth the fact that it \n> obviously ends up being slower than grepping in the currently checked-out \n> tree. It's doing a hell of a lot more, but despite that it's not at all \n> that slow.\n>\n> (In fact, I would say that doing the above command in just 4 seconds is \n> damn impressive - it's a large code-base, and v2.6.13 is several months, \n> and over 20 _thousand_ revisions ago).\n\nThat is a BS praise and you know it ;-).  You do not have delta\nchains that are 20k long, so grepping from the tree 10 revs ago\nand from the tree 20k revs ago would not make a difference.\n\nIt _would_ be impressive to CVS folks, but even there each path\nwould not have 20k revisions.  The kernel patches tend to touch\n3 paths per patch on average, so 60k changes over 18k files\ndistributed unevenly -- my guess (I could count but haven't) is\nprobably 200 revisions at most for most frequently touched file.\n"},{"id":"20040","messageId":"Pine.LNX.4.64.0605152008170.3866@g5.osdl.org","threadId":"4154","inReplyTo":"118833cc0605151910s7619ddf0x8f014adba2a1eba5@mail.gmail.com","subject":"Re: Fix silly typo in new builtin grep","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-16T03:27:40Z","receivedAt":"2006-05-16T03:27:40Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 15 May 2006, Morten Welinder wrote:\n>\n> If I read the code right, it calls regexec for every single character\n> on every single line.  No wonder that takes a while!  Just call it\n> once and it'll search for its match quite nicely.\n\nNo, it calls it once per pattern per line.\n\nBut yes, it calls it once per line, instead of calling it on some bigger \nboundary. Partly because of the line-based output, partly probably because \nregexec() is not actually amenable to a \"<buffer,size>\" kind of usage, but \nis based on NUL-terminated strings.\n\n> 1. If the pattern contains no regexp characters  (and that is very\n>    common), do a strstr.\n> \n> 2. If the pattern must start with a specific character, search for that\n>    by itself.\n\nYeah, we could do some simple stuff, and see if it helps..\n\n\t\tLinus\n"},{"id":"20041","messageId":"Pine.LNX.4.64.0605152028050.3866@g5.osdl.org","threadId":"4154","inReplyTo":"7vu07qfyj0.fsf@assigned-by-dhcp.cox.net","subject":"Re: Fix silly typo in new builtin grep","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-16T03:32:15Z","receivedAt":"2006-05-16T03:32:15Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 15 May 2006, Junio C Hamano wrote:\n> >\n> > (In fact, I would say that doing the above command in just 4 seconds is \n> > damn impressive - it's a large code-base, and v2.6.13 is several months, \n> > and over 20 _thousand_ revisions ago).\n> \n> That is a BS praise and you know it ;-).  You do not have delta\n> chains that are 20k long, so grepping from the tree 10 revs ago\n> and from the tree 20k revs ago would not make a difference.\n\nOh, I agree. I meant in a \"general version-control sense\". I doubt a lot \nof other version-control systems could do it. Git can, exactly because \nit's whole-file based, and our deltas are limited.\n\nSo it's not that \"builtin-grep\" is wonderful. It's that _git_ is\nwonderful, and the builtin-grep just shows one of the end results.\n\nThat's why we have killer features. To show off.\n\n(That said, git will slow down a tad too - the pack-file access won't be \nas optimized for an old version tree, and so you'll seek around some more \nfor the cold-cache case).\n\n\t\tLinus\n"}]}