{"thread":{"id":"40808","subject":"[PATCH] Optimize usage of grep by passing -q","startedAt":"2015-11-16T21:43:29Z","lastAt":"2015-11-17T22:37:11Z","messageCount":4,"participants":["Stefan Beller","Mikael Magnusson","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"273384","messageId":"1447710209-13117-1-git-send-email-sbeller@google.com","threadId":"40808","inReplyTo":null,"subject":"[PATCH] Optimize usage of grep by passing -q","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-16T21:43:29Z","receivedAt":"2015-11-16T21:43:29Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Instead of redirecting all grep output to /dev/null, we can just\npass in -q instead. This preserves the exit code behavior, but is faster.\nAs grep returns true if it finds at least one match, grep can exit promptly\nafter finding the first line and doesn't need to find more occurrences\nwhich would be redirected to /dev/null anyways.\n\nThis is true for the gnu version of grep. I am not sure if all\nversions of grep support this optimization. In case it is not,\nwe'd revert this patch.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n git-bisect.sh              | 5 ++---\n git-rebase--interactive.sh | 2 +-\n git-rebase.sh              | 2 +-\n 3 files changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 5d1cb00..b909605 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -519,8 +519,7 @@ exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n \n \t\tcat \"$GIT_DIR/BISECT_RUN\"\n \n-\t\tif sane_grep \"first $TERM_BAD commit could be any of\" \"$GIT_DIR/BISECT_RUN\" \\\n-\t\t\t>/dev/null\n+\t\tif sane_grep -q \"first $TERM_BAD commit could be any of\" \"$GIT_DIR/BISECT_RUN\"\n \t\tthen\n \t\t\tgettextln \"bisect run cannot continue any more\" >&2\n \t\t\texit $res\n@@ -533,7 +532,7 @@ exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n \t\t\texit $res\n \t\tfi\n \n-\t\tif sane_grep \"is the first $TERM_BAD commit\" \"$GIT_DIR/BISECT_RUN\" >/dev/null\n+\t\tif sane_grep -q \"is the first $TERM_BAD commit\" \"$GIT_DIR/BISECT_RUN\"\n \t\tthen\n \t\t\tgettextln \"bisect run success\"\n \t\t\texit 0;\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex d65c06e..f360ac0 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -1225,7 +1225,7 @@ then\n \tgit rev-list $revisions |\n \twhile read rev\n \tdo\n-\t\tif test -f \"$rewritten\"/$rev && test \"$(sane_grep \"$rev\" \"$state_dir\"/not-cherry-picks)\" = \"\"\n+\t\tif test -f \"$rewritten\"/$rev && test \"$(sane_grep -q \"$rev\" \"$state_dir\"/not-cherry-picks)\"\n \t\tthen\n \t\t\t# Use -f2 because if rev-list is telling us this commit is\n \t\t\t# not worthwhile, we don't want to track its multiple heads,\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex af7ba5f..b6a5f73 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -578,7 +578,7 @@ mb=$(git merge-base \"$onto\" \"$orig_head\")\n if test \"$type\" != interactive && test \"$upstream\" = \"$onto\" &&\n \ttest \"$mb\" = \"$onto\" && test -z \"$restrict_revision\" &&\n \t# linear history?\n-\t! (git rev-list --parents \"$onto\"..\"$orig_head\" | sane_grep \" .* \") > /dev/null\n+\t! (git rev-list --parents \"$onto\"..\"$orig_head\" | sane_grep -q \" .* \")\n then\n \tif test -z \"$force_rebase\"\n \tthen\n-- \n2.6.3.368.gf34be46\n"},{"id":"273400","messageId":"CAHYJk3S-pBQGDk_PPXw5mgzuXhdKqifRB4Vwz1Nu0TecXz4JQg@mail.gmail.com","threadId":"40808","inReplyTo":"1447710209-13117-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] Optimize usage of grep by passing -q","fromName":"Mikael Magnusson","fromEmail":"mikachu@gmail.com","sentAt":"2015-11-17T00:59:11Z","receivedAt":"2015-11-17T00:59:11Z","isPatch":true,"sender":{"key":"mikachu@gmail.com","avatar":null},"body":"On Mon, Nov 16, 2015 at 10:43 PM, Stefan Beller <sbeller@google.com> wrote:\n> Instead of redirecting all grep output to /dev/null, we can just\n> pass in -q instead. This preserves the exit code behavior, but is faster.\n> As grep returns true if it finds at least one match, grep can exit promptly\n> after finding the first line and doesn't need to find more occurrences\n> which would be redirected to /dev/null anyways.\n>\n> This is true for the gnu version of grep. I am not sure if all\n> versions of grep support this optimization. In case it is not,\n> we'd revert this patch.\n\nPOSIX specifies -q, so you should be fine.\nhttp://pubs.opengroup.org/onlinepubs/9699919799/utilities/grep.html\n\n-- \nMikael Magnusson\n"},{"id":"273401","messageId":"CAGZ79kYCNdDV2cDZSiugx6EuY_0BEgGMJJVm8EBWqPEE77sqbw@mail.gmail.com","threadId":"40808","inReplyTo":"CAHYJk3S-pBQGDk_PPXw5mgzuXhdKqifRB4Vwz1Nu0TecXz4JQg@mail.gmail.com","subject":"Re: [PATCH] Optimize usage of grep by passing -q","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-17T01:04:24Z","receivedAt":"2015-11-17T01:04:24Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"+cc Andrey Rybak, who I credit for finding the reasoning below (he\nsent to me privately,\nwithout cc'ing the list)\n\nOn Mon, Nov 16, 2015 at 4:59 PM, Mikael Magnusson <mikachu@gmail.com> wrote:\n> On Mon, Nov 16, 2015 at 10:43 PM, Stefan Beller <sbeller@google.com> wrote:\n>> Instead of redirecting all grep output to /dev/null, we can just\n>> pass in -q instead. This preserves the exit code behavior, but is faster.\n>> As grep returns true if it finds at least one match, grep can exit promptly\n>> after finding the first line and doesn't need to find more occurrences\n>> which would be redirected to /dev/null anyways.\n>>\n>> This is true for the gnu version of grep. I am not sure if all\n>> versions of grep support this optimization. In case it is not,\n>> we'd revert this patch.\n>\n> POSIX specifies -q, so you should be fine.\n> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/grep.html\n>\n\n>From http://www.gnu.org/software/grep/manual/grep.html :\n-q\n--quiet\n--silent\nQuiet; do not write anything to standard output. Exit immediately with\nzero status if any match is found, even if an error was detected. Also\nsee the -s or --no-messages option. (-q is specified by POSIX.)\n-s\n--no-messages\nSuppress error messages about nonexistent or unreadable files.\nPortability note: unlike GNU grep, 7th Edition Unix grep did not\nconform to POSIX, because it lacked -q and its -s option behaved like\nGNU grep's -q option.1\nUSG-style grep also lacked -q but its -s option behaved like GNU\ngrep's. Portable shell scripts should avoid both -q and -s and should\nredirect standard and error output to /dev/null instead. (-s is\nspecified by POSIX.)\n\nReading that in full, I think my patch is a bad idea.\n"},{"id":"273448","messageId":"20151117223711.GC27862@sigill.intra.peff.net","threadId":"40808","inReplyTo":"CAGZ79kYCNdDV2cDZSiugx6EuY_0BEgGMJJVm8EBWqPEE77sqbw@mail.gmail.com","subject":"Re: [PATCH] Optimize usage of grep by passing -q","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-11-17T22:37:11Z","receivedAt":"2015-11-17T22:37:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 16, 2015 at 05:04:24PM -0800, Stefan Beller wrote:\n\n> >> This is true for the gnu version of grep. I am not sure if all\n> >> versions of grep support this optimization. In case it is not,\n> >> we'd revert this patch.\n> >\n> > POSIX specifies -q, so you should be fine.\n> > http://pubs.opengroup.org/onlinepubs/9699919799/utilities/grep.html\n> >\n> \n> From http://www.gnu.org/software/grep/manual/grep.html :\n> [...]\n> Portability note: unlike GNU grep, 7th Edition Unix grep did not\n> conform to POSIX, because it lacked -q and its -s option behaved like\n> GNU grep's -q option.1\n> USG-style grep also lacked -q but its -s option behaved like GNU\n> grep's. Portable shell scripts should avoid both -q and -s and should\n> redirect standard and error output to /dev/null instead. (-s is\n> specified by POSIX.)\n\nI wonder what the current state of \"most\" systems is. 7th Edition Unix\nis probably old enough for us not to worry about. :)\n\nFor the git project, being in POSIX is not an automatic pass for a\nfeature. We care about real systems. I note that we do have quite a bit\nof \"grep -q\" in the test scripts, but not in the actual git-scripts.\n\nThis came up as recently as 2008 (e.g., aadbe44), but I don't recall\nanybody complaining recently. Perhaps Solaris grep finally grew a \"-q\"\noption. Or maybe nobody runs the tests there anymore.\n\nSince this is an optimization, I'd be more interested if we had numbers\nfor the improvement. Are these files really big enough that grepping the\nrest of the file is noticeable versus the cost of starting grep in the\nfirst place?\n\nIf this is something measurable, we might be able to make it a build\nflag (e.g., by wrapping these grep invocations in a shell function in\ngit-sh-setup.sh, and picking the implementation at build time).\n\n-Peff\n"}]}